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

Feature request: --is-install-needed #3888

Closed
ross-p opened this issue Mar 29, 2015 · 15 comments
Closed

Feature request: --is-install-needed #3888

ross-p opened this issue Mar 29, 2015 · 15 comments
Labels
Milestone

Comments

@ross-p
Copy link

ross-p commented Mar 29, 2015

It's terribly difficult to reliably determine in a programmatic manner if composer install actually needs to be executed.

AFAICT the only way is to parse output. And yet composer just changed it so that nothing gets written to stdout, it all goes to stderr instead. And thus is perfectly illustrated a reason why parsing output is a terrible way to do anything that should be automated.

Thus I request a feature composer --is-install-needed which can be used thus:

if composer --is-install-needed --no-dev; then
  composer install --no-dev --optimize-autoloader
fi

The problem right now is that --optimize-autoloader, which should always be used in production, always rewrites the autoload.php, which essentially causes potential downtime every time you run composer install, even if nothing was actually changed.

There exists composer --dry-run but this is not adequate for programmatic interpretation of whether or not something will actually be done, particularly given the recent history of composer changing its output in backwards-incompatible ways.

@fprochazka
Copy link
Contributor

This might allow for effective caching on Travis, because it wouldn't regenerate the autoload.php every time.

Tho, something like

composer install --no-dev --optimize-autoloader --install-only-if-changed

might be nicer

@jakoch
Copy link
Contributor

jakoch commented Mar 30, 2015

Ok, the first problem is the autoload dumping.
You might get around that, by using composer install --no-dev --no-autoloader --dry-run.
The --no-autoloader option skips writing the autoloader.
That allows to use the command in a condition without altering the project.
You can then grep the string Nothing to install or update.
If the string is not found, then there is a need to do an install: composer install --no-dev --optimize-autoloader.

That leaves the second problem (the feature request): you still have to grep, because there is no direct Composer command.

What about changing the default behavior of composer install --no-dev --optimize-autoloader to only write the autoloader if changes occurred? Maybe i'm mistaken, but it doesn't make sense to write and optimize the loading files, if nothing changed.

@ross-p
Copy link
Author

ross-p commented Mar 30, 2015

@jakoch RE adding the --no-autoloader switch to the --dry-run command, that looks like a good thing.

However that still requires that we grep the output of composer to determine if install needs to be run or not.

And as recently as 4-6 weeks ago, composer moved all output from STDOUT to STDERR, and so it effectively broke all code that was trying to grep. (That is why this became an issue).

Furthermore, composer SHOULD be able to change its output style or content whenever it wants. No code should ever expect it to output in a certain format.

But for that to work, we need a way to get info like "does install need to be run" without having to grep for output.

Thus I maintain that some switch like --is-install-needed will give composer the flexibility that it obviously desires (and indeed, expects to have, given that it exercised this recently) with respect to having freedom to change its own output.

What about changing the default behavior of composer install --no-dev --optimize-autoloader to only write the autoloader if changes occurred? Maybe i'm mistaken, but it doesn't make sense to write and optimize the loading files, if nothing changed.

While I agree with you that it seems odd for composer to update the autoload file even when it hasn't made any changes, for all I know the purpose of --optimize-autoloader is to enforce optimization whether or not the code has changed. It seems that composer expects that perhaps you've been using it without an optimized autoloader and then at some point you want to optimize the autoloader even though you haven't changed code. That is how it acts, anyway.

I fear that to change the existing behavior would break any automated workflows that rely on --optimize-autoloader modifying autoload.php every time.

Here again, the --is-install-needed switch would allow people to only optimize if something is going to be updated, and to take no action at all if nothing is to be updated.

@ross-p
Copy link
Author

ross-p commented Apr 1, 2015

@fprochazka I like the --install-only-if-changed suggestion, you're right that might be a better solution. It allows for 1 command to be executed rather than 2, and achieves the same result.

@carnage
Copy link

carnage commented Apr 17, 2015

Perhaps the better solution would be to atomically rewrite autoload.php or not rewrite it if it hasn't changed

@ross-p
Copy link
Author

ross-p commented Apr 17, 2015

@carnage while atomic file writes are never a bad thing, I disagree that it would be a better solution to do that instead of adding a switch.

Any time you write the file, you are wasting CPU and I/O on the production system. Once, to write the file. And more importantly, again because you have invalidated the opcode cache and so during a production run of the software it needs to recompute that cache. That's just silly since the contents of the file have not changed at all.

So +1 for the idea of atomic file writes, but that could be done in addition to the adding of a flag like --install-only-if-changed and not in lieu of it.

@ross-p
Copy link
Author

ross-p commented Apr 17, 2015

@Seldaek Any thoughts on this issue? It boils down to this: Do you force apps to grep your output, and then react every time you change it (as you have done recently, and likely will again), or do you add a new switch of your choice to facilitate automation?

@carnage
Copy link

carnage commented Apr 17, 2015

@ross-p if we are wanting to support automation better, a git style --porcelain flag for all commands (or a relevent subset) might be best, this gives an output which is suitable for machine parsing.

Atomic file writes should probably be a separate feature request

@jakoch
Copy link
Contributor

jakoch commented Apr 17, 2015

Just passing by... Carnage, i think you are spot on with the --porcelain idea.
Based on that i throw in [-m, --machine] Show in a format designed for machine consumption..
Now that stderr and stdout handling landed, the suggestion to provide even more output stability and support machine parsing sounds good.

@stof
Copy link
Contributor

stof commented Apr 18, 2015

Note that composer install itself is far from being atomic. So if your goal is to do a zero-downtime deployment, you would need a more complex process than running composer on the live website.

@sbuzonas
Copy link
Contributor

In regards to the --porcelain switch git uses... isn't the distinction between porcelain and plumbing that they are different levels of operations, plumbing being the lower level?

Isn't the --xml switch machine readable? The only commands I can think of that support it are the symfony standard ones.

The output changes were to be more "unixy" I believe, to allow for output to be grepped. The Nothing to install or update line probably should have remained in STDOUT because that isn't extra information IMO... it's output of the state of an install.

Would it be less "unixy" to non-zero exit on a dry-run if it needed to do something to complete an install?

@ross-p
Copy link
Author

ross-p commented Apr 18, 2015

@carnage @jakoch you guys are right, machine-formatted output would be a valid alternate solution to this problem.

@stof The reason this came up is to ensure that composer install only actually does something if something needs to be done. Instead of doing unnecessary stuff like rewriting the autoloader to the exact same value every time it runs, which it currently does. You're right when it runs and it actually does stuff, it's not atomic, and that's fine. Nobody in a production environment should be having it install stuff out from under a running application anyway; it should be done in a staging area or some similar method and moved into place atomically. However, I think it's reasonable to need to be able to check if composer install is needed to be run without having it rewrite files and without forcing a machine to read variable human output.

@stof
Copy link
Contributor

stof commented Jun 7, 2015

@slbmeh no it would not. A dry-run telling you that something will be done is not an error IMO. An error would be a dry-run telling you that deps cannot be resolved anymore.

@nevvermind
Copy link
Contributor

@slbmeh - re "porcelain": what you said is indeed the main distinction, but it seems some git commands still have such params - git help status:

       --porcelain
           Give the output in an easy-to-parse format for scripts. This is
           similar to the short output, but will remain stable across Git
           versions and regardless of user configuration. See below for
           details.

Vagrant has --machine-readable on every command as well.

Their main point being that they're BC-friendly.

@Seldaek Seldaek added this to the Nice To Have milestone Feb 27, 2016
@Seldaek
Copy link
Member

Seldaek commented Oct 26, 2020

IMO this is just too fringe use case, so that using grep is ok. composer install --no-autoloader 2>&1 | grep 'Nothing to install' for example, then if that fails to match run composer dump-autoload -o

@Seldaek Seldaek closed this as completed Oct 26, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

8 participants