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

fix #7606: Include composer files in phar #8383

Conversation

mind-bending-forks
Copy link

@mind-bending-forks mind-bending-forks commented Aug 8, 2022

This change is completely untested! It will hopefully fix issue #7606 by allowing PackageVersions\Versions::getVersion to read version numbers from the composer files when using the CLI.

If successful, then:

  • composer.json and composer.lock files will be included in the phar archive when built, and
  • when running php psalm.phar -v from the command-line, the correct version number will be reported.

This will hopefully allow `PackageVersions\Versions::getVersion` to read version numbers from the composer files when using the CLI.
@AndrolGenhald
Copy link
Collaborator

There's a script in the bin directory to build the phar, which could then be tested. I'll try it tomorrow and see if this fixes it.

@AndrolGenhald AndrolGenhald added the release:fix The PR will be included in 'Fixes' section of the release notes label Aug 8, 2022
@orklah
Copy link
Collaborator

orklah commented Aug 8, 2022

the CI on PR actually generates a phar. It should be available here: https://output.circle-artifacts.com/output/job/74cf1ac8-3a6e-46bf-b155-b4054b65780d/artifacts/0/build/psalm.phar

@AndrolGenhald
Copy link
Collaborator

Where is the version actually coming from though? I assume if we test the build from this branch it'll correctly say it's the dev version, but we want to know if it'll work for an actual release.

@mind-bending-forks
Copy link
Author

the CI on PR actually generates a phar. It should be available here: https://output.circle-artifacts.com/output/job/74cf1ac8-3a6e-46bf-b155-b4054b65780d/artifacts/0/build/psalm.phar

The phar archive does contain composer.json and composer.lock files. I get

> php psalm.phar -v
Psalm 4.x-dev@2495516e7b0c2cd6b429e51d4f3dcce3cf6d821a

Not sure if that is what would be expected or not.

@orklah
Copy link
Collaborator

orklah commented Aug 8, 2022

Right! I wasn't thinking :p

@AndrolGenhald
Copy link
Collaborator

When I try on one of my projects without the phar it reports Psalm 4.24.0@06dd975cb55d36af80f242561738f16c5f58264f which I assume comes from:

        ...
        {
            "name": "vimeo/psalm",
            "version": "4.24.0",
            "source": {
                "type": "git",
                "url": "https://github.com/vimeo/psalm.git",
                "reference": "06dd975cb55d36af80f242561738f16c5f58264f"
            },
            ...

in that project's composer.lock. Are we sure Psalm's composer.lock matters for this? If so, do we have to change/add a version number in composer.json or composer.lock before building the phar?

@mind-bending-forks
Copy link
Author

mind-bending-forks commented Aug 8, 2022

Apologies if I'm not following or getting confused here. If someone is loading Psalm using a phar archive to check a project (rather than composer), then isn't it the case that there is no guarantee that that project uses composer at all, and even if it does, it would possibly not be referencing Psalm or the version of it that is within the phar archive? Therefore, the version number of psalm in the phar archive needs to be defined within the phar archive itself. It cannot be determined from something outside. If Psalm is attempting to read a version number from a composer.lock file located outside the phar archive itself, then that's a bug?

@AndrolGenhald
Copy link
Collaborator

@mind-bending-forks correct. I'm just trying to figure out where the version number is coming from, and knowing where it comes from without the phar might help know where it comes from for the phar. I figure the version number for the phar probably works similarly to what happens if you run ./psalm on this repository, so maybe I'll try debugging that tomorrow.

I think what might need to happen is that we'll have to add a step to the release process to set the version number for the phar somehow, and I'm not sure if composer.lock or composer.json actually need included in the phar.

@weirdan
Copy link
Collaborator

weirdan commented Nov 11, 2022

The version number comes from various files in vendor/composer directory. If Psalm was installed (during the build, for phar variant) using a recent version of composer, the data comes from vendor/composer/installed.php.

@weirdan
Copy link
Collaborator

weirdan commented Nov 27, 2022

4.x branch is closed now as we prepare for the 5.0 release. Please target the master branch instead.

@orklah
Copy link
Collaborator

orklah commented Dec 20, 2022

Closing as this targets 4.x which is closed. Please reopen on master going forward

@orklah orklah closed this Dec 20, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release:fix The PR will be included in 'Fixes' section of the release notes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants