Automatically closed - issue fixed for 2 weeks with no activity.
Automatically closed - issue fixed for 2 weeks with no activity.
Automatically closed - issue fixed for 2 weeks with no activity.
Automatically closed - issue fixed for 2 weeks with no activity.
- Issue created by @yobottehg
- 🇨🇦Canada mparker17 UTC-4
On the plus side, though, it seems like tests are passing in the merge request as it is now.
- 🇨🇦Canada mparker17 UTC-4
Re: #113 you could apply the patch from that other issue to core using composer-patches and then use it in this issue, yes.
@bradjones1 Hmm... could you explain how to do this, or point me towards some documentation?
Aside: in order to change this for !8101, I'd have to drop all the commits in the 📌 Adding GIN and GIST indexes to PostgreSQL databases RTBC branch (or re-create the branch), update composer.json to apply the patch, then re-apply the commits specific to this branch... which is fine, but I don't want to force-push until I know it's going to work, and local testing doesn't seem to work. For simplicity below, the logs will assume I'm re-creating the branch.
I've set up a dev environment using justafish/ddev-drupal-core-dev...
$ git clone --branch 11.x https://git.drupalcode.org/project/drupal.git d10-core $ cd d10-core $ ddev config --project-type=drupal10 $ ddev start $ ddev corepack enable $ ddev get justafish/ddev-drupal-core-dev $ ddev restart $ ddev composer install... as I expected, it didn't seem like cweagans/composer-patches was required by anything...
$ ddev composer depends 'cweagans/composer-patches' In BaseDependencyCommand.php line 100: Could not find package "cweagans/composer-patches" in your project... so I tried installing cweagans/composer-patches, but that failed because, I guess, core references itself in its own composer.json?
$ ddev composer require 'cweagans/composer-patches:^1' ./composer.json has been updated Running composer update cweagans/composer-patches > Drupal\Composer\Composer::ensureComposerVersion Loading composer repositories with package information Updating dependencies Your requirements could not be resolved to an installable set of packages. Problem 1 - Root composer.json requires drupal/core 1.0.0 (exact version match: 1.0.0 or 1.0.0.0), found drupal/core[dev-main] but it does not match the constraint. Problem 2 - Root composer.json requires drupal/core-project-message 1.0.0 (exact version match: 1.0.0 or 1.0.0.0), found drupal/core-project-message[dev-main] but it does not match the constraint. Problem 3 - Root composer.json requires drupal/core-vendor-hardening 1.0.0 (exact version match: 1.0.0 or 1.0.0.0), found drupal/core-vendor-hardening[dev-main] but it does not match the constraint. Use the option --with-all-dependencies (-W) to allow upgrades, downgrades and removals for packages currently locked to specific versions. Installation failed, reverting ./composer.json and ./composer.lock to their original content. Composer [require cweagans/composer-patches:^1] failed, composer command failed: exit status 2. stderr=So I tried simply adding the patch to composer.json...
$ ddev composer config --json extra.patches.'drupal/core' '{"#3397622, !5839 Adding GIN and GIST indexes to PostgreSQL databases": "https://git.drupalcode.org/project/drupal/-/merge_requests/5839.diff"}' $ git diff diff --git a/composer.json b/composer.json index b56c8cec3e..e30ad42bdf 100644 --- a/composer.json +++ b/composer.json @@ -96,6 +96,11 @@ " and not intended to be used for production sites.", " See: https://www.drupal.org/node/3082474" ] + }, + "patches": { + "drupal/core": { + "#3397622, !5839 Adding GIN and GIST indexes to PostgreSQL databases": "https://git.drupalcode.org/project/drupal/-/merge_requests/5839.diff" + } } }, "autoload": {...seems fine so far, so lets apply the patch...
$ ddev composer update drupal/core > Drupal\Composer\Composer::ensureComposerVersion Loading composer repositories with package information Updating dependencies Your requirements could not be resolved to an installable set of packages. Problem 1 - Root composer.json requires drupal/core 1.0.0 (exact version match: 1.0.0 or 1.0.0.0), found drupal/core[dev-main] but it does not match the constraint. Problem 2 - Root composer.json requires drupal/core-project-message 1.0.0 (exact version match: 1.0.0 or 1.0.0.0), found drupal/core-project-message[dev-main] but it does not match the constraint. Problem 3 - Root composer.json requires drupal/core-vendor-hardening 1.0.0 (exact version match: 1.0.0 or 1.0.0.0), found drupal/core-vendor-hardening[dev-main] but it does not match the constraint. Use the option --with-all-dependencies (-W) to allow upgrades, downgrades and removals for packages currently locked to specific versions. Composer [update drupal/core] failed, composer command failed: exit status 2. stderr=... same as before.
What about just updating the lock file?
$ ddev composer update --lock > Drupal\Composer\Composer::ensureComposerVersion Loading composer repositories with package information Updating dependencies Your requirements could not be resolved to an installable set of packages. Problem 1 - Root composer.json requires drupal/core == 11.9999999.9999999.9999999-dev (exact version match), it is satisfiable by drupal/core[11.x-dev] from composer repo (https://repo.packagist.org) but drupal/core[dev-main] from path repo (core) has higher repository priority. The packages from the higher priority repository do not match your constraint and are therefore not installable. That repository is canonical so the lower priority repo's packages are not installable. See https://getcomposer.org/repoprio for details and assistance. Problem 2 - Root composer.json requires drupal/core-project-message == 11.9999999.9999999.9999999-dev (exact version match), it is satisfiable by drupal/core-project-message[11.x-dev] from composer repo (https://repo.packagist.org) but drupal/core-project-message[dev-main] from path repo (composer/Plugin/ProjectMessage) has higher repository priority. The packages from the higher priority repository do not match your constraint and are therefore not installable. That repository is canonical so the lower priority repo's packages are not installable. See https://getcomposer.org/repoprio for details and assistance. Problem 3 - Root composer.json requires drupal/core-vendor-hardening == 11.9999999.9999999.9999999-dev (exact version match), it is satisfiable by drupal/core-vendor-hardening[11.x-dev] from composer repo (https://repo.packagist.org) but drupal/core-vendor-hardening[dev-main] from path repo (composer/Plugin/VendorHardening) has higher repository priority. The packages from the higher priority repository do not match your constraint and are therefore not installable. That repository is canonical so the lower priority repo's packages are not installable. See https://getcomposer.org/repoprio for details and assistance. Composer [update --lock] failed, composer command failed: exit status 2. stderr=... apparently not!
What if I try running GitLab CI tests locally → ?
$ alias drupal-ci-local='gitlab-ci-local --remote-variables git@git.drupal.org:project/gitlab_templates=includes/include.drupalci.variables.yml=main --variable="_GITLAB_TEMPLATES_REPO=project/gitlab_templates"' $ drupal-ci-local ... FAIL 📦️ Composer > Invalid version string "HEAD-dev" > > validate [--no-check-all] [--check-lock] [--no-check-lock] [--no-check-publish] [--no-check-version] [-A|--with-dependencies] [--strict] [--] [<file>]... so I'm not sure what to do next.
- 🇨🇦Canada mparker17 UTC-4
> Re: #113 you could apply the patch from that other issue to core using composer-patches and then use it in this issue, yes.
I merged in the commits, but that idea sounds be a lot easier to review, so I'm going to try that shortly.
- @mparker17 opened merge request.
- 🇫🇷France DuaelFr Montpellier, France
@Berdir #15 > The language module provides *only* the entity type + bundle-level setting, that scales fairly well. Only when you also enable content_translation then you get the vast list of per-field-per-bundle checkboxes. Those two things were added at different times, and both pretty early in the D8 lifecycle.
What if we only changed the bit added by content_translation to only add links/buttons in this form leading to a specific page for each entity type? I think we don't even need to split it to a page per bundle to begin with (could be done in a follow up).
- 🇦🇺Australia larowlan 🇦🇺🏝.au GMT+10
larowlan → changed the visibility of the branch 512864-comment-count-query to hidden.
- 🇺🇸United States bradjones1 Digital Nomad Life
Re: #13 you could apply the patch from that other issue to core using composer-patches and then use it in this issue, yes.
- 🇺🇸United States mradcliffe USA
@mparker17 thanks for asking. I think the best thing is to start an issue fork and apply the patch you re-rolled into the issue fork branch and start a merge request.
Merge requests will soon be the only way to test and accept changes to Drupal core and contributed projects on drupal.org
- 🇨🇦Canada mparker17 UTC-4
@bradjones1 would it be helpful to update this patch to use the API in 📌 Adding GIN and GIST indexes to PostgreSQL databases RTBC , given that it is RTBC?
Anything else I could help with?
- 🇨🇦Canada mparker17 UTC-4
Added an update hook. Feedback welcome!
Tagging with "Performance" because it affects performance. Tagging with "PostgreSQL" because it particularly affects sites running on the PostgreSQL database. See the Issue tags -- special tags documentation → for more information.
- 🇦🇺Australia dpi Perth, Australia
Some of what is discussed here is outlined in ✨ Allow CSP to be conditionally added by controller content Active
- 🇫🇷France DuaelFr Montpellier, France
Good job here!
One concern though:
Isn't "Add" an inappropriate button label in some rare edge case where CSS is disabled or overridden somehow? I believe it would be better to use another string explaining what's happening here like "Loading" for example. - 🇨🇦Canada mparker17 UTC-4
I don't think it's possible to write tests for this, but if you have ideas, I would very much appreciate some mentorship on how to write them!
Going to leave this as "Needs review". Feedback is welcome!
- @mparker17 opened merge request.
- Issue created by @mparker17
- 🇺🇸United States smustgrave
Feedback does appear to be addressed and deprecation seems correct.
LGTM
Automatically closed - issue fixed for 2 weeks with no activity.
- 🇬🇧United Kingdom catch
Yes I think so too, it would be a good thing to add alongside a theme builder.
I also opened #3446582: Responsive images recipe → a couple of days ago, to ship some default image styles matching Olivero's break points to start with and linked this issue from there too.
- 🇫🇷France GaëlG Lille, France
I just read about the Starshot project. This issue could be related?
Automatically closed - issue fixed for 2 weeks with no activity.
- 🇨🇦Canada gapple
One possibility might be to break up the alter event into two phases:
- Global alterations, which are not dependent on context (but with adding appropriate cache tags to the event object). This could even be preemptively generated on cache rebuild.
The module's own config would be one instance (tagged withconfig:csp.settings), or the sources added by library info (tagged withlibrary_info). - Per-request alterations that act on things like which libraries are attached to the request, or if something on the page wants to use a nonce.
CKEditor requiresstyle-src 'unsafe-inline', which is only added if it's attached to the page's libraries.
Something like google_tag straddles both, because it is mostly likely present on every page, but would preferentially use a nonce - and the order of alterations when attempting to use a nonce is important ( ✨ Add helper for safely appending nonce/hash sources Fixed , ✨ Enable conditional/alternate directive values Active )
- Global alterations, which are not dependent on context (but with adding appropriate cache tags to the event object). This could even be preemptively generated on cache rebuild.
