Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support local builds for all installers (Docker, MSI, Pip, Debian, RPM) #4696

Merged
merged 1 commit into from
Oct 18, 2017

Conversation

derekbekoe
Copy link
Member

  • packaged_releases -> build_scripts
  • Docker: build Docker image from src code directory, support private SDKs
  • Debian: local builds
  • MSI: local builds
  • remove old scripts
  • RPM: local builds

@derekbekoe
Copy link
Member Author

@troydai @yugangw-msft This should be the last PR from me that's required for the Wednesday deadline.

- Docker: build Docker image from src code directory, support private SDKs
- Debian: local builds
- MSI: local builds
- remove old scripts
- RPM: local builds
@@ -9,8 +9,7 @@
%define name azure-cli
%define release 1%{?dist}
%define version %{getenv:CLI_VERSION}
%define source_sha256 %{getenv:CLI_DOWNLOAD_SHA256}
%define source_url https://azurecliprod.blob.core.windows.net/releases/azure-cli_packaged_%{version}.tar.gz
%define repo_path %{getenv:REPO_PATH}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Too bad that relative paths are not supported, which otherwise would save an environment variable

Copy link
Member Author

@derekbekoe derekbekoe Oct 18, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added it so user doesn't have to run script from root of repo.
I should be able to remove it though. Will take a look in another PR on improvements to release process.

if %errorlevel% neq 0 goto ERROR
:: Install them to the temp folder so to be packaged
%BUILDING_DIR%\python.exe -m pip install -f %TEMP_SCRATCH_FOLDER% --no-cache-dir azure-cli

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can remove a couple of empty lines here

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can do.
Will do in next CI so I don't have to wait for CI to pass again.

@@ -74,7 +56,8 @@ make install
# It does not affect the built .deb file though.
$source_dir/python_env/bin/pip3 install wheel
for d in $source_dir/src/azure-cli $source_dir/src/azure-cli-core $source_dir/src/azure-cli-nspkg $source_dir/src/azure-cli-command_modules-nspkg $source_dir/src/command_modules/azure-cli-*/; do cd $d; $source_dir/python_env/bin/python3 setup.py bdist_wheel -d $tmp_pkg_dir; cd -; done;
Copy link
Contributor

@yugangw-msft yugangw-msft Oct 18, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if easy, please move the loop body to new lines. The current line is too long.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will do in next PR.

@derekbekoe
Copy link
Member Author

@troydai Can you take a look and I'll merge?

org.label-schema.build-date=$BUILD_DATE \
org.label-schema.vcs-url="https://github.com/Azure/azure-cli.git" \
org.label-schema.docker.cmd="docker run -v ${HOME}:/root -it azuresdk/azure-cli-python:<version>"

WORKDIR azure-cli
COPY . /azure-cli
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we remove this folder from the image after build?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The folder would only contain the source code due to the .dockerignore file we have.
Don't see harm in including it.

privates_dir = os.path.join(root_dir, 'privates')
if os.path.isdir(privates_dir) and os.listdir(privates_dir):
whl_list = ' '.join([os.path.join(privates_dir, f) for f in os.listdir(privates_dir)])
exec_command('pip install {}'.format(whl_list))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ask forgiveness instead of permission.

try:
    whl_list = ' '.join([os.path.join(private_dir, f) for f in os.listdir(privates_dir)])
    exec_command('pip install {}'.format(whl_list))
except OSError:
    pass

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay can make this change in next PR.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I looked into this and this would run 'pip install <no_files>' if privates directory exists and there are no whls in there.
I could add an if statement to check but then it kind of defeats the point I think.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants