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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Action erroring out on with incorrect invalid composer.json report #206

Closed
jrfnl opened this issue Dec 25, 2021 · 3 comments 路 Fixed by #213
Closed

Action erroring out on with incorrect invalid composer.json report #206

jrfnl opened this issue Dec 25, 2021 · 3 comments 路 Fixed by #213
Labels
bug Something isn't working

Comments

@jrfnl
Copy link
Contributor

jrfnl commented Dec 25, 2021

Description

Okay, so this is a "fun" one 馃槙. If I make a simple --no-update change ahead of running the composer-install action, it will error out saying the composer.json file is invalid, while in actual fact the composer.json file is perfectly valid, but the (committed) lock file is out of date (which may be completely intentional).

Making a --no-update change ahead of running the composer-install action is a common way to ensure the caching done via composer-install is used optimally, while still being able to make select changes depending on the PHP version the workflow is being run on.

Steps to reproduce

  1. On a repo with a committed composer.lock file
  2. Run a composer .... --no-update command or other Composer command which won't run update, like composer config --unset platform.php
  3. Try to run a composer update via the action and see it fail.
  4. Run composer validate --strict: it will indicate the composer.json is valid, but the lock file is out of date.
    ./composer.json is valid but your composer.lock has some errors
    # Lock file errors
    - The lock file is not up to date with the latest changes in composer.json, it is recommended that you run `composer update` or `composer update <package name>`.
    

Example code to reproduce the issue: https://github.com/jrfnl/composer-install-action-bug

Action run showing the bug: https://github.com/jrfnl/composer-install-action-bug/actions/runs/1620893221

Expected behavior

That the composer-install action doesn't error out on the lock file being out of date as this may be wholly intentional, especially if a (selective) composer update is being run via the composer-install action.

Environment details

  • version of this package: 2.0.2
  • PHP version: not relevant
  • OS: not relevant
@jrfnl jrfnl added the bug Something isn't working label Dec 25, 2021
@jrfnl
Copy link
Contributor Author

jrfnl commented Dec 25, 2021

FYI: I've got a fix lined up for this, as well as the test for it, I just can't get the test to be recognized by expect.

Unfortunately, I'm out of luck with the (otherwise very informative) CONTRIBUTING doc, as it presumes dev on a *nix based machine, while I'm on Windows, so autoexpect isn't available (the last Windows release for coreutils is 5.3.0).

How would you like me to proceed ?

@ramsey
Copy link
Owner

ramsey commented Dec 28, 2021

FYI: I've got a fix lined up for this, as well as the test for it, I just can't get the test to be recognized by expect.

Go ahead and push your fix and open the PR for it. I can help with the expect tests.

@jrfnl
Copy link
Contributor Author

jrfnl commented Dec 28, 2021

FYI: I've got a fix lined up for this, as well as the test for it, I just can't get the test to be recognized by expect.

Go ahead and push your fix and open the PR for it. I can help with the expect tests.

Pulled as draft in #213

ramsey pushed a commit that referenced this issue Dec 28, 2021
As far as I can see, the `--no-check-lock` argument has been available since Composer 1.0, so adding this argument to the `composer validate` command should be safe and should prevent the issues as reported in #206.

Includes tests (though the test isn't running properly yet).

Fixes #206

Co-authored-by: jrfnl <jrfnl@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants