composer-patches
composer-patches copied to clipboard
Fix a Unix breaking bug caused by a hack
~~This first checks if you are running windows before changing the patch command to a flag not supported on FreeBSD.~~ Patch is given the --posix flag now.
Should be a fix for #182 If someone with access to multiple windows machines can test this a bit more thoroughly, would be appreciated!
This is not just a Windows thing. --no-backup-if-mismatch exists in whatever version of patch
comes with Ubuntu as well.
FreeBSD is not high on my priority list, mostly because it's one of those things that will constantly break. To my knowledge, there aren't any services similar to Travis CI that provide a way to do automated testing on FreeBSD, so I cannot make any guarantees about it.
If you can find a service that offers FreeBSD, I'm not opposed to looking into official FreeBSD support here, but until that point, my inclination is to recommend that you use a more standard setup.
The reason the flag is used is that you want POSIX behaviour on Patch, in that case, you can also use "--posix" as supported on Linux (GNU Patch) & POSIX Patch.
This is why I changed the flag to --posix. Please merge it? Official support cannot be expected but I fixed this issue while being fully backwards compatible. Why not merge it in?
We are not asking for official FreeBSD support, I completely understand that would take considerably more time. The above patch however keeps backwards-compatibility, and will instantly fix 4 outstanding issues, so I see no reason to not accept this request.
I can get behind using the --posix
flag. It's a superset of what --no-backup-if-mismatch
does, but that's okay.
Note also that master
is where the future 2.0.0 release will live. That release is going to take some time to happen, so you might consider opening a similar PR against 1.x
.
There's something funky going on with Travis today. I'm not gonna merge this until the tests pass, but I think the tests shouldn't be affected by this change. I'll investigate the Travis issues later and then this should be good to go.
only failures showing atm are "allowed" so can has fix? ;)
Can confirm that 33fb8ed573413e09f10efef011046fac5eed10e1 is working for me on Alpine Linux 3.6/3.7
@cweagans Can you try rebuilding in travis? Hopefully it works. I also created #190 for the 1.x branch
@Techwolf12 I've tried rebuilding a couple times over the past few days, and composer just stalls on the php 7.1 acceptance tests. I'm not really sure what's going on there, but I'm also not really in a position to spend a bunch of time debugging it. Can you close this PR and reopen off of the same branch? Maybe there's some stuck cache on Travis or something.
It seems to be composer cache, maybe there is a flag to build composer without cache?
I cleared the caches in the Travis UI. Not sure what's going on. I'll try to look into it this weekend.
As already flagged on #159 this fix won't address a similar issue with BusyBox, which runs on most docker images including the composer official one.
Neither --no-backup-if-mismatch
, nor --posix
, nor -d
switches exist in there.
In the meanwhile to fix the issue mentioned above with docker composer official image I've create a POC image gambry/composer. I'm going to test it a bit and if fixes the problem for running composer/patches through docker composer I will create a proper reliable docker repository, with test coverage.
(Sorry for the cross-post. I think there may be people like me using the docker composer official image and crying due the issue.)
@cweagans Travis tests passed, can we merge this now ?
Thanks!
Please... this is getting annoying
I'm running a fork on our company Gitlab instance to get around this currently
Yeah, would be really nice to have this fix. We've got workarounds in a lot of our internal build processes as well.
@Techwolf12 probably wouldn't hurt to flatten your commits.
I wrote an email to Cameron, I hope we'll get an answer very soon.
Hi all,
I got a reply from @cweagans , he's going to take care of this next week and Drupalcon !
Thanks in advance!
drupol have you heard anything more about this getting resolved?
Yes, I wrote the maintainer and he's going to work on this during the Drupalcon, which is happening now...dunno nothing else :(
We changed the flag to -f because --posix had some weird behaviour when files didn't exist yet (/dev/null as input file)
Feel free to (temporarily) use the following, please do note that this will be an unmaintained repo and can be pulled offline at any time by us. You should use the official repo when it is fixed. It also contains my fix for the Lightning profile (which uses a different document root and breaks core patching): #204
Edit the repositories in your composer.json:
"repositories": {
"drupal": {
"type": "composer",
"url": "https://packages.drupal.org/8"
},
"cweagans/composer-patches": {
"type": "vcs",
"url": "https://gitlab.uncinc.nl/pub/composer-patches"
}
},
Then run:
composer require "cweagans/composer-patches":"dev-bsd as 1.x-dev"
(as 1.x-dev fixes dependencies on composer-patches)
Thanks Techwolf but I will wait for the official version to get fixed although that is not making the boss happy.
@criley If you want, you can clone/fork my repo: https://gitlab.uncinc.nl/pub/composer-patches and run it internally on your own git server?
13f91edc59c conflict should be easy to fix. just a file move.
Hello. I still can't use the package on FreeBSD due to the --no-backup-if-mismatch
flag. Is there any chance for this to be fixed soon? Also, tell me if I can help in any way to get this fixed.
If I understand the github UI correctly, it looks like this branch no longer applies. Can you rebase this? So that when @cweagans gets back to this issue, there won't be any failures on this.
@joris-vercammen-cali I'll be happy to do it if it would be merged, which I don't think will happen. See https://github.com/cweagans/composer-patches/pull/184#issuecomment-380871460 for a hacky 'fix' if you want to use it in your project.
I left some feedback above that was not taken into account. I assumed that you had lost interest, so I haven't prioritized this.
- Needs a rebase
- Needs Travis to finish properly
- Probably needs a companion PR against the 1.x branch if you want to use this anytime soon.
I'm happy to merge this when at least 1 and 2 are done.
@Techwolf12 can fix do the issues that @cweagans mentioned?
In case of alpine container following steps helped me. Patch version (2.7.6) have an option --no-backup-if-mismatch
.
echo '@em http://dl-cdn.alpinelinux.org/alpine/edge/main' >> /etc/apk/repositories
apk add patch@em
In case of alpine container following steps helped me. Patch version (2.7.6) have an option
--no-backup-if-mismatch
.echo '@em http://dl-cdn.alpinelinux.org/alpine/edge/main' >> /etc/apk/repositories apk add patch@em
Patch 2.7.6 seems to have landed in Alpine Linux v3.9 main repository. So the above hack may not be necessary anymore.
The acceptance tests hang forever because of a prompt. If line 27 of c45ce03358 is changed to the following, then all tests succeed:
"patch %s --posix --batch -d %s < %s"
I had the same issue running on Docker. I decided to fix by upgrading patch. This is my docker file
FROM drupaldocker/php:7.2-fpm-2.x
RUN apk update
RUN apk add --no-cache patch
In case of alpine container following steps helped me. Patch version (2.7.6) have an option
--no-backup-if-mismatch
.echo '@em http://dl-cdn.alpinelinux.org/alpine/edge/main' >> /etc/apk/repositories apk add patch@em
Patch 2.7.6 seems to have landed in Alpine Linux v3.9 main repository. So the above hack may not be necessary anymore.
On 3.10 is still affected, for whatever reason, I had to explicitly install patch package to get the unaffected newest version, then it worked.
@lukasz-zaroda Not according to this. Did you try running apk --update add patch
?
Either way I have switched over to https://github.com/vaimo/composer-patches is a little more flexible in that it offers console commands that you can use to reapply patches. It might be subject to the same issue tho. Although I think it falls back to using git apply
So the above hack may not be necessary anymore.
FreeBSD is also still affected.
However, FreeBSD patch
still supports the --posix
option.
@cweagans What do you think about #334 ?
@kentr Should be fixed now, #334 got merged!