Problem/Motivation

See discussion in https://drupal.slack.com/archives/CGKLP028K/p1724653387328039

CI_PROJECT_ID differs for core test runs depending on whether you're a core committer or not, so we can't use it to determine the target to fetch artifacts from.

Steps to reproduce

Proposed resolution

Remaining tasks

User interface changes

Introduced terminology

API changes

Data model changes

Release notes snippet

Issue fork drupal-3470641

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

Comments

catch created an issue. See original summary.

catch’s picture

Status: Active » Needs review

catch’s picture

Title: Hardcode the core project ID for fetching artificats from the gitlab API » Hardcode the core project ID for fetching artifacts from the gitlab API

mstrelan made their first commit to this issue’s fork.

catch’s picture

Title: Hardcode the core project ID for fetching artifacts from the gitlab API » gitlab artifact caching doesn't work due to differences with project ID and build directories

As well as the project ID being different, we're also running into both the yarn lint and phpstan caches using absolute paths.

Tried a rewrite as part of the pipeline process and it partially helps (i.e. phpstan fails to use the cache, but gives less criteria it failed by), but not enough:

https://git.drupalcode.org/issue/drupal-3470641/-/jobs/2585012

https://git.drupalcode.org/issue/drupal-3470641/-/jobs/2585008

Possible workaround mentioned here is to use bind mounts:

https://github.com/phpstan/phpstan/issues/8599#issuecomment-2059227168

e.g. we'd bind mount the current directory to /tmp/build, cd, do the composer install + run the linting jobs there both for the cache warming job and for the individual linting jobs - that way everything runs in the same directory as far as yarn and phpstan are concerned.

However to fully test that we'd need to commit the change to HEAD so that the cache warming job runs with the bind mount, then see what the effect on pipelines is after that. Can probably hack the API URLs to point to the pipeline instead of core just to make sure it doesn't break things, but not easy to demonstrate it actually works.

An extra complexity here is while phpstan gives you an idea why the cache wasn't used, none of the yarn linting tools do (or at least I haven't found it). But probably if we can make phpstan work, the others will work too.

One thing is that in a way this vindicates the idea to use artifacts instead of distributed caching because with distributed caching we'd have a lot less control over how the cache files are generated, so would get cache misses all over the place with even less information on how to debug.

catch’s picture

Status: Needs review » Needs work
catch’s picture

Priority: Normal » Major

Tried a bind mount but the user in the pipeline doesn't have permission to create one. This might need some help from infrastructure, i.e. can our k8s runner setup allow the bind mount, or can we do something there to have a consistent directory name?

cmlara’s picture

Since we’re on docker runners an over simplified possibility

mkdir /build;
cp -Ria $CI_PROJECT_DIR /build
cd /build
<execute as normal>
cp -Ria anything_you_need_to_archive $CI_PROJECT_DIR 
bbrala’s picture

Issue tags: +Barcelona2024

Was talking a little about this with @catch at dc. I'm thinking we might be able to just run the stylelint jobs in a symlinked directory to make sure the paths are consistent.

Something this simple might just work?

ln -s $CI_PROJECT_DIR /build
cd /build

And then run caching, linters and such. This could consolidate cacheing dirs in the cache files and make it consistent everywhere. I want to see how that works locally, but internet here is not really being helpfull.

bbrala’s picture

cmlara was right on slack. Symlinks dont work.

bbrala’s picture

Made a solution with the copy suggestion. After a full green run we need to analyse the files see if paths make sense.

bbrala’s picture

Status: Needs work » Needs review

Think this should work. But cant get the cached artifacts to download from the issue fork project even after changing the project id. Not sure why.

It also seems the artifacts key in gitlab ci does some magic where it actually finds the artifact/report in the current directory. Not sure how that is possible, but hey, im not complaining to not have to move files around.

I think we should test the pipelines failing also, but im done for today. COuld use a review here.

catch’s picture

This looks like it should work.

But cant get the cached artifacts to download from the issue fork project even after changing the project id.

I think to test this, it would also be necessary to hack the lint warming job to run on MR pipelines (which it doesn't), so that it creates the artifacts in the first place.

Left one suggestion on the MR to try to explain a bit more what's going on in the comment. Apart from that it looks good. Will be hard to be 100% confident that it's working until after we commit it unfortunately, but that's the nature of what we're fixing.

bbrala’s picture

You're up early hehe.

Anyways, I did run the job on the 11.x branch in the issue fork. And I can download the artifacts from the UI. So perhaps if we change the urls of the caches to the normal Web ui urls instead of the api one we might be able to use those.

I'll try that while working on your feedback. See if we can acultually validate.

bbrala’s picture

Ok, so i changed the URL of the download for cspell to a run on the 11.x branch in the same issue fork. That seems to have worked perfectly.

https://git.drupalcode.org/issue/drupal-3470641/-/jobs/2875946

Don't think i can prove this to work more than this. :) I'll revert the hard code.

nicoloye’s picture

Status: Needs review » Reviewed & tested by the community

I reviewed the MR for Björn, we discussed about it at Drupalcon Barcelona 2024.
Everything seems good to me!

  • catch committed 8db24ab8 on 11.0.x
    Issue #3470641 by bbrala, catch, mstrelan, cmlara, nicoloye: gitlab...

  • catch committed 0624b81e on 11.x
    Issue #3470641 by bbrala, catch, mstrelan, cmlara, nicoloye: gitlab...
catch’s picture

Version: 11.x-dev » 10.4.x-dev
Status: Reviewed & tested by the community » Patch (to be ported)

Committed/pushed to 11.x and cherry-picked to 11.0.x, thanks!

Doesn't cherry-pick cleanly to 10.4.x but I think we might want it there for the next couple of years of backport MRs.

bbrala’s picture

Just got home cause of delays. Yay.

But had to check: https://git.drupalcode.org/issue/drupal-3421014/-/jobs/2885532

It did find cache here?

catch’s picture

https://git.drupalcode.org/issue/drupal-3421014/-/jobs/2885532 definitely found the cache:

9538/15309 core/modules/system/tests/src/Kernel/Extension/ModuleHandlerTest.php cached
 9539/15309 core/modules/system/tests/src/Kernel/Form/ElementsFieldsetTest.php cached
 9540/15309 core/modules/system/tests/src/Kernel/Form/FileElementTest.php cached
 9541/15309 core/modules/system/tests/src/Kernel/Form/FormElementLabelTest.php cached
 9542/15309 core/modules/system/tests/src/Kernel/Form/FormElementMaxlengthTest.php cached
 9543/15309 core/modules/system/tests/src/Kernel/Form/FormObjectTest.php cached
 9544/15309 core/modules/system/tests/src/Kernel/Form/ProgrammaticTest.php cached

The output is very verbose, but had previously left it like that so we could verify that it's using the cache, which it wasn't until this issue got fixed!

smustgrave’s picture

Version: 10.4.x-dev » 10.5.x-dev

Probably too late for 10.4 but maybe backport to 10.5?

longwave’s picture

Version: 10.5.x-dev » 11.0.x-dev
Status: Patch (to be ported) » Fixed

Too late to backport now, we've moved on from here quite a bit.

Now that this issue is closed, review the contribution record.

As a contributor, attribute any organization that helped you, or if you volunteered your own time.

Maintainers, credit people who helped resolve this issue.

Status: Fixed » Closed (fixed)

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