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

VersionGuesser not playing well with monorepositories #8477

Closed
Toflar opened this issue Dec 10, 2019 · 11 comments
Closed

VersionGuesser not playing well with monorepositories #8477

Toflar opened this issue Dec 10, 2019 · 11 comments
Labels
Bug
Milestone

Comments

@Toflar
Copy link
Contributor

@Toflar Toflar commented Dec 10, 2019

I was really ripping my hair out on this one so I had to debug the reason why it's not working and found out some interesting stuff :-)

So what I'm trying to do is using a path repository so it's symlinked into my project which allows me to easily develop my stuff within a running application:

    "repositories": [
        {
            "type": "path",
            "url": "/......./project"
        }
    ]

Now what I noticed is that even though I made sure I checked out the my feature1 branch on my project, composer did not update to my dev-feature1 branch but instead downloaded dev-master from packagist.

I went down the rabbit hole and found the reason to reside in this check: https://github.com/composer/composer/blob/master/src/Composer/Package/Version/VersionGuesser.php#L210

So my composer.json contains a few self.version and that's why my dev-feature1 branch is skipped because it's treated as "cannot work".
However, I'm using a monorepository and my self.version hints actually come from this:

    "replace": {
        "bundle1": "self.version",
        "bundle2": "self.version",
        "bundle3": "self.version"
    },

All of those bundles, however, are part of my repository. That means I cannot checkout a feature branch and pull that into my project when I'm using a typical monorepository setup.

So I wonder:

  1. Are the replacements even relevant there?
  2. If they are, would you accept something like monorepository-dependencies or something (similar to non-feature-branches) so that self.version is only a problem if it's not one of those?
  3. Or any other idea?
@aschempp

This comment has been minimized.

Copy link
Contributor

@aschempp aschempp commented Dec 11, 2019

maybe the self.version check should only be on the requirements and not on everything like the provides?

@alcohol alcohol added the Support label Dec 12, 2019
@Seldaek

This comment has been minimized.

Copy link
Member

@Seldaek Seldaek commented Dec 22, 2019

@aschempp I don't understand what you are suggesting. If we see self.version we have to replace it by something to make it usable, so if not the actual version what would you suggest?

@Toflar I'm not sure about your suggestion, seems overly complex and something nobody will ever use aside from you :) Defining "version":"dev-master" in the project composer.json should help to avoid version guessing going for dev-feature1.. But that may not be practical if you have to change this all the time. Have you tried https://github.com/franzliedke/studio ? Maybe it'd be working out better than path repo for your use case?

@Toflar

This comment has been minimized.

Copy link
Contributor Author

@Toflar Toflar commented Dec 23, 2019

But that may not be practical if you have to change this all the time. Have you tried https://github.com/franzliedke/studio ? Maybe it'd be working out better than path repo for your use case?

I haven't, thanks for the hint. But I don't really feel like I should need any extra plugin.
Why exactly are feature branches skipped if there's self.version in the composer.json? I mean, it just falls back to dev-master in that case which is still a branch that also contains self.version in its composer.json so I don't get the difference 😄

@aschempp

This comment has been minimized.

Copy link
Contributor

@aschempp aschempp commented Jan 4, 2020

I don't understand what you are suggesting. If we see self.version we have to replace it by something to make it usable, so if not the actual version what would you suggest?

Its just a quick guess, but I found it confusing that Composer is doing a string match, and not an exact lookup. The self.version could be anywhere in extra or require-dev, and not actually affect the current resolving process…

@vtsykun

This comment has been minimized.

Copy link

@vtsykun vtsykun commented Jan 4, 2020

I had a similar problem, I tried to add a new option for the path repository to disable the VersionGuesser and specify the package versions manually, here #6578 created PR for this

@Seldaek

This comment has been minimized.

Copy link
Member

@Seldaek Seldaek commented Jan 13, 2020

Just so I can try and repro this.. What are you requiring? dev-feature1 of your package in the path repo? Can I get a repro case with public repos if possible?

@Toflar

This comment has been minimized.

Copy link
Contributor Author

@Toflar Toflar commented Jan 14, 2020

  1. mkdir test
  2. cd test
  3. mkdir mono-repo
  4. cd mono-repo
  5. git clone git@github.com:contao/contao.git .
  6. git checkout -b my-feature
  7. cd ..
  8. Create a composer.json with the following contents:
{
    "require": {
        "contao/contao": "dev-my-feature"
    },
    "repositories": [
        {
            "type": "path",
            "url": "./mono-repo"
        }
    ]
}
  1. composer update.

You'll notice that Composer will tell you it could not find dev-my-feature although we created that branch in our mono repository before.
This is because of VersionGuesser::guessFeatureVersion(). In fact for our monorepo, both conditions (the branch-alias and the self.version test) would evaluate to true. So I'd need some way to tell composer that I really want that dev-my-feature branch because I know it's all good for that monorepo 😄

@Seldaek

This comment has been minimized.

Copy link
Member

@Seldaek Seldaek commented Jan 14, 2020

Yup ok so now I am fairly sure this is related/duplicate of #8498 - for which a fix is incoming soon. Will ping you when it's pushed so you can try and confirm.

@Seldaek Seldaek closed this in a2dadb9 Jan 14, 2020
@Seldaek

This comment has been minimized.

Copy link
Member

@Seldaek Seldaek commented Jan 14, 2020

Ok please try with latest snapshot, should be creating two versions now for dev-my-feature and dev-master or whichever the my-feature branch came from. No need for extra config field 👍

@bytehead

This comment has been minimized.

Copy link

@bytehead bytehead commented Jan 14, 2020

Thank you for fixing! ❤️

@Toflar

This comment has been minimized.

Copy link
Contributor Author

@Toflar Toflar commented Jan 14, 2020

Yeah, that indeed solves the issue! Thank you!! ❤️

@Seldaek Seldaek added Bug and removed Support labels Jan 14, 2020
@Seldaek Seldaek added this to the 1.9 milestone Jan 14, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
6 participants
You can’t perform that action at this time.