phar-io / phive

The Phar Installation and Verification Environment (PHIVE)
https://phar.io
BSD 3-Clause "New" or "Revised" License
581 stars 45 forks source link

`--trust-gpg-keys` option for `update` #296

Open JakeQZ opened 3 years ago

JakeQZ commented 3 years ago

It seems this option is only available for install and not for update.

But install will install the latest version and ignore version constraints set in phive.xml, and there doesn't seem to be a way of specifying version constraints on the command line (at least I can't see anything listed when running phive without parameters to get the built-in help).

theseer commented 3 years ago

It seems this option is only available for install and not for update.

That is correct. Given that to update something it has to have been instaled first, there so far was no point in having that switch here. The only valid edge case I can see that with an update the signing key changes and one would want to automagically allow the updated key.

But install will install the latest version and ignore version constraints set in phive.xml

That should certainly not be the case: If you have an installed version in the phive.xml, that very version is (or should at least) be reinstalled. That's the exact and sole reason to have that information tracked. On update it should update to whatever is latest that still falls into the range defined by the constraint.

and there doesn't seem to be a way of specifying version constraints on the command line (at least I can't see anything listed when running phive without parameters to get the built-in help).

Looks like we're indeed lacking documentation for that. We very much do support that: phive install <package-specifier>@<constraint> (phive install phpunit@^8.0 for instance).

JakeQZ commented 3 years ago

there so far was no point in having that switch here

We're storing the .phars under version control and running the update on a build server (actually just GitHub CI actions), with the .phars copied to a tools directory within the source tree.

So the 'user' running the update won't usually be the same as the one initially installing. Also, as this is an automated process, there is no-one to press Y; we are using echo y | as a workaround but this may have security implications.

(We need to be able to run different versions of PHPUnit depending on the PHP version, because there isn't a version compatible with both PHP 7.2 and PHP 8.)

If you have an installed version in the phive.xml, that very version is (or should at least) be reinstalled.

Starting with the following entry in phive.xml:

  <phar name="phpunit" version="^7.5.20" installed="7.5.20" location="./tools/phpunit.phar" copy="true"/>

I run

phive install phpunit

The phive.xml entry is updated to the following:

  <phar name="phpunit" version="^9.4.3" installed="9.4.3" location="./tools/phpunit" copy="false"/>

The version in `tools' is not updated but a batch file (this is on Windows) is created linking to version 9.4.3 installed for the current user.

Not sure if that's what's supposed to happen. However, if I were to instead run phive update phpunit from the same starting point, no changes are made other than the creation of a batch file linking to the local copy in tools - this is as I would expect.

theseer commented 3 years ago

there so far was no point in having that switch here

We're storing the .phars under version control and running the update on a build server (actually just GitHub CI actions), with the .phars copied to a tools directory within the source tree.

So the 'user' running the update won't usually be the same as the one initially installing.

Interesting use case. Let me think about it. I'm still not convinced, as I don't see this as preferable or even viable process:

Also, as this is an automated process, there is no-one to press Y; we are using echo y | as a workaround but this may have security implications.

As your main problem is that your CI run doesn't have the required public key in its key ring because the original install occurred on a different system, you could simply fix that by having the key imported directly using gnupg. phive uses a standard gnupg with a "private" keyring in ~/.phive/gpg.

(We need to be able to run different versions of PHPUnit depending on the PHP version, because there isn't a version compatible with both PHP 7.2 and PHP 8.)

If you have an installed version in the phive.xml, that very version is (or should at least) be reinstalled.

Starting with the following entry in phive.xml:

  <phar name="phpunit" version="^7.5.20" installed="7.5.20" location="./tools/phpunit.phar" copy="true"/>

I run

phive install phpunit

The above command tells phive to install whatever is the latest version of phpunit explicitly ignoring the configured settings in phive.xml. While that works as intended, maybe we should actually change that because I guess it's unexpected behavior and inconsistent with the update command.

For now: If you merely want to re-create the state configured in phive.xml, simply use phive install without any additional parameters.

The version in `tools' is not updated but a batch file (this is on Windows) is created linking to version 9.4.3 installed for the current user.

I see why that is happening and I agree that is probably at least surprising if not wrong: It previously was installed using copy=true and now - because of your call being phive install phpunit - it falls back to it's default of creating a link / batch file. What's indeed seemingly missing is the deletion of the previous install. So I guess that qualifies as a bug ;). One that might vanish when we change the aforementioned behavior of install.

Not sure if that's what's supposed to happen. However, if I were to instead run phive update phpunit from the same starting point, no changes are made other than the creation of a batch file linking to the local copy in tools - this is as I would expect.

Yes. The later indeed seems correct.

jrfnl commented 5 months ago

The only valid edge case I can see that with an update the signing key changes and one would want to automagically allow the updated key.

I'm not sure how much of an edge case that is.

The recommendation (on phar.io) is to set a key up to be valid for a year, so basically the key for most projects will change on average once a year, which to me makes this sound like a very valid request.

theseer commented 5 months ago

Thanks for adding your opinion.

I'm not strictly against adding this option to update. I just do not see the use case where that would be helping.

The only valid edge case I can see that with an update the signing key changes and one would want to automagically allow the updated key.

I'm not sure how much of an edge case that is.

The recommendation (on phar.io) is to set a key up to be valid for a year, so basically the key for most projects will change on average once a year, which to me makes this sound like a very valid request.

We do not have a problem with changing keys. Phive actually detects that a key has changed and requests a confirmation whether or not this is intended.

The --trust-gpg-keys option is intended for CI usage, where no interactive confirmation on install (and yes, neither on update) is possible. I still fail to see why one would want to run update on CI though as the result is lost after the job completed.

If you have the tools committed along, there's no need to run phive at CI at all. If you only committed the phive.xml (or .phive/phars.xml), phive install is the way to go, potentially with the --trust-gpg-keys option in case you do not have the public keys in the keyring cached / warmed upfront to get the desired state back.

The only - and to me that is an edge case - scenario I can see where running update on CI would make sense is when you want to try to see whether updated tools are available and whether or not the build still works with these. And only when install was never run on CI before - resulting in no warmed keyring being available - would we need a means to auto-import new/changed keys.

I'm also not convinced that a change of keys should/can be treated the same as a previously unknown key. But that's a different story.

jrfnl commented 5 months ago

@theseer Thanks for your response.

We do not have a problem with changing keys. Phive actually detects that a key has changed and requests a confirmation whether or not this is intended.

Ah, that's interesting. In that case, I agree shouldn't be needed. But that makes the issue I reported in #427 all the more weird (though I am still presuming I'm doing something wrong there).

Sorry for the noise. I came across this issue while looking to see if what I was seeing in #427 had been reported before.

theseer commented 5 months ago

Sorry for the noise. I came across this issue while looking to see if what I was seeing in #427 had been reported before.

Your comments are - as far as my experience goes - never noise. And this issue is still open for the very reason I want to get more input before making a change.

I currently have hardly any time to work on open source, so I'm even more reluctant to implement things I do not understand the use case for :)

JakeQZ commented 5 months ago

reluctant to implement things I do not understand the use case for :)

We switched to not having the PHARs under version control (as you may see from the cross-referenced PRs/commits). This seems to work better in general for multiple development environments across various platforms, and we've not IIRC had any issues since.

Possibly we may have misunderstood the best practices at the time of setting up, though that was before GitHub Actions, when we were using Travis, so I can't be sure.

I think it should be stated clearly in the docs that it is not recommended to put the PHARs under version control. (Though there may be use-cases where you want that, for some specific reason.)