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

CLI tool does not update when using Windows #1147

Open
mattc-cogapp opened this issue Aug 11, 2022 · 7 comments
Open

CLI tool does not update when using Windows #1147

mattc-cogapp opened this issue Aug 11, 2022 · 7 comments

Comments

@mattc-cogapp
Copy link

We run automated scripts on a Windows machine to populate a database and then switch environment variables to switch between them (a sort of cursory green/blue style workflow). Unfortunately, the build scripts have to be run on Windows due to a particular tool we use that does not run very well at the moment on Linux machines.

As part of these scripts, we run the Platform.sh CLI commands to update environment variables. This was working well for a while, but now when we get prompted to update the CLI if a new verison is available, Windows reports the CLI as having been updated - but this is not actually the case.

When running in verbose mode, we can see that there's an access denied error:

C:\Users\cogapp>platform self:update -vvv
...
Update to version 3.81.0? [Y/n] y
Updating to version 3.81.0
Deprecated: version_compare(): Passing null to parameter [#1](https://support.platform.sh/hc/requests/1) ($version1) of type string is deprecated in phar://C:/Users/cogapp/.platformsh/bin/platform/vendor/padraic/phar-updater/src/VersionParser.php on line 165
Warning: rename(C:\Users\cogapp\.platformsh\bin/platform.phar.temp,C:\Users\cogapp\.platformsh\bin\platform): Access is denied (code: 5) in phar://C:/Users/cogapp/.platformsh/bin/platform/vendor/padraic/phar-updater/src/Updater.php on line 382
The Platform.sh CLI has been successfully updated to version 3.81.0

C:\Users\cogapp>platform --version
Platform.sh CLI 3.80.3

However the message afterwards is misleading, as the CLI tool has not actually been updated.

I suppose this boils down to two things:

  1. Should this work under Windows? If so - then the above is a legitimate error (at least for us).
  2. Should the error message say something else in the event that an error occurred during an update?

We're currently running PHP 8.1.7 on an AWS Windows machine (I believe it's one of the old Enterprise LTS Windows versions - 20H2 or something like that)

@benr-cogapp
Copy link

Actually I'd suggest four things. If in fact this should (1) run under Windows then:

(2) The error on line 382 is reported (in -vvv verbose mode) as a warning (and therefore nor normally shown to the user, and not treated as a failure to update); instead it should be handled as an error, and the update process should report the error, instead of reporting that the CLI has been succesfully updated.

(3) This error shouldn't occur! The observed behaviour is that after running the update command .platformsh\bin contains two files, platform.phar.tempand platform. Swapping these does indeed render the CLI updated. I don't know whether the code attempts to rename the current platform out of the way, before issuing the rename command to put the new version in place; and that after failing to do so, reverts the previous rename. Or whether it attempts to use rename to overwrite the previous file. Some possibilities:

  • if the latter, perhaps that does work on some platforms, but not on (this flavour of) Windows
  • I notice that the paths passed to rename contain a mixture of Linux and Windows (fore and back) slashes - perhaps this is the problem
  • it may indeed be a permissions problem, although all the files seem to have the same properties.

(4) A final issue is that although we invoke the command platform self:update --yes --timeout=120 at the start of the daily script, in an attempt to ward off the problem by avoiding a check for two hours, it has no apparent effect. If there is an update, the update process of course fails; and the next invocation of platform ..., just a minute later, produces another request to consider updating.

@pjcdawkins
Copy link
Collaborator

Thank you both for the detailed descriptions.

I agree it looks like the key is that the failure to rename is not properly handled, and that needs to be fixed.

But... I wouldn't expect to run self:update in a script. Are you doing that to avoid the update check? I would advise instead setting the environment variable PLATFORMSH_CLI_NO_INTERACTION to 1, e.g. (bash/POSIX):

export PLATFORMSH_CLI_NO_INTERACTION=1

which removes the chance of any blocking questions, and also disables update checking.

PS. The mix of back- and forward-slashes is ugly but it should be handled fine by PHP.

@benr-cogapp
Copy link

Hi Patrick,

Thanks for your response.

The process is using platform variable:get to get the value of an environment variable; and the problem is that when there's an update to the CLI available, it interprets the information about the update as the value of the variable.

(I'm almost certain that we experimented with using --yes to avoid this, without success; I know we did on some other commands; but I can't now be sure whether we tried it on variable:get. Of course, we can only experiment in this area when there is an update available and it hasn't yet been applied!)

Hence, we tried having the batch script that kicks off the process first do a self:update to avoid this problem.

However, we'll now try PLATFORMSH_CLI_NO_INTERACTION which I wasn't aware of - thanks for the tip!

@benr-cogapp
Copy link

Meanwhile, please let me know if there's some way we can help you debug the underlying issue with updating the CLI.

@benr-cogapp
Copy link

Hi Patrick - now that there's been an update allowing us to test it - I can confirm that using PLATFORMSH_CLI_NO_INTERACTION has solved our main problem. Thanks very much for this,

It remains the case (as of 3.80.0 -> 3.81.1) that the updater fails on Windows.

@pjcdawkins
Copy link
Collaborator

Thanks for the update. There are 2 things remaining: debugging the permissions issue (to fix Windows update) and also patching or replacing the now-abandoned updater library (to fix the error message).

So maybe let's focus on this to start with

Warning: rename(C:\Users\username\.platformsh\bin/platform.phar.temp,C:\Users\username\.platformsh\bin\platform): Access is denied (code: 5) in phar://C:/Users/username/.platformsh/bin/platform/vendor/padraic/phar-updater/src/Updater.php on line 382

It might be related to this note in the PHP docs:

Note: On Windows, if to already exists, it must be writable. Otherwise rename() fails and issues E_WARNING.

Is the existing platform (phar file) itself writable?

@benr-cogapp
Copy link

Hi Patrick

is the existing platform (phar file) itself writable?

Yes it is.

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

No branches or pull requests

3 participants