Composer: Do not include #sha if the source.reference is unkown

[florisluiten]

When a package is installed that does not provide any files, the composer.lock file does not include the source.reference attribute. For example, the Roave/SecurityAdvisories package is a depency-blocker that only provides meta-data.

However, the composer.lock that is generated still append the hash as a seperator plus an empty commit-sha. This results in an incorrect packagename. The version-check then fails, but this failure is not picked up and thus it seems everything is up-to-date.

By checking the commit-sha before appending the hash, the generated composer.lock is no longer invalid and solves this problem.

[jurre]

Thanks @florisluiten, your explanation and change make sense to me. Could you add a test-case that verifies this behavior? <3

[xlgmokha]

If you're able to provide an example php project fixture that can be used to reproduce this issue then I might be able to help with adding some tests. I hope this helps.

[jeffwidman]

gentle nudge @florisluiten, any chance of tests or even just an example php repo to use as a test fixture? See ☝️

[deivid-rodriguez]

I tried using Roave/SecurityAdvisories in a composer.json file and the composer.lock file did include source.reference, so maybe this is something that has been fixed in composer itself?

[jeffwidman]

I checked with @deivid-rodriguez and he tested this using native composer update... we need to check it with dependabot itself just to ensure that our wrapper around composer functionality isn't causing the breakage.

The neat thing which Deivid helped me realize is that we don't need a full-blown PHP app (unlike go mod tidy) an example composer.json file should be enough for testing the generated composer.lock output.

[florisluiten]

Hi all,

sorry for the late reaction, it seems this has slipped through the cracks of my TODO list.

I'm no longer using dependabot, so creating a test-case is not going to be easy. But an example composer.json would look something like this:

{
    "name": "floris/dependabot-core",
    "authors": [
        {
            "name": "Floris Luiten",
            "email": "[email protected]"
        }
    ],
    "require-dev": {
        "roave/security-advisories": "dev-latest"
    }
}

Does this help?

[deivid-rodriguez]

I think that's the kind of manifest that I tried. What I don't understand is that if you're not checking a composer.lock file into version control, the Dependabot should not generate one either, so no issues there. And if you're checking a composer.lock file, composer generates a proper source.reference field. So, is the issue that dependabot is actually removing this field?

[florisluiten]

[...] composer.lock file, composer generates a proper source.reference field[...]

IIRC the source.reference field was generated, but with an empty commit_sha and resulted in an incorrect name that ended in ##; which was not a a valid reference at the time. Maybe something changed in the meantime (2 years is a long time) that causes the issue to no longer appear. If that's the case, feel free to close this PR :)

[deivid-rodriguez]

I think it's worth checking different versions of composer then. We still support v1, so if that's the previous behavior due to a bug in composer or whatever, this PR could be worth including.