Right now all you can do is something like:
projects[htmlpurifier][patch][] = "http://drupal.org/files/issues/htmlpurifier-library_0.patch"
which will download the patch and apply it with 'patch -p0 -d $project_directory < patch'
but many patches require a different strip level or a specific origin file.
I propose adding additional functionality (while keeping the existing one as well) that would allow:
projects[htmlpurifier][patch][0][url] = "http://drupal.org/files/issues/htmlpurifier-library_0.patch"
projects[htmlpurifier][patch][0][strip] = 0
projects[htmlpurifier][patch][0][target] = ""
I'm attaching a patch that enables this
Comment | File | Size | Author |
---|---|---|---|
#128 | drush_make-745224-git-apply-125.patch | 3.92 KB | mrfelton |
#126 | drush_make-745224-git-apply-126.patch | 3.72 KB | Steven Jones |
#125 | drush_make-745224-git-apply-125.patch | 3.66 KB | Steven Jones |
#118 | drush_make_git-apply_test.tgz | 11.39 KB | ao2 |
#110 | git_apply_subdir.sh_.gz | 317 bytes | ao2 |
Comments
Comment #1
medlefsen CreditAttribution: medlefsen commentedJust a quick note, both strip and target are optional and will default to 0 and "" respectively.
Comment #2
dmitrig01 CreditAttribution: dmitrig01 commentedhow about changing the notation to projects[x][patch][url][strip] = 0
Comment #3
dmitrig01 CreditAttribution: dmitrig01 commentedComment #4
yhahn CreditAttribution: yhahn commentedI don't think we should support badly created patches. As described by the docs on Drupal.org, patches should be created from the root directory of the specified project and follow various other conventions. It's simple enough to re-roll a patch from the correct location and it helps everyone who is collaborating on a project for the patch to be in good shape.
Obviously we could spend our time on
drush_make
supporting everything under the sun but I think it's worth making decisions that encourage best practices as well.Comment #5
makara CreditAttribution: makara commentedGit adds "a/" and "b/" to the file paths, when creating with either "git diff" or "git format-patch".
Comment #6
adamdicarlo CreditAttribution: adamdicarlo commented@makara,
You're supposed to roll patches with this command:
I use a bash alias for the purpose:
Comment #7
adamdicarlo CreditAttribution: adamdicarlo commentedOn the other hand, I happened to just look at patch formatting instructions at http://drupal.org/node/1054616 -- apparently patches are supposed to be in the format-patch format these days (since git migration). Huh.
So it looks a bit like drush make will need to support old style (-p0) and new style (-p1) patches.
Comment #8
patcon CreditAttribution: patcon commentedPosted a comment to relevant git handbook page:
http://drupal.org/node/1054616#comment-4152234
Comment #9
makara CreditAttribution: makara commentedI think it's even better if it can support "git am" or similar.
Comment #10
joshuajabbour CreditAttribution: joshuajabbour commentedYes, as far as I can tell, drush_make can't handle any of the newer patches on d.o
Comment #11
mrf CreditAttribution: mrf commentedWhen I follow the new git patch directions that are now available on every single project page the patch fails to apply with drush make.
It's too much to ask users to reformat all patches just to work with make. Being able to specify the type of patch would be a nice temporary fix, but ideally the newer format would just work.
Comment #12
patcon CreditAttribution: patcon commented@makara Correct me if I'm wrong, but I think the catch of
git am
exclusively (which may not be the suggestion :) is that it would require posters of working patches to reveal their email addresses to google bots.. (or fudge it, which doesn't seem right)Comment #13
Steven Jones CreditAttribution: Steven Jones commentedSubscribe.
Even a stop-gap of trying -p1 if -p0 fails would be good here.
Comment #14
makara CreditAttribution: makara commentedYes I think git am is too much now. It is not for the purpose of applying some changes locally. I think git apply is better. But patch -p1 is enough here.
Comment #15
tim.plunkettThe convention for drupal.org is to use -p1.
Comment #16
JeremyFrench CreditAttribution: JeremyFrench commentedJust to add something to the mix here. I wanted to download a stable version of a module and to apply a patch from the projects repository. This patch was not submitted via the issue queue so the natural choice was to use this
http://drupalcode.org/project/flot.git/patch/0b9d38d28e9f66cee239e7f92cc...
However this won't work with drush make and other than rolling my own patch for this diff. I can't do much about it.
Comment #17
Cyberwolf CreditAttribution: Cyberwolf commentedSubscribing.
Comment #18
glennpratt CreditAttribution: glennpratt commentedFollowing the input from Berdir -> http://drupal.org/node/1054616#comment-4179940
This appears to correctly apply patches from http://drupal.org/node/1054616 and older patches without configuration.
Comment #19
tim.plunkettPerfect.
Comment #20
dasjolooks good
Comment #21
boombatower CreditAttribution: boombatower commentedsubscribe
Comment #22
rjmackay CreditAttribution: rjmackay commentedsubscribe. Will test this out soon.
Comment #23
rickvug CreditAttribution: rickvug commentedI can also confirm that this works with new and old style patches. RTBC.
Comment #24
christianchristensen CreditAttribution: christianchristensen commentedThis is awesome! I want to go to there!
Comment #25
jhedstromThe patch in #18 fixes this for 3.x.
With the official end of p0 patches on the horizon (http://groups.drupal.org/node/140204), it seems critical to back port this to 2.x.
Comment #26
rfayI really recommend that you use "git apply" instead of "patch -px".
In the initial round with the testbots, I used patch -px and it meant that binary and copy/move patches wouldn't apply.
Here's the code being used currently on the testbots, see #1107552: Make git patch application more robust and use git for patch application
Comment #27
glennpratt CreditAttribution: glennpratt commentedI tend to agree, but I was trying to make the change minimal.
I assume $this->patch_levels_to_try = array(0, 1); ?
Comment #28
rfaypatch_levels_to_try = array(1,0). But never mind. I don't think it has to be fancier as fancy as that - it was overkill. IMO the existing structure is fine, I just recommend using git apply to apply. Try -p1 then try -p0.
One nice thing you may want to leverage about git though: If you use
git apply --check
it doesn't actually change anything. So you can test whether it applies in one easy step, and then apply it if it applies cleanly.Comment #29
Dane Powell CreditAttribution: Dane Powell commentedsubscribing
Comment #30
jhedstrom@rfay, wouldn't using git apply only work with git checkouts? drush make currently supports many systems other than git, including non-versioned tar/zip downloads, and not all of these downloads come from d.o.
Comment #31
rfayI hadn't thought about that, but I just tried git apply on a non-git directory, and it works fine. Actually, lots better because of the
git apply --check
which still works fine.I imagine that binary patches wouldn't work.
Amazingly, copy-move patches do still work in a non-git directory.
So yes, I think git apply will still work fine. It will require that git be available (in the same way that patch must be available).
Comment #32
Dane Powell CreditAttribution: Dane Powell commentedI think this is fairly major, considering that -p0 patches will be officially deprecated on May 31 and already most patches I'm seeing in the issue queues are -p1 format. I'm happy to test patches to get this to rtbc, though I'm only using 6.x-2.x right now.
Comment #33
glennpratt CreditAttribution: glennpratt commentedNew patch with git apply and uses --check before applying.
Comment #34
glennpratt CreditAttribution: glennpratt commentedOops, left some debugging in the last git command.
Comment #35
rfayI didn't test it, but it looks fine to me.
Comment #36
jhedstromThe patch in #34 works for 3.x. Here is the corresponding patch for 2.x.
Comment #37
Dane Powell CreditAttribution: Dane Powell commentedI can confirm that #36 allows patches generated by git to be applied. Thanks!
Comment #38
Dane Powell CreditAttribution: Dane Powell commentedSooo... RTBC?
Comment #39
izmeez CreditAttribution: izmeez commentedsubscribing
Comment #40
jhedstromThe patch from #36 (for drush make 2.x) isn't actually applying the patches even though
is returning true. I haven't tested against 3.x yet, but suspect a similar issue with the patch in #34.
Comment #41
Dane Powell CreditAttribution: Dane Powell commented@jhedstrom- really? I'm using #36 to great success on 2.x- patches usually apply just fine, and when they fail, it shows up in the drush make log as an error.
Comment #42
jhedstrom@Dane Powell, yeah--git apply seems to be silently failing. This seems to only happen under the following conditions: attempt to patch module foo, within a git repo that is ignoring contrib modules (via .gitignore). If module foo is moved completely out of the .git directory structure, say to /tmp, then git apply works as expected.
git apply fails on the above.
Comment #43
jhedstromYou can reproduce the git apply behavior I mention in #42 as follows:
Comment #44
adamdicarlo CreditAttribution: adamdicarlo commentedLooks like there's another way this `git apply` silent-failure bug can be triggered. Check out this post I just found:
http://data.agaric.com/raw/git-apply-does-not-work-from-within-local-che...
So it seems to me that git-apply is unusable in these two conditions:
* your .gitignore indicates ignorance of the to-be-patch file
* your current directory is in an unrelated git repository
The common element apparently is that the to-be-patched file is not committed (and not staged?) in your current-directory git repository.
It seems the solution requires either not using git-apply or changing into a separate directory when running git-apply. Unless there's some git-apply flag that I'm missing that could work around this behavior.
Comment #45
glennpratt CreditAttribution: glennpratt commentedI don't think there is a git apply flag, I looked. I think the correct solution is making sure we apply the patches outside any git repo (except perhaps for the current module).
Thing is, it appears that is happening already.
This is a section of
drush --verbose --debug make test.make www
, working on a -p0 patch, where test.make has both types of patches, run inside a git repo with www/sites/all in .gitignore. Note, we patch in /tmp already.I manually verified the patch is applied correctly in the resulting build.
I put this back to Needs Review, just because I would need more info to be able to fix that issue.
Comment #46
jhedstrom@glenpratt, you're right, the build is already taking place outside of any git repo. So, by adding calls to chdir into the build directory, git apply works as expected. The attached patch is for 2.x, I'll reroll the 3.x one shortly.
Comment #47
jhedstromOops. That last patch had an xdebug call in it.
Comment #48
jhedstromAnd here is the 3.x patch.
Comment #49
jhedstromAnd now, a working one for 3.x (the last patch was still using $project_directory instead of $this->project_directory). Sorry for the noise.
Comment #50
glennpratt CreditAttribution: glennpratt commentedInteresting... I'm wondering what I missed trying to reproduce what you found.
Comment #51
j0nathan CreditAttribution: j0nathan commentedSubscribing.
Comment #52
ldweeks CreditAttribution: ldweeks commentedsubscribing
Comment #53
Dane Powell CreditAttribution: Dane Powell commentedMarked #1110430: Unable to apply patch generated following instructions on drupal.org as dupe.
Comment #54
q0rban CreditAttribution: q0rban commentedHere's a different (and more abstract approach) that was on #1110430: Unable to apply patch generated following instructions on drupal.org.
Comment #55
jhedstromI haven't tested #54 yet, but it doesn't appear to include the workaround needed for the issues in #40-#48. It also switches back to using patch instead of git apply. I've been successfully using #48 for a few weeks at this point.
Comment #56
q0rban CreditAttribution: q0rban commentedFailed to mention, the problem with using git apply is that when it fails, it still returns 0 (success), so there's no way to detect failure. So, the patch in #54 doesn't use git apply, and instead uses patch and allows you to specify options to that command. It defaults to -p1, but you can specify any options you choose. More flexible IMO.
Comment #57
jhedstrom@q0rban, from my testing (see #43), git apply does properly return non 0 on failure when it is outside the context of another git repository, that's why the patch in #47/48 is using chdir to actually go to the tmp directory where the make file is being built.
Comment #58
rfay@q0rban, the fundamental reason for using git apply is that it can apply patches containing moves, copies, and binary diffs, things which patch of course cannot do.
Comment #59
q0rban CreditAttribution: q0rban commented"git apply does properly return non 0 on failure when it is outside the context of another git repository, that's why the patch in #47/48 is using chdir to actually go to the tmp directory where the make file is being built."
That was not my experience in testing for writing the patches in #1110430, but I'll give it another go. Thanks!
Comment #60
q0rban CreditAttribution: q0rban commentedThe patches in #47/49 would not work for non-git downloads. I still think the approach here needs to not assume git. Are binary diffs really an edge case we need to account for?
Comment #61
rfayWe've already had patches with both binary diffs and copies/moves in the queue.
If we don't support them, we won't be able to apply patches from the issue queue that could normally be applied and that people would expect to work.
Comment #62
q0rban CreditAttribution: q0rban commentedI'll try to work on a patch for this, but I have my doubts. ;)
Comment #63
jhedstromq0rban, can you post some example patches for testing non-git downloads failing? I've been using it on some fairly old .make files that are presumably still using pre-git patches.
Comment #64
q0rban CreditAttribution: q0rban commentedSorry, i was using git am and that was failing. You are right, git apply works on non git repos. However, git apply still is returning 0 for me when I use patches that don't apply.
Comment #65
q0rban CreditAttribution: q0rban commentedIt seems like this might be a bug in drush itself, or possibly PHP exec(): if I run the command by hand, I get a sane exit status:
Comment #66
rfayI guess we have to talk about what kind of patch and what kind of problem. Just for sanity, I just tried a good patch and a bad patch against Examples D7. Note that the return values are correct (I'm using git apply --check here in the first round of tries, which is a wonderful thing). Also note that git doesn't actually apply if a patch application fails. The patches I used are attached.
So here are the things I can think of that are causing the discrepancy:
* Different versions of git?
* Different types of patch failures?
* Different patch application context?
Comment #67
q0rban CreditAttribution: q0rban commentedOkay, figured out my problem. I had a trailing semicolon on the command that was being executed, and drush_shell_exec then appends 2>&1 to that. Patch attached against 6.x-3.x-dev.
Comment #68
q0rban CreditAttribution: q0rban commentedWhoops, looks like somehow the patch got stripped to 120 chars wide. New patch attached.
Comment #69
glennpratt CreditAttribution: glennpratt commented@q0rban - Why are we taking a config approach? This means existing drush make files will not work.
Also, you don't have to use git am to apply a patch made with git-format-patch, I can apply them just fine with git apply or patch. I feel like it's a mistake to link the patch command to the command that generated the diff.
I could see a use case for adding 'git am' option with --working-copy, but that seems like scope creep for this issue.
Comment #70
rfaygit am is probably completely inappropriate for drush make. It actually makes a commit (if a patch was created with git format-patch), and we don't need that. And yes, any patch that applies with git am should apply with git apply.
Comment #71
q0rban CreditAttribution: q0rban commentedI think it's more flexible and safer.
You don't have to use git-format-patch, but I figured some might like the option. I can remove it if it is a problem.
Hmm, Randy has coerced me into using git instead of patch for that very reason (the application used to create the diff should also be the application that applies the patch).
Comment #72
rfayI don't agree that "the application used to create the diff should also be the application that applies the patch". I think git apply should be used for every patch in a context like this. I have found no downside to this approach.
Comment #73
jhedstromThe patch in #68 will require p0 patches to be explicitly declared in the .make file--is this desired? If so, it will break all .make files out there pointing at p0. The benefit of the approaches earlier in this thread was that they were backwards compatible.
Comment #74
rfayI'm not a drush make user, so don't have much of a voice here, but @q0rban, I remain a bit baffled why you started over, when the earlier patches (#47/49) in this issue were (I thought) near-RTBC. I favored them as simple and unobtrusive and backward-compatible.
I guess we should decide at this point which way we're going to go. I vote for the simpler approach.
Comment #75
glennpratt CreditAttribution: glennpratt commentedI'm +1 for git apply only and determine the patch level automatically, mainly because it's what PIFR is doing.
If you run into a problem, re-roll the offending patch to Drupal standards. That will make everyones life easier.
Convention over configuration or something like that...
Comment #76
rfayRemembering, of course, that the testbots will cease to auto-detect patch levels at the end of this month.
Comment #77
q0rban CreditAttribution: q0rban commentedI didn't start over, I picked up from my previous patch that happened to be on a different issue, and added the git-apply functionality like you requested.
The reason the patches in 47/9 don't work is that they require git. We can't assume git. If you're writing an install profile with a drupal-org.make file for d.o you can assume git, but if a developer is writing a make file for themselves and they don't have git, the patches will fail, with no notification of why. The approach in #68 sticks to convention, but adds configuration to allow the developer to customize the way in which the patch is applied in any way they choose.
I agree that existing drush make files will be broken, but what did we expect in changing from p0 to p1? "If you run into a problem, re-roll the offending patch to Drupal standards." Exactly.
I think we need Dmitri to weigh in here before we proceed, but I feel like everyone here is coming at this from a "fix drush_make for d.o" mentality and not a "fix drush_make for developers and d.o" mentality. Flexibility wins, in my book.
Comment #78
jhedstromI think we can assume git. Even if a developer is using drush make for a client project, using svn or bzr, they should still have git on their machine. Back when drupal was on cvs,
project[...][download][type] = CVS
failed silently and inexplicably if the system didn't have CVS installed--this sort of usability can, and should, be addressed in a separate issue.I expect, that wherever possible, something like drush make retain backwards compatibility--especially, as you mention, if patches are coming from non-drupal.org sources (say, a patch to a jquery library or a WYSIWYG) where p0 is still the standard. Using git apply, in combination with testing for p1 or p0, allows both formats to simply work, without having to write an extra line for every patch in the make file.
Comment #79
q0rban CreditAttribution: q0rban commentedI think assuming git is a mistake, for the very reason that you mention, for "patches [that] are coming from non-drupal.org sources (say, a patch to a jquery library or a WYSIWYG)."
Maybe I just joined this issue the wrong way. I didn't intend to ruffle any feathers, I just brought along the patch that had been worked on in a duplicate issue and was trying to accommodate everyone's desire to default to git-apply, as well as add some extra features that I think developers would like, such as the ability to specify custom options and choosing the application that will apply the patch.
But I will stop. Randy says 47/49 are rtbc.
Comment #80
q0rban CreditAttribution: q0rban commentedWhoops, nevermind, Randy said they were "near RTBC"
Comment #81
q0rban CreditAttribution: q0rban commentedOk, here's an updated version based off of #49. Differences are:
git apply --check -p'1'
). I tried to use %d, but that didn't work (kept outputting 0 there)Comment #82
q0rban CreditAttribution: q0rban commentedWhoops, comment got placed in the wrong spot.
Comment #83
jhedstromThe patch in #82 works for 3.x for me.
I started on a backport to 2.x, but wasn't sure where the 2.x equivalent of $this->name would be found.
I'm changing this issue from a feature request to a bug, since without this patch, patching via drush make from d.o. no longer works.
Comment #84
univate CreditAttribution: univate commentedI am using the patch is #82 without any problems, I have it working on a project with both -p0 and -p1 patches and everything is building correctly. I much prefer this patch to other previous ones that require special syntax changes to the *.make files.
Tempted to change this to RTBC
Comment #85
glennpratt CreditAttribution: glennpratt commented@q0rban - Thank you for the work! I didn't mean to sound argumentative, just sharing my 2 cents.
This has been RTBC before. I'm using #82 with success. univate likes it. RTBC
Comment #86
q0rban CreditAttribution: q0rban commentedNo, not at all, I didn't take it that way. I was just getting frustrated because I was misunderstanding and subsequently put a bunch of time into the other solution. :)
Comment #87
mrfelton CreditAttribution: mrfelton commentedPatch in #82 works for me. Thanks.
Comment #88
mrfelton CreditAttribution: mrfelton commentedSorely in need of a 2.x version of this.
Comment #89
jhedstrom@mrfelton, the patch in #47 is what I've been using in 2.x for now--I started to roll the patch in #82 into 2.x, but ran into an issue not knowing what the 2.x equivalent of $this->name would be.
Comment #90
webchickSubscribe. Coming here from #1148774: Automatically patch core in Drush Makefile.
Comment #91
Steven Jones CreditAttribution: Steven Jones commentedWill post a 2.x version of #82 in a couple of hours.
Comment #92
Steven Jones CreditAttribution: Steven Jones commentedThat would be
$this->name
, simple!Attached is basically the patch from #82 ported to Drush make 6.x-2.x.
Comment #93
Steven Jones CreditAttribution: Steven Jones commentedCan we use the
drush_shell_cd_and_exec
function here, instead of manually calling 'cd' here? I think it's possibly drush 4 only though, not sure how that should be handled in drush make? Do we need to raise the requirements?Also, it should be noted that this change is going to have a fair bit of impact for patches that include new files, and that need to be applied with '-p0', as there is a bug in most versions of git that prevents such patches being applied, and they fail instead with
We might need to point this out in the release notes, as some make files will start breaking for this reason I would imagine.
Powered by Dreditor.
Comment #94
Darren Shelley CreditAttribution: Darren Shelley commented#82 fails for me with 3.x
On entry to the following line my current working directory is the drupal root and the project_directory is "custom_search", changing directory to it therefore fails for me:
EDIT: Fixed with re-download of drush make, perhaps had old version of make.
Comment #95
jhedstrom@Darren Shelley do you get an error message? Can you run the command with
--debug
and post back any relevant messages here?Comment #96
Darren Shelley CreditAttribution: Darren Shelley commented@jhedstrom,
I redownloaded drush, make and the patch so I could be confident of the commit numbers and this resolved my issue.
Thankyou.
Comment #97
jhedstrom@Steven Jones I don't think raising the requirements to Drush 4 should be an issue, since Drush 3 isn't even listed on the module page anymore.
Regarding the p0/new file issue, do you have any more details on which versions of git this happens, or which version it was fixed in? Mentioning that in the release notes seems sufficient.
(Also bumping this to critical, as patching via drush make will not work for p1 patches, and official end of p0 support on d.o. was May 31.)
Comment #98
Steven Jones CreditAttribution: Steven Jones commentedGit don't seem to use a nice issue tracking system, instead using mailing lists so I can't seem to find the place in which it was fixed, but a google search does turn up lots of related posts:
http://www.google.com/search?btnG=1&pws=0&q=fatal%3A+git+apply%3A+bad+gi...
I'm using git from ubuntu 10.10, which http://packages.ubuntu.com/maverick/git seems to suggest means that git has this issue in the 1.7 series, which is a bit scary.
Comment #100
Agileware CreditAttribution: Agileware commentedSubscribing
Comment #101
drew reece CreditAttribution: drew reece commented#92 Worked for me with drush make 6.x-2.2
Comment #102
xatoo CreditAttribution: xatoo commentedThe patch in #92 worked fine for me as well (drush_make 6.x-2.2).
I don't know whether there is consensus yet on whether or not to require Drush 4. I however rerolled Stevens patch from #92 for 3.x and changed drush_shell_exec to drush_shell_cd_and_exec.
Comment #103
Steven Jones CreditAttribution: Steven Jones commentedI really don't want to introduce a regression in drush make, we'll cause an awful lot of pain for people. I wonder if we can detect git failing in this way, and then try patching with the old method, using patch.
Comment #104
Steven Jones CreditAttribution: Steven Jones commentedOkay, so here's #92 plus a fallback to 'patch -p0' if we detect 'git apply' being funny about new files in patches. This should cover all the bases, apart from if git throws the error message in another language, which I'm not sure we can/should do much about.
This is for drush make 6.x-2.x.
Comment #105
Steven Jones CreditAttribution: Steven Jones commentedAnd a patch with the fallback for those that prefer to roll with 6.x-3.x.
Reviews please!
Comment #106
kasperg CreditAttribution: kasperg commented#104 works for me. Thanks!
Comment #107
j0nathan CreditAttribution: j0nathan commentedHi,
With patch in #104 I was able to apply patches in both formats in the same makefile using drush make 2.2 and drush 4.4.
Thank you
Comment #108
gboudrias CreditAttribution: gboudrias commentedSubscribing
Comment #109
justintime CreditAttribution: justintime commentedsubscribe
Comment #110
ao2 CreditAttribution: ao2 commentedHi, in my scenario
git-apply
does not work, let me explain:git
(so/profiles/myprofile/.git
exists)/profiles/myprofile/modules/contrib/
and their files are all untracked in the.git
above from the profile)normally
git-apply
should fallback to normalpatch
when handling files not controlled by git, but in this case it fails to apply changes to untracked files in subdirs, so I think tryingpatch
first is the way to go to be more failsafe, and then possibly givegit-apply
a try if it that fails, so we can use its features (like git files renaming whichpatch
does not support).However I think that even relying on
patch
alone is more than acceptable, so we get rid of the hard dependency on git as well.Let me know if the description of my scenario is not clear enough, I am also attaching a bash script to recreate what I am experiencing with
git apply
.Thanks,
Antonio
Comment #111
lelizondo CreditAttribution: lelizondo commentedpatch in #104 worked for me. thanks.
Comment #112
glennpratt CreditAttribution: glennpratt commented@ao2
Projects are downloaded and patched in a temp directory, not in your Git repo, exactly because of that concern. My make files are always in Git repos and I have a mix of all sorts of modules sources. If this is indeed a problem, the patch you have needs work.
Patch cannot correctly apply all Git patches. Git is what Drupal and all of contrib uses, so I don't see that as an option.
Comment #113
boombatower CreditAttribution: boombatower commentedSeems like git apply and fall back to patch -p0 would work, but warn people or maybe make it a setting you have to explicitly override? Otherwise lets just make git a dependency and move forward. Either way we need to at minimum support git apply.
Comment #114
ao2 CreditAttribution: ao2 commented@glennpratt Ah, maybe that is what I was missing, I thought the patching was happening at the final destination (
/profiles/myprofile/modules/contrib/mymodule
in my case), not in the temp dir already, I will check that.If the patch is applied locally in a temp dir before moving to the destination, then
git apply
should work in my scenario too indeed.Thanks for mentioning this.
Comment #115
ao2 CreditAttribution: ao2 commentedI checked and the problem is still there because patches are applied to projects inside the destination tmpdir, not the project local tempdir, in my case the patch is applied to
/tmp/drush_make_tmp_1309939611/__build__/profiles/myprofile/modules/contrib/mymodule
, so still in a subir of the git controlled/tmp/drush_make_tmp_1309939611/__build__/profiles/myprofile/
.Maybe I wasn't able to express my problem clearly: only
myprofile
is fetched with git in my case, notmymodule
, and that is what makesgit-apply
to fail. Is anyone able to replicate what I am experiencing?If I got it right the current sequence of actions performed by drush_make for a project is:
Maybe 2 and 3 should be switched, but for that to happen copying to the destination dir should be decoupled form the download task.
Thanks,
Antonio
Comment #116
jonhattan#104 also works for me for a
git diff
patch. Of course I get the project to be patched from git.Comment #117
Steven Jones CreditAttribution: Steven Jones commented@ao2 are you saying that if you have a .gitignore file in the git repo, then git apply will honour that file and won't patch the files specified in it?
Comment #118
ao2 CreditAttribution: ao2 commentedI am attaching a stripped down example to show what I mean.
Remember, only
myprofile
if downloaded with git (the fact that I am using thefile://
transport does not make any difference at all), not the other modules.Download the archive and follow the README.
From README.asciidoc
drush_make patched with http://drupal.org/files/issues/drush_make-745224-git-apply-105.patch
Fetch the code
Verify that the patch has been applied
In the file myprofile.make there is a patch specified for the rubik theme:
But it looks like it is not applied by git-apply, because myprofile is under git control, while the themes/contrib/rubik/ subdir is not.
As you can see the patch still applies even if it is supposed to be applied already according to myprofile.make and the drush output.
If I try to run git apply here, it does not work:
Note that it fails silently no indication at all that the patch has not been applied.
Thanks,
Antonio
Comment #119
glennpratt CreditAttribution: glennpratt commented@ ao2 Oh, I see. I'm guessing --working-copy is part of replicating that issue.
I'm going to play around with
git apply
flags, but you are probably on the right track to patch in project temp dir.Comment #120
glennpratt CreditAttribution: glennpratt commentedPoked around, asked in IRC, don't see a way around it, other than patching in temp dirs for projects.
Comment #121
Steven Jones CreditAttribution: Steven Jones commentedOkay, so this is git, so is uber configurable, specifically the
GIT_WORK_TREE
environment variable can be set to the work tree:$project_directory
meaning that it won't get applied to higher level git repos if they exist.We can also add the
--verbose
flag to give more feedback.Drush make has some tests at the moment, we should add some tests too!
Comment #122
ao2 CreditAttribution: ao2 commented@Steven Jones thanks, that worked indeed.
This change is enough to make patch in #104/#105 cover my scenario too:
I used
.
because wecd $project_directory
already.Reordering the download, copy and patch tasks would still be nice but that can be done later.
Should I send a fixed up patch or will you do that Steven?
And what about improving the commit messages a little bit? This is the scheme I use:
Thanks again,
Antonio
Comment #123
Steven Jones CreditAttribution: Steven Jones commentedI'm seeing that git is unable to patch some files that patch used to be able to do, which is a bit strange, I need to investigate more and will roll a patch with the changes in later.
Comment #124
ao2 CreditAttribution: ao2 commentedAbout the reordering of the "download", "copy" and "patch" operations, I opened a dedicated issue #1212834: Decouple "download" and "copy" operations..
This one can be solved independently from that reordering problem.
Thanks,
Antonio
Comment #125
Steven Jones CreditAttribution: Steven Jones commentedRight, so here's a patch for drush make 6.x-2.x that incorporates the GIT_WORK_TREE fix. I've also updated the patch tests so they pass.
Comment #126
Steven Jones CreditAttribution: Steven Jones commentedAnd a version for 6.x-3.x
Comment #127
mrfelton CreditAttribution: mrfelton commented#104 applies to 4.4 and gets things working for me. Thanks.
Comment #128
mrfelton CreditAttribution: mrfelton commentedEDIT: I take that back. It worked with one exception. So I tried the patch at #125 which also doesn't always work.
With a make file like this:
drush_make 6.x-2.2 and the patch at #125 applied. Neither of the attempts to patch with git work, and so it resorts to using patch -p0 which doesn't work either since it's a git patch. This revised patch also adds another fallback to try with patch -p1, which gets it working.
Comment #129
thedavidmeister CreditAttribution: thedavidmeister commentedpatch #128 does seem to make patches work that previously failed when run through aegir.
thanks!
Comment #130
Steven Jones CreditAttribution: Steven Jones commentedgit should be able to apply the patch from the makefile in #128: http://drupal.org/files/issues/1053674-Use_colorbox_module_setting-4_0_0... as that's the entire point of using git apply in drush make, any idea as to why it doesn't apply?
Comment #131
ShaunDychko CreditAttribution: ShaunDychko commented#125 is working nicely for me. Thank you very much.
Both http://drupal.org/files/issues/preserve_roles.patch and http://drupal.org/files/issues/securepages-750104-3.patch wouldn't apply before the patch in #125.
Comment #132
jhedstromLooks like this was committed a few weeks ago. Marking as fixed.
http://drupal.org/commitlog/commit/11066/9df4b5fd0264a30572dc5188337055d...
http://drupal.org/commitlog/commit/11066/d1a6fcd7c1f359c1a655bd927644f15...
Comment #133
Steven Jones CreditAttribution: Steven Jones commented@mrfelton Looking at the patch that isn't applying, it doesn't apply with git, so isn't a 'valid' drupal.org patch, so would need to be re-rolled.
Awesome that we got this in!
Comment #134
dmitrig01 CreditAttribution: dmitrig01 commentedOh, sorry for not marking at such. Thanks all!
Comment #136
jgraham CreditAttribution: jgraham commentedGIT_WORK_TREE does not resolve the issue described by ao2 for me in comments 110, 115.
I wasn't sure if this issue should be re-opened so I created a new one;
http://drupal.org/node/1276872
Comment #137
thedavidmeister CreditAttribution: thedavidmeister commentedI'm using patch #128.
What I'm seeing when building from a make file that includes patches:
Without --working-copy, or through Aegir
* A module pulled from d.o. as a tarball (ie. not tracked by git) and just identified with a version number will have patches applied correctly
* A module taken from a git repo with a revision set in the make file will have patches applied correctly
Running drush make through the command line with --working-copy
* Modules pulled from d.o. as a tarball will NOT have patches apply at all, but drush will report that the patch was successful and even generate PATCHES.txt
* Modules cloned from a git repo from a set revision will have patches apply correctly and drush will report exactly the same as the previous case where the patch failed
Comment #138
thedavidmeister CreditAttribution: thedavidmeister commentednm, just saw jgraham's post