Core must NEVER reference a dev-master (or @dev) package.
When it does, it breaks composer_manager and any other usage of "composer update" because Composer tries to update the package but finds no matching .git directory for it.

We already fixed mink-phantomjs-driver in #2629772: Update mink-phantomjs-driver to a tagged release but missed jcalderonzumba/gastonjs, which wasn't triggering an error cause there were no new commits.
Since there's no jcalderonzumba/gastonjs release at this point, we should start by pinning the requirement to a commit, therefore resolving the bug.

CommentFileSizeAuthor
#11 2652068.8.for-8.1.patch9.38 KBcatch
#9 2652068.8.for-8.1-do-not-test.patch9.38 KBalexpott
#8 2652068.8.patch8.99 KBalexpott
#5 update-2652068-5.patch4.14 KBmglaman
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

bojanz created an issue. See original summary.

bojanz’s picture

bojanz CreditAttribution: bojanz at Centarro commented
Title: Pin jcalderonzumba/gastonjs to a commit » Update jcalderonzumba/gastonjs to a tagged release
Status: Active » Postponed

webflo points out that only a tagged release will work.

So we need to wait on https://github.com/jcalderonzumba/gastonjs/issues/18

tom friedhof’s picture

tom friedhof CreditAttribution: tom friedhof at ActiveLAMP commented

To work around this you can just delete the dependency in question out of the vendor directory. We have these two lines in our rebuild script.

rm -rf $DOCUMENT_ROOT/vendor/jcalderonzumba/gastonjs
cd $DOCUMENT_ROOT && composer drupal-update
JeroenT’s picture

JeroenT
Primary language Dutch
Location 🇧🇪
CreditAttribution: JeroenT at iO commented
Status: Postponed » Active

Issue https://github.com/jcalderonzumba/gastonjs/issues/18 is now fixed.

mglaman’s picture

mglaman
Primary language English
Location WI, USA
CreditAttribution: mglaman at Centarro commented
Status: Active » Needs review
FileSize
update-2652068-5.patch4.14 KB

Here is a patch which updates jcalderonzumba/gastonjs to v1.0.2

Berdir’s picture

Berdir
Primary language German
Location Switzerland
CreditAttribution: Berdir at MD Systems GmbH commented
Status: Needs review » Reviewed & tested by the community

Not sure about our current standards/rules on how to specify the dependency versions exactly but this seems consistent with other examples.

This would be very nice to see fixed.

saltednut’s picture

saltednut
she/her
Primary language English
Location Chicago
CreditAttribution: saltednut at Acquia commented

++ this is a pain for distro maintenance right now! :D

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott at Chapter Three commented
Status: Reviewed & tested by the community » Needs review
FileSize
2652068.8.patch8.99 KB

The patch attached in #5 generates an error after applying and running composer install...

[RuntimeException]
  The .git directory is missing from /Volumes/devdisk/dev/drupal/vendor/jcalderonzumba/gastonjs, see https://getcomposer.org/commit-deps for more information

I think this is the error the patch is trying to fix. Here's a patch that does the update AND also passes the composer install test.

I generated this by:

  1. Removing gastonjs from core/composer.json
  2. Remove the gastonjs directory
  3. Running composer update --lock
  4. Adding the correct dependency to core/composer.json
alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott at Chapter Three commented
FileSize
2652068.8.for-8.1-do-not-test.patch9.38 KB

And here's a patch for 8.1 since the patch in #8 does not apply. Generated in exactly the same way.

Berdir’s picture

Berdir
Primary language German
Location Switzerland
CreditAttribution: Berdir at MD Systems GmbH commented
Status: Needs review » Reviewed & tested by the community

Discussed and tested.

The difference is that the previous patch didn't update installed.json, so composer still assumed it had a source checkout and the old version and got confused.

The patch in #8 is the right thing to do and composer install doesn't update anything as it shouldn't, since it is already there. Sorry for not testing this better.

catch’s picture

catch
he/him
Primary language English
CreditAttribution: catch at Third and Grove commented
Version: 8.0.x-dev » 8.1.x-dev
FileSize
2652068.8.for-8.1.patch9.38 KB

Just running the 8.1 patch past the bot in case.

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 11: 2652068.8.for-8.1.patch, failed testing.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott at Chapter Three commented
Status: Needs work » Reviewed & tested by the community

@catch that's a bug - you can't change version and submit a patch to test on that branch at the same time. I pressed retest :)

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott at Chapter Three commented

Created #2667338: Update file's version for testing when issue's version is selected to address the d.o testing bug...

  • catch committed 3c4b6ab on 8.1.x 
    Issue #2652068 by alexpott, mglaman: Update jcalderonzumba/gastonjs to a...

  • catch committed 5122398 on 8.0.x 
    Issue #2652068 by alexpott, mglaman: Update jcalderonzumba/gastonjs to a...
catch’s picture

catch
he/him
Primary language English
CreditAttribution: catch at Third and Grove commented
Status: Reviewed & tested by the community » Fixed

Committed/pushed separately to 8.1.x and 8.0.x.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.