-
Notifications
You must be signed in to change notification settings - Fork 3k
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
Conversation
derekbekoe
commented
Oct 18, 2017
- 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
@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
87660c6
to
9045ae4
Compare
@@ -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} |
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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 | ||
|
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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; |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
@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 |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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)) |
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.