Account created on 26 February 2013, over 12 years ago
#

Merge Requests

More

Recent comments

🇪🇸Spain fjgarlin

fjgarlin changed the visibility of the branch 3327584-always-include-orgs-in-api-request to hidden.

🇪🇸Spain fjgarlin

fjgarlin changed the visibility of the branch 3327584-always-send-author-as-credit to hidden.

🇪🇸Spain fjgarlin

fjgarlin changed the visibility of the branch 3327584-send-deleted-accounts-records to hidden.

🇪🇸Spain fjgarlin

fjgarlin changed the visibility of the branch 3327584-send-sa-to-d10 to hidden.

🇪🇸Spain fjgarlin

fjgarlin changed the visibility of the branch 3327584-send-contrib-records-to-d10 to hidden.

🇪🇸Spain fjgarlin

fjgarlin changed the visibility of the branch 3327584-empty-credits-count-fix to hidden.

🇪🇸Spain fjgarlin

Tho the environments feature does exist: https://docs.gitlab.com/ci/environments/

Not sure if this is something that we can leverage here.

🇪🇸Spain fjgarlin

Ignore the above forum post. That's from 2019 and it doesn't seem to be available in the project settings.

🇪🇸Spain fjgarlin

The URL to the docs in the issue description no longer works, and I found this https://forum.gitlab.com/t/modfy-the-deployed-gitlabpages-in-ci-rather-t..., so maybe the way this works has changed.

Worth testing the environments option to see if we get this on our instance.

🇪🇸Spain fjgarlin

It’s in the output of the job, but as it is collapsed by default you may have missed it.

🇪🇸Spain fjgarlin

The Composer install and require drush section is collapse by default.

The problem is there:

Your requirements could not be resolved to an installable set of packages.
  Problem 1
    - Root composer.json requires drupal/core-dev 11.2.0 -> satisfiable by drupal/core-dev[11.2.0].
    - Root composer.json requires drupal/core-recommended 11.2.0 -> satisfiable by drupal/core-recommended[11.2.0].
    - Root composer.json requires jangregor/phpstan-prophecy ^1.0 -> satisfiable by jangregor/phpstan-prophecy[1.0.0, 1.0.1, 1.0.2].
    - drupal/core-dev 11.2.0 requires mglaman/phpstan-drupal ^2.0.7 -> satisfiable by mglaman/phpstan-drupal[2.0.7].
    - jangregor/phpstan-prophecy 1.0.0 requires phpstan/phpstan ^1.0.0 -> satisfiable by phpstan/phpstan[1.0.0, ..., 1.12.x-dev].
    - mglaman/phpstan-drupal 2.0.7 requires phpstan/phpstan ^2.1 -> satisfiable by phpstan/phpstan[2.1.0, ..., 2.1.x-dev].
    - Conclusion: don't install jangregor/phpstan-prophecy 1.0.1 (conflict analysis result)
    - Conclusion: don't install jangregor/phpstan-prophecy 1.0.2 (conflict analysis result)
    - Conclusion: don't install one of phpstan/phpstan[2.1.17], jangregor/phpstan-prophecy[1.0.0] (conflict analysis result)
    - You can only install one version of a package, so only one of these can be installed: phpstan/phpstan[1.0.0, ..., 1.12.x-dev, 2.0.0, ..., 2.1.x-dev].
🇪🇸Spain fjgarlin

Issue created here 📌 Update pages test Active . Creating an MR with doc changes now.

🇪🇸Spain fjgarlin

We could invite a non-maintainer to run MR pipeline in GTD
If you want, we can revoke my access to the project temporarily, then I can create an issue/MR and see.

Happy to coordinate for this.

🇪🇸Spain fjgarlin

If looks good. I just thought about a possible edge case, which we can deal with in here. So NW based on that.

🇪🇸Spain fjgarlin

Thanks for the issue and MR. I looks good. RTBC.

🇪🇸Spain fjgarlin

Yup, it can be controlled by a variable, that's totally fine (and probably desired).

🇪🇸Spain fjgarlin

That's great actually! I think we're on the right track. Let's try those two things that you mention, but yeah, I'd set the strict option by default for all pages jobs.

🇪🇸Spain fjgarlin

my MR runs were built to that url, which was perfect as it did not alter the live site
Let's experiment with that. If the main project has a URL and the MR another, and they don't overwrite one another, we can document it. But let's test it and be sure of that first.

I haven't seen nor found a variable that we could set up. There is an API which we might be able to use.

Question: if we choose the unique domains, are the page URLs created in the main project or in the fork project? I guess it depends on where the MR is created (which depending on the permissions within the project is either canonical project or fork).

🇪🇸Spain fjgarlin

I'll change priority to "major" so the "critical" issues are worked on first. Then this one.

🇪🇸Spain fjgarlin

Good point. It might be posting to itself first and then doing the search. I've got some 5xx just loading https://api.drupal.org/api/drupal/core%21themes%21claro%21claro.theme/11.x, so if the page is pretty heavy, doing the search on top of it might trigger this.

I'm pretty sure that we can change the behaviour of the search.

🇪🇸Spain fjgarlin

The last run's artifact has a correct file: https://project.pages.drupalcode.org/-/gitlab_templates_downstream/-/job...

I assume that the reporting might aggregate data from other runs and just take the last results.

As for the --debug producing wrong formated data, yeah, it might be an upstream issue. When --error-format=junit is present, --debug should not change the format of the output, but it does.

Maybe, on our end, we can replace --debug with empty string on this line:
- php $CI_PROJECT_DIR/vendor/bin/phpstan analyze . --autoload-file="$CI_PROJECT_DIR/vendor/autoload.php" $LEVEL $_PHPSTAN_EXTRA --no-progress --error-format=junit > $CI_PROJECT_DIR/junit.xml || true. Also, probably in this line too:
- php $CI_PROJECT_DIR/vendor/bin/phpstan analyze . --autoload-file="$CI_PROJECT_DIR/vendor/autoload.php" $LEVEL $_PHPSTAN_EXTRA --no-progress --error-format=gitlab > $CI_PROJECT_DIR/phpstan-quality-report.json || true

That way, even if it's set, it will be ignored for the output file.

This issue looks good, hence the RTBC. But we should create a follow-up for the above case, as we know the reason and we have a potential fix.

🇪🇸Spain fjgarlin

The MR looks good.

If you browse the artifacts for the phpstan, you can see the issue. See this: https://project.pages.drupalcode.org/-/gitlab_templates_downstream/-/job...

The contents of the file are:

tests/src/FunctionalJavascript/GitlabTemplatesDownstreamThreeTest.php
tests/src/Functional/GitlabTemplatesDownstreamTwoTest.php
tests/src/Functional/GitlabTemplatesDownstreamOneTest.php
src/form.inc
gitlab_templates_downstream.module
<?xml version="1.0" encoding="UTF-8"?><testsuite failures="0" name="phpstan" tests="1" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:noNamespaceSchemaLocation="https://raw.githubusercontent.com/junit-team/junit5/r5.5.1/platform-tests/src/test/resources/jenkins-junit.xsd"><testcase name="phpstan"></testcase></testsuite>

If you also see phpstan-quality-report.json, the contents are:

tests/src/FunctionalJavascript/GitlabTemplatesDownstreamThreeTest.php
tests/src/Functional/GitlabTemplatesDownstreamTwoTest.php
tests/src/Functional/GitlabTemplatesDownstreamOneTest.php
src/form.inc
gitlab_templates_downstream.module
[]

So some output must be getting into the files.

🇪🇸Spain fjgarlin

I already tested a while back the possibility of testing the "pages" job with a MR-specific URL and it's not possible: #3426311: Allow testing documentation pages via MRs

I know that it requires docker locally and that the URL cannot be shared, but this reproduces 100% what the pages job produces: https://project.pages.drupalcode.org/gitlab_templates/help/documentation...

🇪🇸Spain fjgarlin

Won't fix because it's not possible with our instance setup.

🇪🇸Spain fjgarlin
$ mkdocs build --help
Usage: mkdocs build [OPTIONS]

  Build the MkDocs documentation

Options:
  -c, --clean / --dirty           Remove old files from the site_dir before building (the default).
  -f, --config-file FILENAME      Provide a specific MkDocs config. This can be a file name, or '-' to read from
                                  stdin.
  -s, --strict / --no-strict      Enable strict mode. This will cause MkDocs to abort the build on any warnings.
  -t, --theme [mkdocs|readthedocs]
                                  The theme to use when building your documentation.
  --use-directory-urls / --no-directory-urls
                                  Use directory URLs when building pages (the default).
  -d, --site-dir PATH             The directory to output the result of the documentation build.
  -q, --quiet                     Silence warnings
  -v, --verbose                   Enable verbose output
  -h, --help                      Show this message and exit.

From that, maybe we could/should use the -s flag?

🇪🇸Spain fjgarlin

The issue is nothing to do with the search, it's just that producing some uncached pages takes longer than 30 seconds sometimes, and then the 5xx gets cached.

The amount of seconds before showing a 5xx is set for all our infra. I don't know if we can tweak that per-project. I'll investigate.

Another option could be using something like https://www.drupal.org/project/warmer , but I'm not sure that'll be enough.

🇪🇸Spain fjgarlin

Yes please. I haven't been following too closely on that (really busy with other tasks) but wanted your review precisely for that. Thanks!

🇪🇸Spain fjgarlin

I think if the DA doesn't have any objection to gitlab pages, and also doesn't want to provide some kind of alternative hosting, then that is fine then!

I'll tag @drumm to chip in just in case, but I don't think we have any objection. In fact, we support it by default in all contrib: https://project.pages.drupalcode.org/gitlab_templates/jobs/pages/

The hosting for that is done automatically in our GitLab instance.

I fully agree with that approach, one can be way more user friendly, one can be more technical. Both have a place and an audience and should be able to coexist (eg: the Laravel links and repos provided before).

Should you want quicker progress, you could start with a contrib project where you start putting all the MD files, structure, menu, etc and build the site from there and play with all the options available. Once fully validated and agreed, it can easily be moved to core under a "docs" folder if needed.

🇪🇸Spain fjgarlin

Merged. Will deploy in the next round of deployments to the site.

🇪🇸Spain fjgarlin

Following on the "Cache" example, we'd have:
- Generated 100% by code: https://api.laravel.com/docs/12.x/Illuminate/Support/Facades/Cache.html (this is what api.drupal.org does)
- Generated 100% by MD files: https://laravel.com/docs/12.x/cache (source code at https://github.com/laravel/docs/blob/12.x/cache.md)

🇪🇸Spain fjgarlin

Using GitLab Pages for core documentation is, I think, a core decision, not a DA.

Those Symfony and Laravel documentation pages are the equivalent of "GitLab pages", probably generated from MD files.
The api.drupal.org just parses code.

If we take the example of Laravel, these would be the matching sites in Drupal:
- https://laravel.com/docs/12.x => Site generated with GitLab pages from MD files in Drupal core (easy to achieve but maintained by core and generated via GitLab CI)
- https://api.laravel.com/docs/12.x/index.html => api.drupal.org (this is generated with Doctum, a tool I evaluated before porting the "api" module/site to modern Drupal)

🇪🇸Spain fjgarlin

Ignore the "api" module error. That's a conflict with a dependency and it's something that we are working on in the "api" queue.

🇪🇸Spain fjgarlin

I think the code here should be enough. Please review it in case there is a better approach.

🇪🇸Spain fjgarlin

I see that the issue mentions "GitLab pages" as the agreed way forwards. A GitLab pages site is a separate site from api.drupal.org.
Is there any way that is expected for these two sites to communicate? Or is the goal to completely move away from api.drupal.org for code documentation?

🇪🇸Spain fjgarlin

Another try with both "drupal/core" and "drupal/core-recommended":

$ mkdir test && cd test
$ composer require drupal/core && ls -l vendor/drupal/core | grep "^d" && composer remove drupal/core
...
drwxr-xr-x@  5 myuser  mygroup   160B Jun  6 01:34 assets/
drwxr-xr-x@  4 myuser  mygroup   128B Jun  6 01:34 config/
drwxr-xr-x@ 14 myuser  mygroup   448B Jun  6 01:34 includes/
drwxr-xr-x@  5 myuser  mygroup   160B Jun  6 01:34 lib/
drwxr-xr-x@ 59 myuser  mygroup   1.8K Jun  6 01:34 misc/
drwxr-xr-x@ 76 myuser  mygroup   2.4K Jun  6 01:34 modules/
drwxr-xr-x@  6 myuser  mygroup   192B Jun  6 01:34 profiles/
drwxr-xr-x@ 31 myuser  mygroup   992B Jun  6 01:34 recipes/
drwxr-xr-x@ 15 myuser  mygroup   480B Jun  6 01:34 scripts/
drwxr-xr-x@  7 myuser  mygroup   224B Jun  6 01:34 tests/
drwxr-xr-x@  8 myuser  mygroup   256B Jun  6 01:34 themes/
...
$ composer require drupal/core-recommended && ls -l vendor/drupal/core | grep "^d" && composer remove drupal/core-recommended
...
drwxr-xr-x@  5 myuser  mygroup   160B Jun  6 01:34 assets/
drwxr-xr-x@  4 myuser  mygroup   128B Jun  6 01:34 config/
drwxr-xr-x@ 14 myuser  mygroup   448B Jun  6 01:34 includes/
drwxr-xr-x@  5 myuser  mygroup   160B Jun  6 01:34 lib/
drwxr-xr-x@ 59 myuser  mygroup   1.8K Jun  6 01:34 misc/
drwxr-xr-x@ 76 myuser  mygroup   2.4K Jun  6 01:34 modules/
drwxr-xr-x@  6 myuser  mygroup   192B Jun  6 01:34 profiles/
drwxr-xr-x@ 31 myuser  mygroup   992B Jun  6 01:34 recipes/
drwxr-xr-x@ 15 myuser  mygroup   480B Jun  6 01:34 scripts/
drwxr-xr-x@  7 myuser  mygroup   224B Jun  6 01:34 tests/
drwxr-xr-x@  8 myuser  mygroup   256B Jun  6 01:34 themes/
...

So assuming that that folder is going to be there

🇪🇸Spain fjgarlin

That's not exactly the approach of the templates, but I guess whatever is removing the folder in the above might be removing the folder in our setup.

🇪🇸Spain fjgarlin

Note that projects installed with the "drupal/core-recommended"

$ composer create-project drupal/recommended-project my_site_name
...
$ cd my_site_name && ls -la web 
total 104
drwxr-xr-x@ 19 myuser  mygroup   608B Jun 17 10:58 ./
drwxr-xr-x@  9 myuser  mygroup   288B Jun 17 10:58 ../
-rw-r--r--@  1 myuser  mygroup   1.0K Jun 17 10:58 .csslintrc
-rw-r--r--@  1 myuser  mygroup   151B Jun 17 10:58 .eslintignore
-rw-r--r--@  1 myuser  mygroup    41B Jun 17 10:58 .eslintrc.json
-rw-r--r--@  1 myuser  mygroup   2.4K Jun 17 10:58 .ht.router.php
-rw-r--r--@  1 myuser  mygroup   7.6K Jun 17 10:58 .htaccess
-rw-r--r--@  1 myuser  mygroup    87B Jun 17 10:58 INSTALL.txt
-rw-r--r--@  1 myuser  mygroup   3.1K Jun 17 10:58 README.md
-rw-r--r--@  1 myuser  mygroup   268B Jun 17 10:58 autoload.php
drwxr-xr-x@ 53 myuser  mygroup   1.7K Jun  6 01:34 core/
-rw-r--r--@  1 myuser  mygroup   1.5K Jun 17 10:58 example.gitignore
-rw-r--r--@  1 myuser  mygroup   549B Jun 17 10:58 index.php
drwxr-xr-x@  3 myuser  mygroup    96B Jun 17 10:58 modules/
drwxr-xr-x@  3 myuser  mygroup    96B Jun 17 10:58 profiles/
-rw-r--r--@  1 myuser  mygroup   2.0K Jun 17 10:58 robots.txt
drwxr-xr-x@  7 myuser  mygroup   224B Jun 17 10:58 sites/
drwxr-xr-x@  3 myuser  mygroup    96B Jun 17 10:58 themes/
-rw-r--r--@  1 myuser  mygroup   804B Jun 17 10:58 update.php
🇪🇸Spain fjgarlin

fjgarlin changed the visibility of the branch 1.0.x to hidden.

🇪🇸Spain fjgarlin

Needs work:
- The output of the queries for volunteer and organizations is different. D7 separates by comma, D10 has new rows for each.
- D7 query includes unpublished issues, and should not.

🇪🇸Spain fjgarlin

I know that a new database is created and then dropped for a set of tests, but that shouldn't affect the command that you are trying to run, as it's just about showing DBs and grants.

Maybe we can try running the command from the mysql binary directly and see if we get any additional information.

🇪🇸Spain fjgarlin

Thanks for the acknowledgment @vinodhini.e

🇪🇸Spain fjgarlin

I don't think that D7 has that problem.

In core, for D10+, they still use "mysql", like us: https://git.drupalcode.org/project/drupal/-/blob/11.x/.gitlab-ci/pipelin...

Whereas in D7, they use "drupal": https://git.drupalcode.org/project/drupal/-/blob/7.x/.gitlab-ci/pipeline...

I still don't know if the problem is with the images (tho we use the same ones on D7 and D10...), or with the way that the scripts setup the creation of the database.

🇪🇸Spain fjgarlin

There is an attempt at removing orphan entities: https://git.drupalcode.org/project/api/-/blob/2.x/api.module?ref_type=he...

It's probably not configured. So we should try this first.

🇪🇸Spain fjgarlin

That's not the way the templates work. Evertyhing is documented in https://project.pages.drupalcode.org/gitlab_templates/jobs/, in the "When do the jobs run?" section.

What you need (as mentioned in the slack there https://drupal.slack.com/archives/C08CJ9K74MB/p1749769260020559) is the following:

composer:
  variables:
    DRUPAL_CORE: 10.2.0
🇪🇸Spain fjgarlin

cspell cannot work with symlinked files

That might have been the reason why the approach on this job was different to the others.

100% agree with #15 about reverting and trying the other approach (and 100% on the grateful-but-no-pressure part too).

🇪🇸Spain fjgarlin

Looks good, and totally fine to leave that debug flag for when we run the downstream pipelines too.

🇪🇸Spain fjgarlin

You can have the queue job running every 10 minutes (for example) and use the --time-limit=590 parameter for a new clean run. That way the queue will be continuously running but should not take too much memory. Change 10 minutes for whatever your memory threshold / time is.

🇪🇸Spain fjgarlin

Falta el video. Una vez este subido podemos cerrar el issue.

Gracias @jlbellido!

🇪🇸Spain fjgarlin

I added a bit to the message, but the whole logic is perfect in my opinion.

I don't think we need to worry about the allow_failure setting. If it's false, this addition will make the pipeline fail, and rightfully so, because they want the phpstan job and this is not possible, so they need to do something about it.

RTBC after the suggestion is applied (feel free to change the language if needed) and the testing code is removed. NW for now.

🇪🇸Spain fjgarlin

That is a great point. We need to test and check what's going to happen there.

I added it as a blocker, together with another important case that will need to happen as well.

🇪🇸Spain fjgarlin

I guess it makes sense to do the cd $DRUPAL_PROJECT_FOLDER to match what the other jobs do. We might have just forgotten about cspell when we did the change.

NW based on your suggestions.

🇪🇸Spain fjgarlin

Hi @vinojbi and @vinodhini.e. Note that the issue was already RTBC in #6 the day before you commented on this issue. Also the screenshots provided don't add anything new, as there was one already in #4 with the same changes.

@vinodhini.e, adding your company to both organization and customer won't have any double impact on credits. In fact, doing so might be seen as wanting to game the contribution system to try to obtain double credit.

I recommend you both to go through this page https://www.drupal.org/community/contributor-guide , see the links in it (specially "Issue Queue Etiquette") and even the two videos at the bottom. These resources will help you improve the quality of your contributions, making sure that the time that you spend is meaningful and impactul for you and the community.

🇪🇸Spain fjgarlin

Code looks good and really simple. We are also replacing the Printer as it was intended when the separate repo was created. Tests are happy so I'm merging this.

🇪🇸Spain fjgarlin

Code looks good and the tests are still happy (including the new additions).

🇪🇸Spain fjgarlin

I just checked this again and I think that the current implementation is correct.

The header and the footer are outside the main content area. The main menu is outside, but visually is on top of the content, and the content has padding on top to accommodate for this.

I'm going to mark it as "Fixed" again as I think this is what is expected.

🇪🇸Spain fjgarlin

Yeah, the output from run-tests.sh is the URL given in SIMPLETEST_BASE_URL, which is http://localhost/web in most cases (this line).

The tests can be run locally, on GitLab CI, on a jenkins server (as before) or any other system, so we cannot give that script the final GitLab pages URL (eg: https://project.pages.drupalcode.org/-/keycdn/-/jobs/5521834/artifacts/).

Having said that, something that we could try is to run the run-tests.sh command and capture the output command > output.log, and then run a sed command on that output to replace http://localhost/ with https://project.pages.drupalcode.org/-/$CI_PROJECT_NAME/-/jobs/$CI_JOB_ID/artifacts/ and then show that file as output.

The question about this approach is whether we will get real-time feedback, as we do now vs needing to wait for the full run and then dump it all in one go. This wouldn't be a good user experience in my opinion.

We can try doing it and see how far we get or we could even make a variable to get this behaviour (or use PHPUNIT_EXTRA to pass that option).

🇪🇸Spain fjgarlin

I just moved the file inside a scripts folder. It looks good. Merged.

🇪🇸Spain fjgarlin

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

🇪🇸Spain fjgarlin

The real problem here is this

$ php $CI_PROJECT_DIR/vendor/bin/phpstan --version
Could not open input file: /builds/project/seven/vendor/bin/phpstan

phpstan comes from drupal/core-dev, and this was only included in versions 10+, not 9 or below.

The fix here should be to check for the phpstan binary directly, if not present, exit with a warning before going any further (not an error because we haven't actually checked the code). Let's make sure that the warning is visible via the $DIVIDER syntax that we've used in other places.

I don't think that we need to do if/else logic about Drupal versions, or try to bring phpstan in any other way. I would just check for the existence of the tool and bail out early if not present with an useful message.

🇪🇸Spain fjgarlin

I think the$DRUPAL_PROJECT_FOLDER was just introduced later than the latest changes we did in cspell job.

We can either add the internal file to our ignore list for cspell or just try to run cspell on the $DRUPAL_PROJECT_FOLDER folder directly.

🇪🇸Spain fjgarlin

This is a great refactoring! RTBC.

Will try to test further locally later too.

🇪🇸Spain fjgarlin

I guess it's PerimeterX blocking this @drumm

🇪🇸Spain fjgarlin

MR19 and MR20 look good.

For modern Drupal, we have the composer output, but yeah, maybe a 1-2 liner with just the contrib modules can be nice (separate issue).

🇪🇸Spain fjgarlin

I thought that it was intentional in this MR, that's why I didn't mention anything. Agree, we can review that code in the related issue.

🇪🇸Spain fjgarlin

Yeah, GitLab generates random unique domain names, but they are not predictable. It's nicer as a path from projects.pages.drupalcode.org in my opinion.

🇪🇸Spain fjgarlin

Merged. Thanks for noticing the documentation page!

🇪🇸Spain fjgarlin

The MR looks good. RTBC.

I think there is a setting in the Gitlab repo admin area to fix the URL, if that's what we want here.
It's under Deploy > Pages > Set unique domain name (it might be checked by default and we just need to uncheck it).

🇪🇸Spain fjgarlin

Hopefully this will help (if not fix) with the related issues.

Production build 0.71.5 2024