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

Change of master branch alias to 1.4.x-dev breaks packages depending on 1.3.x-dev #313

Closed
ademarco opened this issue Sep 2, 2019 · 59 comments

Comments

@ademarco
Copy link

ademarco commented Sep 2, 2019

Problem

While creating the 1.4.0 release of this project this line was changed:

image

This resulted in broken builds for all packages that depended on behat/mink-selenium2-driver: 1.3.x-dev such as Drupal core and many others. In turn this resulted in several CI/CD builds to fail with the following message:

  Problem 1
    - webflo/drupal-core-require-dev 8.7.x-dev requires behat/mink-selenium2-driver 1.3.x-dev -> no matching package found.

Also, we cannot simply fix our composer.json files by upgrading it to 1.4.x-dev since builds that depends on current or previous versions of, say webflo/drupal-core-require-dev, have no way of fixing that dependency, other than forking this project, creating a 1.3.x-dev branch and update all their composer.json by adding a custom repository information.

Proposed solution

The proposed solution is to create an actual 1.3.x-dev branch on this project, branching off the correct commit hash, this branch will then be published on Packagist, allowing all projects to access it.

@oleg-andreyev
Copy link
Contributor

Depending on dev version is not quite correct As soon as @stof will release 1.4.0, you'll be able to use 1.4.0 as temporary solution you can use dev-master#3ab9f315e9ad63cc518ccc72910754fedd6de015

@oleg-andreyev
Copy link
Contributor

@aik099 @stof as temporary solution from our side, let's create 1.3.x-dev branch which should be branched from commit before 3ab9f31

@ademarco
Copy link
Author

ademarco commented Sep 2, 2019

Thank you @oleg-andreyev ! I realize that that's not ideal but that alias was published on Packagist and people started using it relying on the fact that, since it was published, it would not be removed.

I'd consider keeping this as a permanent solution for the issue, also in future, when 1.5.0 will be released.

@stof
Copy link
Member

stof commented Sep 2, 2019

An branch alias for master will change over time. And it will always be published (otherwise the alias is useless). The whole point of the alias is to be able to depend on ^1.3.0@dev, to get that code which will use dev-master for now, but will match future releases too (and will also match the bumped alias).
Defining exact-version constraints relying on a branch alias is very fragile, and cannot be covered by a BC policy (or we would have to never change the library anymore, once we define a branch alias for master).

Branching 1.3.x-dev from a commit which will never be part of a 1.3.x release (as this commit will be part of 1.4.0) will not happen. If we were branching 1.3.x today, it would have to be from the last published 1.3.x tag (to create a 1.3.(x+1) hotfix).

@ademarco
Copy link
Author

ademarco commented Sep 2, 2019

@stof so the correct thing to do is to branch 1.3.x-dev from 1.3.1 (commit 473a9f3) and then create 1.3.2 as an hotfix on that branch, correct?

@Dropa
Copy link

Dropa commented Sep 2, 2019

Projects created on top of https://github.com/drupal-composer/drupal-project at least explodes by this change.

Maybe just add one more alias, where 1.3.x-dev just points to 1.3.1?

Edit: assuming that latest 1.3.x-dev before update of master alias was not ahead of 1.3.1

@stof
Copy link
Member

stof commented Sep 2, 2019

well, the thing is, we don't have something to hotfix as 1.3.2, and so we don't need such 1.3.x branch.
Defining a requirement on an exact match of a dev branch alias is simply not something that webflo/drupal-core-require-dev should do, and this is the place where things should get fixed.

@ademarco
Copy link
Author

ademarco commented Sep 2, 2019

Yeah the problem is that that version constraints is in old webflo/drupal-core-require-dev tags, which are used on all sort of builds.

For what is worth this actually fixes it on the "host project", meaning the project that depends on webflo/drupal-core-require-dev:

        ...
        "webflo/drupal-core-require-dev": "~8.6",
        "behat/mink-selenium2-driver": "1.4.x-dev as 1.3.x-dev"
}

Edit: since people are linking at this comment in PRs I'm linking here the workaround we ended up using in our projects, which is slightly more refined that this one above. Check #313 (comment)

@stof
Copy link
Member

stof commented Sep 2, 2019

well, if webflo/drupal-core-require-dev fixes its requirement and releases a new 8.x version, this will fix it for your project. and it would be the right fix to do, rather than forcing another project to create additional maintenance work (and again each time in the future if this package keeps doing the same) to workaround a situation created by a package that we don't use.

@zuernBernhard
Copy link

Webflo will still be on vacation until 11.09.2019 ...

@timmillwood
Copy link

We don't depend on webflo/drupal-core-require-dev but in a similar way are depending on 1.3.x-dev in 4+ branches in around 100 repos.

A 1.3.x would really help us in the short term as we look at rolling out an update to all those repos.

@stof
Copy link
Member

stof commented Sep 2, 2019

Well, repos which use a composer.lock are not broken by this change until the time you run a composer update (at which point you can fix the constraint too)

@JeremySkinner
Copy link

We have a lot of projects broken by this because we depend on webflo/drupal-core-require-dev which we install during our CI process. @ademarco's workaround (aliasing 1.4.x-dev as 1.3.x-dev) works well, although does mean having to update every project.

It'd be good if a 1.3.x-dev alias could be recreated and pushed out again as this is going to be a problem for a lot of people.

@Boegie
Copy link

Boegie commented Sep 2, 2019

        "behat/mink-selenium2-driver": "1.4.x-dev as 1.3.x-dev"

I think we should play it a bit more safe by not using a (possibly moving) 1.4.x-dev but maybe a commit SHA that will be static in time?

I've gone with one that wasn't updated recently, since that was probably(?) used for 1.3.x-dev a long time:

    ...
    "behat/mink-selenium2-driver": "dev-master#a38512485e905738321f793b3acd3e7411a85101 as 1.3.x-dev",
    "webflo/drupal-core-require-dev": "~8.7.0"
    ...

[...] although does mean having to update every project.
It'd be good if a 1.3.x-dev alias could be recreated and pushed out again as this is going to be a problem for a lot of people.

Of course, this would be less work for a lot of people. (Not saying using a -dev tag in webflo/drupal-core-require-dev was the best plan to begin with)

@stof
Copy link
Member

stof commented Sep 2, 2019

it would be good if webflo/drupal-core-require-dev could fix their mess instead of forcing us to create our own mess to workaround it.
You are asking the Mink maintainers to fix a situation they are not responsible for at all here.
And anyway, I won't have time to do that right now (and if I had time to work on Mink right now, I would still rather work on preparing things for a next release than working on such workaround).

@JeremySkinner
Copy link

You are asking the Mink maintainers to fix a situation they are not responsible for at all here.

I appreciate that webflo/drupal-core-require-dev have done the wrong thing and should fix this, but deleting a public alias that downstream packages may depend on (even if they technically shouldn't) is analagous to deleting a release that people already depend on (and is precisly why most package managers don't allow deletion of packages once they've been published), so I do think mink does have some responsibility here.

I appreciate that you're busy though, and we do have a workaround so it's not a showstopper.

@dbosen
Copy link

dbosen commented Sep 2, 2019

tbh. I also do think, that the Mink maintainers are partially responsible for this mess. As you did not release a new version for three years, projects were more or less forced to use the -dev tag.

@oleg-andreyev
Copy link
Contributor

@JeremySkinner
While I understand this issues, it's not fault of Mink maintainers. You should use -dev on your own risk, maintainer cannot guarantee here anything.

@dbosen
It's open source, people are spending their free time on this...
If you see that package wasn't released for 3 years, maybe we as a community need to help maintainers, especial if package is heavily used in your project

@aik099
Copy link
Member

aik099 commented Sep 2, 2019

To summarize I see several solutions (in above comments) that involve changing composer.json file in projects, that use Mink. When implemented they will allow using master branch and not get all tests broken because of an alias change.

P.S.

  • As an open-source project nothing is really guaranteed except likely passing tests on Travis CI.
  • If somebody does have free time to spend on moving this project forward, then we're (as maintainers) looking forward to it.

@aik099 aik099 mentioned this issue Sep 3, 2019
@JeremySkinner
Copy link

JeremySkinner commented Sep 3, 2019

@oleg-andreyev I appreciate your point of view, but will respectfully disagree. Part of choosing to maintain and support an open source project involves understanding how the community are using that project and being empathetic even when the community perhaps starts using the project in a way that wasn’t intended. As someone who runs a reasonably popular oss project, there have been times where I’ve changed something that I believed was in the best interest of the project, that has caused pain for the community, and in those cases I always try to take time to consider the feedback and re-examine those choices if necessary. At the end of the day, our oss projects exist to help developers deliver value.

In this particular case, a large portion of the Drupal community depend on drupal-core-require-dev, probably without even realising that it had a dev dependency on mink (I know I didn’t). Yes, drupal-core-require-dev Drupal maybe did the wrong thing by depending on dev, and yes, they should fix that, but that doesn’t change the fact that choosing to remove the alias has had a real world negative impact on a lot of people. I do appreciate that OSS maintainers’ time is precious (believe me I really do understand that first-hand) and you have to prioritise, but I’d still ask that you’d consider restoring the alias as a gesture of goodwill until Drupal can be updated, as that would literally help a lot of people.

@aik099
Copy link
Member

aik099 commented Sep 3, 2019

@JeremySkinner , is this a viable solution:

  1. create protected 1.3 branch (just for this workaround) from commit prior to an alias change
  2. in 1.3 branch change 1.3.x alias from dev-master to dev-1.3

?

Theoretically (I don't do such tricks every day to be certain) that should restore missing 1.3.x alias and lock it in place to avoid any further commits coming into it.

@upchuk
Copy link

upchuk commented Sep 3, 2019

Quite a lot of people here are "blaming" webflo but in reality this issue is in Drupal core: https://github.com/drupal/drupal/blob/8.8.x/composer.json#L14 and that is what webflo copies over from require-dev to require. So the expectation that webflo should have to fix this for Drupal is not correct. It should be fixed in Drupal core.

Update: Drupal core issue: https://www.drupal.org/project/drupal/issues/3078671

@stof
Copy link
Member

stof commented Sep 3, 2019

2\. in `1.3` branch change `1.3.x` alias from `dev-master` to `dev-1.3`

this is useless. A dev-master branch alias has no effect outside the master branch.

Theoretically (I don't do such tricks every day to be certain) that should restore missing 1.3.x alias and lock it in place to avoid any further commits coming into it.

but then, are we expected to create the same mess again each time we release a minor version ? Updating the branch alias for master is a required step of releasing a minor version (as the master branch will then be the codebase for the next minor).
The other alternative would be to remove the branch alias entirely, but this would mean that it would be impossible to install a dev version to fulfill a ^1.3.1@dev requirement, which would be worse for the ecosystem than breaking cases making an exact match on a dev alias.

So the expectation that webflo should have to fix this for Drupal is not correct. It should be fixed in Drupal core.

Then ask Drupal core to fix it. I don't know this webflo thing and so I don't know where they take their constraint from.

@rodrigoaguilera
Copy link

Solutions for Drupal core are being discussed at https://www.drupal.org/project/drupal/issues/3078671

@chrfritsch
Copy link

chrfritsch commented Sep 3, 2019

Please just create a 1.4.0 tag, so drupal can update it's composer file to fix this mess.

@aik099
Copy link
Member

aik099 commented Sep 3, 2019

this is useless. A dev-master branch alias has no effect outside the master branch.

@stof , here my suggestion in more details:

  1. master branch have alias from dev-master to 1.4.x
  2. 1.3 branch have alias from dev-1.3 to 1.3.x

Since Packagist will read composer.json in all of the branches it will end up creating 2 aliases:

  • one pointing to master branch
  • another one pointing to 1.3 branch

Are you certain this won't work?

but then, are we expected to create the same mess again each time we release a minor version ?

Nope. We should mention in README, that people shouldn't depend on aliases at all. If they will, then we promise no support.

lhridley added a commit to codementality/drupal-project-build that referenced this issue Sep 3, 2019
…th revised alias

minkphp/MinkSelenium2Driver#313

Branch alias for master was changed from 1.3.x-dev to 1.4.x-dev.

This PR adds an explicit declaration for behat/mink-selenium2-driver, mapping
1.4.x-dev to 1.3.x-dev as an alias.

This is a temporary fix until Drupal Core corrects this issue (see above minkphp project
issue for more details)

On branch feature-23-mink-selenium2-explicit-dependency

Changes to be committed:
	modified:   build-project.sh
@aik099
Copy link
Member

aik099 commented Sep 4, 2019

I'm not sure if it's safe to create branch named exactly as alias in composer.json. To be on the safe side I've suggested to:

  1. create 1.3 branch (without .x part)
  2. changing composer.json in that branch and creating alias named 1.3.x pointing to that branch

@stof
Copy link
Member

stof commented Sep 4, 2019

@aik099 no need for an alias. A 1.3 or 1.3.X branch already has 1.3.X-dev as version in composer.
but creating a 1.3.x branch pointing to commits that will only ever be part of a 1.4.0 release looks wrong to me.

I will look into releasing the 1.4.0 stable release in the next few days.

@ryanaslett couldn't drupal update their constraint to ^1.4.0@dev (or ~1.4.0@dev is you don't want to allow future 1.5 releases to match), which would allow resolving it to the existing master branch and would also then allow using the stable release once tagged) ? Or is there actually something blocking you in the uptodate master branch ?

@oleg-andreyev
Copy link
Contributor

oleg-andreyev commented Sep 4, 2019

@stof maybe we can adopt same approach as Symfony is using? Each minor version has it's own branch? This way we'll never have such issues again.

@aik099
Copy link
Member

aik099 commented Sep 4, 2019

... maybe we can adopt same approach as Symfony is using? Each minor version has it's own branch? This way we'll never have such issues again.

@oleg-andreyev , that approach would be painful for contributors, because they'll need to remember into which branch the PR should go into. Also maintainers needs to merge back and forth the branches. There is a possibility to change PR base branch in GitHub UI directly, but that might as well result in a conflicts down the road.

In Symfony Fabien has magical script, that allows him to move PR around different branches if contributor has submitted it into wrong branch. Git-wise it works correctly, but GitHub-wise it looks awful, because every moved PR looks Closed instead of Merged.

@stof
Copy link
Member

stof commented Sep 4, 2019

plus, it only makes sense if you actually plan to maintain minor versions in parallel (which makes sense in the case of a big project like Symfony, but not in our case. Fabien does not do that either for Twig).

@greg-1-anderson
Copy link

I have tested creating 1.3.x, and it works fine. It would be fine to create 1.3 instead. Please create one of these branches.

Drupal cannot change the constraint from 1.3.x-dev to something else. The constraint is recorded forever in every stable tag from Drupal 8.6.0 to 8.7.7 (just came out today). Anyone who tries to install an old stable version with Composer will run into problems.

It is not a problem to create a 1.3.x branch that will never have another 1.3 release on it. It is an ordinary and expected thing that, if you depend on a branch such as 1.3.x-dev that you may get commits that belong on 1.4.x, because 1.3.x-dev means the same thing as dev-master (or did, a short while ago), which means everything newer than the last stable release on the 1.3 line. I can understand your point about not wanting the 1.4.x commits on 1.3.x-dev if you were going to maintain the 1.3.x branch. However, we are not asking you to maintain the 1.3.x-dev branch; we just need it to repair the sins of the past. When 1.3.x-dev did exist, it had commits on it destined only for 1.4.x. We do not want that history rewritten.

As soon as there is a stable 1.4 release, we will move Drupal to that (similarly with a new stable 1.7 release on behat/mink). We are not looking to solve the problem of how to abuse using the minor release dev branches in the future. Drupal is going to stop doing that. We only want to solve the problem of the past, where Drupal is using 1.3.x-dev constraints in old existing stable tags.

Thank you for your understanding and help.

@ryanaslett
Copy link

@stof ^1.3@dev is what we should have done, had we realized that 1.3.x was a branch alias and not a true development branch, but the issue now is that even if we were to update our constraint, we wont be able to affect all of the existing users who currently have 1.3.x-dev in their composer.json/composer.lock files. Our experience shows us that we'll be receiving support questions on this issue for many years to come.

So,

creating a 1.3.x branch pointing to commits that will only ever be part of a 1.4.0 release

Is what we're asking for, as those commits used to appear to us to be part of 1.3.x until the branch alias changed, which effectively backdated all of the commits from 1.3.1 onwards as being on the 1.4.x line.

We definitely appreciate your help in getting this resolved.

@aik099
Copy link
Member

aik099 commented Sep 4, 2019

@greg-1-anderson , are you sure, that in #313 (comment) the commit should be 8684ee4 (some random commit IMO) and not 0a09c43 (commit just before alias was changed)
?

@greg-1-anderson
Copy link

@aik099: 8684ee4 is what we would prefer, because that is the commit that is in Drupal's composer.lock file. Other more recent commits would also work, but since this branch is being created to support broken Drupal builds, and not to serve as a maintained 1.3 branch, I think that the conservative choice is best.

If you prefer to use 0a09c43, though, that would be acceptable.

@josephdpurcell
Copy link

If anyone lands here because of this issue while upgrading Lightning 4 while using BLT please see acquia/blt#3840.

@greg-1-anderson
Copy link

Just wondering if comment 527995474 by @aik099 means there will be a 1.3 branch created here (using either 8684ee4 or 0a09c43)? @ryanaslett actually asked for the later commit. I'm indifferent, as long as we have the branch.

@ademarco
Copy link
Author

ademarco commented Sep 5, 2019

@josephdpurcell this is what we ended up doing in our projects' composer.json:

   "behat/mink-selenium2-driver": "dev-master#8684ee4 as 1.3.x-dev",
   "behat/mink": "dev-master#a534fe7"
},

As also behat/mink was a -dev we opted for locking that too, waiting for a release. See minkphp/Mink#786 (comment) for more info.

@aik099
Copy link
Member

aik099 commented Sep 5, 2019

I've created 1.3 branch. I can confirm, that 1.3.x-dev version now exists on Packagist: https://packagist.org/packages/behat/mink-selenium2-driver

@aik099 aik099 closed this as completed Sep 5, 2019
@greg-1-anderson
Copy link

Thanks!

@lhridley
Copy link

lhridley commented Sep 5, 2019

Thank you @aik099 !

@WarpedOne
Copy link

Awesome. Thanks so much, @aik099 !!!!!

@JeremySkinner
Copy link

That’s great, thank you for making the change 😊

@BaluErtl
Copy link

BaluErtl commented Sep 5, 2019

Thanks @aik099 and other nice people at Mink's side.

@Drupal-Infrastructure
Copy link

Thank you so much. This saves us.

@oleg-andreyev
Copy link
Contributor

@aik099 I hope it was intentionally that you've created 1.3.x after a385124 which bumped PHP to 5.4 ;)

@greg-1-anderson
Copy link

greg-1-anderson commented Sep 5, 2019

Drupal 8 requires PHP 5.5 or later, so Drupal is indifferent to the inclusion of that commit. 8684ee4 might have been better, but let's not go back now. There should not be much demand for the 1.3 branch outside of Drupal users (presuming that 1.4.0 is tagged soon).

@oleg-andreyev
Copy link
Contributor

@greg-1-anderson sure, we won't got back! I hope that we all learned a lesson and will never have this issue again.

@aik099
Copy link
Member

aik099 commented Sep 6, 2019

@aik099 I hope it was intentionally that you've created 1.3.x after a385124 which bumped PHP to 5.4 ;)

That wasn't my intension. The goal way to create release with most of the features committed, but without change where alias was renamed.

@pookmish
Copy link

pookmish commented Sep 6, 2019

I can't seem to get the 1.3.x-dev version in my CircleCi tests. Locally it works fine and I can install the correct version. In the CI/CD I continue to get no matching package found when running composer update or composer require. Is there any trick to solving this?

i've already tried composer clear-cache to no help

@snize
Copy link

snize commented Oct 22, 2019

@pookmish I had similar problem. Try this

  1. Remove composer.lock
  2. Run composer update nothing to regenerate composer.lock file

https://gist.github.com/snize/7d90fb26de07028d296881cb10a7917d

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests