Fixed self-update command with non-sha1 version argument

[mickaelandrieu]

Hi,
I have updated this command that have a weird behavior when we use non corrects arguments:

I think the if statment shouldn't check for 2 conditions but only one on the validity of the hash, because the check between actual updated one and latest sha1 version is already done later.

@Seldaek does it solve the issue ?

Also, How to realy test it ? This command is not loaded when we are outside of "Phar" context.

[stof]

the condition you removed is not about comparing to the current version. It is about comparing to the latest version, to avoid displaying such message when the user has not passed the argument.

and the condition you added is wrong. It prevents using an explicit version, while it is the only valid case (passing a sha1 hash does not work unless you pass the hash of the latest version, in which case it is useless to pass it)

[mickaelandrieu]

@stof this does not make sense (to me) to make a preg_match on a sha1 hash if the only accepted value is the latest (that we already know without any hash).

2 valid solutions:

• we allow any valid hash
• we disallow hashes

So if a version is provided and is a valid hash => DL, if not => error.
If no version argument => DL of latest.

Do you agree?

[sbuzonas]

@mickaelandrieu to test you can compile the phar with the script in the bin dir. You need to ensure that phar.readonly is off in your php.ini when you run it.

In regards to the remark on the hash verification. There are not builds for specific hashes only the latest. Updating by specific version is useful for build servers that use a tested composer version. I do not want to blindly update my build machine to the latest version of composer. At this time we whitelist the version it's allowed to upgrade to. Allow the error to occur, and send out a notification to QA when the version no longer matches.

[alcohol]

The only valid update targets are the latest version (sha), or an existing tag/release. hello-world is not a sha, so Composer assumes you specified a valid tag/release. It then fails to download it because it actually does not exist. There is no need to add further validation here in my opinion. The error is clear enough.

[mickaelandrieu]

Humm... after reading your comments and have better knowledges on how composer selfupdate works. I agree with @alcohol (you, again :) ) and think it's a funny edge that don't need any update.

Thank you all, closed ;)

[Seldaek]

For the record what I meant on twitter was that if the version arg's value is update we could just make it null which would fix the typo case of composer self update. I doubt it's such a common typo though so not sure it's worth doing.

[mickaelandrieu]

@Seldaek agree.