Skip to content

[Packaging] Fix #22741: az upgrade: This command becomes non-blocking on Windows #26464

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.

Sign up for GitHub

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

Merged
bebound merged 12 commits into from
Oct 18, 2023
Merged

[Packaging] Fix #22741: az upgrade: This command becomes non-blocking on Windows #26464

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
a7d5d28
Init
bebound May 10, 2023
5bbb874
Minor fix
bebound May 17, 2023
fd1c702
Update src/azure-cli/azure/cli/command_modules/util/custom.py
bebound May 19, 2023
14a80a5
Minor fix
bebound May 19, 2023
11f4647
Add instruction
bebound May 19, 2023
89f4f78
Minor fix
bebound May 19, 2023
b049e8b
Minor fix
bebound May 19, 2023
a65cb46
Minor fix
bebound May 19, 2023
527123a
Merge branch 'dev' into fix-upgrade
bebound Sep 13, 2023
b2509e6
Merge remote-tracking branch 'upstream/dev' into fix-upgrade
bebound Sep 28, 2023
c8bb0f3
Save MSI to ~\AppData\Local\Temp\azure-cli-msi
bebound Sep 28, 2023
ffcd51d
Remove pathlib
bebound Oct 16, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion build_scripts/windows/scripts/build.cmd
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ if "%CLI_VERSION%"=="" (
echo Please set the CLI_VERSION environment variable, e.g. 2.0.13
goto ERROR
)
set PYTHON_VERSION=3.10.10
set PYTHON_VERSION=3.10.8

set WIX_DOWNLOAD_URL="https://azurecliprod.blob.core.windows.net/msi/wix310-binaries-mirror.zip"
set PYTHON_DOWNLOAD_URL="https://www.python.org/ftp/python/%PYTHON_VERSION%/python-%PYTHON_VERSION%-embed-win32.zip"
Expand Down
7 changes: 5 additions & 2 deletions src/azure-cli/azure/cli/command_modules/util/custom.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ def upgrade_version(cmd, update_all=None, yes=None): # pylint: disable=too-many
from azure.cli.core.util import get_latest_from_github
try:
latest_version = get_latest_from_github()
local_version = '2.45.0'
if latest_version and parse(latest_version) <= parse(local_version):
logger.warning("You already have the latest azure-cli version: %s", local_version)
update_cli = False
Expand Down Expand Up @@ -187,10 +188,12 @@ def _upgrade_on_windows():
"""
logger.warning("Updating Azure CLI with MSI from https://aka.ms/installazurecliwindows")
tmp_dir, msi_path = _download_from_url('https://aka.ms/installazurecliwindows')

logger.warning("Installing MSI")
import subprocess
exit_code = subprocess.call(['msiexec.exe', '/i', msi_path])
subprocess.Popen(['msiexec.exe', '/i', msi_path])
logger.warning("Installation started, please wait for a few minutes.")
Comment thread
bebound marked this conversation as resolved.
Outdated
import sys
sys.exit(0)

@jiasli jiasli May 18, 2023 •
edited
Loading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

As stated in the PR description, this makes us be unable to tell if the upgrade is successful. Better to confirm with PM whether this is an acceptable solution. Otherwise, we can develop some "upgrader.exe" written in C.

@bebound bebound May 19, 2023 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

We can create a upgrader to update az to prevent running az, but we need to call az to map az upgrade command to that upgrader.exe. We still need to keep az running to retrieve the upgrade status, and we face this issue again.

az upgrade is very complicated. It downloads MSI, waits for installation to finish and updates extensions in one command.
I can hardly come up with an idea to fix this issue without breaking change.

@jiasli jiasli May 19, 2023

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I thought about it again and feel it is indeed not possible. A GUI application can have such upgrader.exe to upgrade the product itself, but this can't be done for a console application.

Consider this invocation chain: python.exe(1) -> upgrader.exe -> python.exe(2)

  • To keep the second python.exe connected to the original terminal context, every executable on the invocation chain must be alive and wait for its downstream executable to exit. In order words, if python.exe(1) subprocesses upgrader.exe and exits, then after upgrader.exe upgrades python.exe, subprocesses python.exe(2) and exits, it is no longer possible to connect python.exe(2) back to the original terminal context - the output is out of order.
  • If the first python.exe is alive, upgrader.exe cannot update it.

-> A contradiction!

Consider the following code:

import subprocess

p = subprocess.Popen(['python.exe', '-c', 'import time; time.sleep(3); print("awaken")'])
print('Exiting')

Output:

PS D:\cli\testproj> python.exe main.py
Exiting
PS D:\cli\testproj> awaken
|

The first python.exe will exits and gives control back to the terminal. The second python.exe can print, yes, but it is out of order.

How to solve it? Wait for the subprocess to exit:

import subprocess

p = subprocess.Popen(['python.exe', '-c', 'import time; time.sleep(3); print("awaken")'])
p.wait()
print('Exiting')

Output:

PS D:\cli\testproj> python.exe main.py
awaken
Exiting
PS D:\cli\testproj> |

@bebound bebound May 19, 2023 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yes, exactly.

I come up an idea to break the python dependency of python.exe(1) -> upgrader.exe.
We can modify az.cmd, if the param is upgrade, call upgrader.exe, otherwise, run az.

Furthermore, we can write the upgrade logic in bat, it looks feasible. But is it worth writing dedicated logic for Windows?


if exit_code:

@jiasli jiasli May 19, 2023

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

These below lines can never be reached, right?

@bebound bebound May 19, 2023

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yes, they are not reachable. This is draft PR for test only currently.

logger.warning("Installation Failed. You may manually install %s", msi_path)
Expand Down