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

Ignoring patches from dependencies doesn't work #112

Closed
dmsmidt opened this issue Mar 7, 2017 · 11 comments
Closed

Ignoring patches from dependencies doesn't work #112

dmsmidt opened this issue Mar 7, 2017 · 11 comments

Comments

@dmsmidt
Copy link

dmsmidt commented Mar 7, 2017

I'm trying to ignore a patch defined in the composer.json of the Drupal distribution 'Open Social'.

The setup mentioned in the readme, doesn't seem to work.

This part of the code seems never to be used:

 if (isset($extra['patches'])) {
          if (isset($patches_ignore[$package->getName()])) {
            foreach ($patches_ignore[$package->getName()] as $package => $patches) {
              if (isset($extra['patches'][$package])) {
                $this->io->write('<info>ignore patches each:' . implode(',', $patches) . '</info>');
                $extra['patches'][$package] = array_diff($extra['patches'][$package], $patches);
              }
            }
          }
          $this->patches = array_merge_recursive($this->patches, $extra['patches']);
        }

And in the following part regardless of ignore definitions dependency patches are added.

    foreach ($this->installedPatches as $patches) {
@danepowell
Copy link
Collaborator

I've noticed the same thing... I'm not able to get the patches-ignore functionality to work with Lightning.

@danepowell
Copy link
Collaborator

A couple of notes from Sam Mortenson:

  1. Ignored patches don't support inheritance--i.e. if Lightning were to ignore an upstream patch, your sub-profile based on Lightning would not automatically ignore that patch.
  2. We've seen caching problems with patches-ignore and patches where composer update doesn't re-apply or re-ignore patches, so my team has to clear composer cache often before building.
  3. If neither of the above help, try evaluating one of the referenced PRs

@dmsmidt
Copy link
Author

dmsmidt commented Apr 20, 2017

@danepowell

  1. That is correct, only the root composer.json is used for ignoring patches.
  2. I've had similar problems.
  3. It would be great if you could test Fix ignore dependency patches #118. Note that there are still cases that don't get caught, even with Fix ignore dependency patches #118. See that issue for details.

@seanhamlin
Copy link

seanhamlin commented Sep 30, 2017

Also came across this issue with composer-patches. I am trying to ignore a patch from acquia/lightning, and no matter what I try, I cannot get this to work.

@AlexSkrypnyk
Copy link

@cweagans this is still an issue in 1.6.3. This broken functionality does not allow to set "composer-exit-on-patch-failure": true, as CI builds will fail for any projects relying on Lightning, as there is a patch in Lightning that does not apply.

@cweagans
Copy link
Owner

cweagans commented Dec 2, 2017

@alexdesignworks I think this is the wrong issue for that. Maybe you're looking for #170?

@ghost
Copy link

ghost commented Mar 9, 2018

Any progress here?

Maybe #118 can be simplified for now to get accepted and committed. Simply removing intermediate package (lightning, for example) should work.

@saltednut
Copy link

saltednut commented Apr 13, 2018

I am currently finding that this mostly works, however patches are not re-ignored when running composer update -- the patches from nested projects do work on composer install

In my case, I have acquia/lightning very deeply nested inside a dependency to acquia/df and it works on the initial install but again, not for updates.

When I try to run composer update I see an error that the patch I have listed in patches-ignore cannot apply. (that's why we're ignoring it...)

I do not see this error on composer install because the patch is successfully ignored.

@frederickjh
Copy link

I commented on pull #118 that I tested this using the instructions in first issue that reference in that pull request, in this comment. This was using the forked and patch repo.

This works but does not work when using a separate patches file. The patches-ignore section needs to be in the main composer.json in my test.

I also try going back to an unpatched version using the syntax in the README with the patches-ignore section in the main composer.json but this does not work.

@frederickjh
Copy link

I have switched to version 1.6.6 and now ignoring patches does work if the patches-ignore section is placed in composer.json.

Kingdutch added a commit to goalgorilla/open_social_dev that referenced this issue Dec 13, 2022
Open Social 10 contains a patch for one of our dev dependencies that is
actually outdated.

Unfortunately we must ignore in our `test/` composer.json and can't do
this in our _dev package itself as per
cweagans/composer-patches#112 so it's only a
fix for our own CI and Open Social will have to fix its own stuff in not
patching dev dependencies (but updating this package instead).
Kingdutch added a commit to goalgorilla/open_social that referenced this issue Dec 13, 2022
Open Social 10 contains a patch for one of our dev dependencies that is
actually outdated.

Unfortunately we must ignore in our `test/` composer.json and can't do
this in our _dev package itself as per
cweagans/composer-patches#112 so it's only a
fix for our own CI and Open Social will have to fix its own stuff in not
patching dev dependencies (but updating this package instead).

Copied from goalgorilla/open_social_dev@b4e8545
Kingdutch added a commit to goalgorilla/open_social that referenced this issue Dec 13, 2022
Open Social 10 contains a patch for one of our dev dependencies that is
actually outdated.

Unfortunately we must ignore in our `test/` composer.json and can't do
this in our _dev package itself as per
cweagans/composer-patches#112 so it's only a
fix for our own CI and Open Social will have to fix its own stuff in not
patching dev dependencies (but updating this package instead).

Copied from goalgorilla/open_social_dev@b4e8545

Newer versions of Open Social also contain this patch but in a newer
version. We should move that to the open_social_dev package but we leave
that for a future separate PR.
Kingdutch added a commit to goalgorilla/open_social that referenced this issue Dec 14, 2022
Open Social 10 contains a patch for one of our dev dependencies that is
actually outdated.

Unfortunately we must ignore in our `test/` composer.json and can't do
this in our _dev package itself as per
cweagans/composer-patches#112 so it's only a
fix for our own CI and Open Social will have to fix its own stuff in not
patching dev dependencies (but updating this package instead).

Copied from goalgorilla/open_social_dev@b4e8545

Newer versions of Open Social also contain this patch but in a newer
version. We should move that to the open_social_dev package but we leave
that for a future separate PR.
Kingdutch added a commit to goalgorilla/open_social that referenced this issue Jan 9, 2023
Open Social 10 contains a patch for one of our dev dependencies that is
actually outdated.

Unfortunately we must ignore in our `test/` composer.json and can't do
this in our _dev package itself as per
cweagans/composer-patches#112 so it's only a
fix for our own CI and Open Social will have to fix its own stuff in not
patching dev dependencies (but updating this package instead).

Copied from goalgorilla/open_social_dev@b4e8545

Newer versions of Open Social also contain this patch but in a newer
version. We should move that to the open_social_dev package but we leave
that for a future separate PR.
Kingdutch added a commit to goalgorilla/open_social that referenced this issue Jan 10, 2023
Open Social 10 contains a patch for one of our dev dependencies that is
actually outdated.

Unfortunately we must ignore in our `test/` composer.json and can't do
this in our _dev package itself as per
cweagans/composer-patches#112 so it's only a
fix for our own CI and Open Social will have to fix its own stuff in not
patching dev dependencies (but updating this package instead).

Copied from goalgorilla/open_social_dev@b4e8545

Newer versions of Open Social also contain this patch but in a newer
version. We should move that to the open_social_dev package but we leave
that for a future separate PR.
Kingdutch added a commit to goalgorilla/open_social that referenced this issue Jan 17, 2023
Open Social 10 contains a patch for one of our dev dependencies that is
actually outdated.

Unfortunately we must ignore in our `test/` composer.json and can't do
this in our _dev package itself as per
cweagans/composer-patches#112 so it's only a
fix for our own CI and Open Social will have to fix its own stuff in not
patching dev dependencies (but updating this package instead).

Copied from goalgorilla/open_social_dev@b4e8545

Newer versions of Open Social also contain this patch but in a newer
version. We should move that to the open_social_dev package but we leave
that for a future separate PR.
Kingdutch added a commit to goalgorilla/open_social that referenced this issue Jan 17, 2023
Open Social 10 contains a patch for one of our dev dependencies that is
actually outdated.

Unfortunately we must ignore in our `test/` composer.json and can't do
this in our _dev package itself as per
cweagans/composer-patches#112 so it's only a
fix for our own CI and Open Social will have to fix its own stuff in not
patching dev dependencies (but updating this package instead).

Copied from goalgorilla/open_social_dev@b4e8545

Newer versions of Open Social also contain this patch but in a newer
version. We should move that to the open_social_dev package but we leave
that for a future separate PR.
Kingdutch added a commit to goalgorilla/open_social that referenced this issue Jan 19, 2023
Open Social 10 contains a patch for one of our dev dependencies that is
actually outdated.

Unfortunately we must ignore in our `test/` composer.json and can't do
this in our _dev package itself as per
cweagans/composer-patches#112 so it's only a
fix for our own CI and Open Social will have to fix its own stuff in not
patching dev dependencies (but updating this package instead).

Copied from goalgorilla/open_social_dev@b4e8545

Newer versions of Open Social also contain this patch but in a newer
version. We should move that to the open_social_dev package but we leave
that for a future separate PR.
Kingdutch added a commit to goalgorilla/open_social that referenced this issue Jan 23, 2023
Open Social 10 contains a patch for one of our dev dependencies that is
actually outdated.

Unfortunately we must ignore in our `test/` composer.json and can't do
this in our _dev package itself as per
cweagans/composer-patches#112 so it's only a
fix for our own CI and Open Social will have to fix its own stuff in not
patching dev dependencies (but updating this package instead).

Copied from goalgorilla/open_social_dev@b4e8545

Newer versions of Open Social also contain this patch but in a newer
version. We should move that to the open_social_dev package but we leave
that for a future separate PR.
@cweagans
Copy link
Owner

cweagans commented Feb 7, 2023

main doesn't resolve patches from dependencies, so this is no longer applicable.

@cweagans cweagans closed this as completed Feb 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
7 participants