Early Bird Registration for DrupalCon Portland 2024 is open! Register by 23:59 PST on 31 March 2024, to get $100 off your ticket.
Currently, git patches are blindly applied with -p1 and then -p0 if that fails. There are scenarios such as core:
/README.txt
/modules/README.txt
and more commonly addition of files that could cause this approach to behave inappropriately.
Comment | File | Size | Author |
---|---|---|---|
#8 | pifr.apply_patches_with_git_1107552_08.patch | 6.43 KB | rfay |
Comments
Comment #1
rfayThere are two areas that need improvement:
* Patch application
* Determining what files were changed
We can improve patch application by doing a git apply --check or a patch -p1 --dry-run and seeing whether it will apply before actually trying it.
We can improve the determination of what files were changed rather dramatically by using git apply -v --check and parsing the result rather than doing what we're currently doing.
There is still some risk, particularly when just adding files, that this will make a mistake. But for the time being I think it's worth the risk. I'd like to phase out -p0 patches within the next year or so and then remove the code that handles this.
Comment #2
rfayWe definitely need to use git apply for actual application. See #22336: Move all core Drupal files under a /core folder to improve usability and upgrades.
Also, need to see what will happen with binary patches.
Comment #3
rfayComment #4
rfayHere are the basics of what needs to happen with this patch:
1. Test patch application instead of just applying the patch.
2. Use git apply to apply the patch
3. Change to using the results of git apply --dry-run -v to determine what files are changed; much simpler.
4. Determine the consequences or implications of binary patches.
Comment #5
scor CreditAttribution: scor commentedsubscribe. this should help to make the infrastructure more consistent, and once we figure out if #1116900: git format-patch displays file moves as whole verbose diffs is possible, make patches smaller :)
Comment #6
scor CreditAttribution: scor commentedoops
Comment #7
rfayI'm going to see if I can take a look at this tonight.
If it can be done and tested, it's easy to deploy because it just means deploying it on the testbots.
Comment #8
rfayI believe this is going to work. I'm testing it now, and will try it out on the scratch servers.
Comment #10
pillarsdotnet CreditAttribution: pillarsdotnet commented+1
Comment #11
rfayI guess we'll have to fix the tests someday.
I committed this to 6.x-2.x: 5e8508
I tested it with binary patches and with moves. I also tested it with a number of -p0 and -p1 patches. It seems OK, although I wouldn't be surprised if there are a few patches that applied with patch that don't apply.
To create a binary patch:
git format-patch --binary --full-index --stdout
To create a moves-and-copies patch (one that magically moves and copies):
git format-patch -C -M --stdout
Comment #12
pillarsdotnet CreditAttribution: pillarsdotnet commentedYay! (rfay++)
Question for rfay:
What do I need to put in my .gitconfig file so that the "--binary --full-index -C -C -M" options are the default?
I tried reading the git docs and couldn't figure it out. Or if there's a tutorial anywhere that you think answers the question, a link would be awesome.
For now I'm just using a bash alias...
Comment #13
rfayCreated 6.x-2.4 release containing this and requested deployment in #1122890: Deploy 6.x-2.4 of PIFR on qa.drupal.org
Comment #14
rfay@pillarsdotnet,
git help format-patch
is the fastest way to the options.git help config
is the fastest way to "how to set up .gitconfig".It looks to me like
git config --global diff.renames copy
is what you want for automatic rename and copy detection.I don't see instructions in there about using binary. I haven't experimented with it, but it may not be something you want on all the time. Seems relatively unusual that we want binary handling.
Comment #15
pillarsdotnet CreditAttribution: pillarsdotnet commentedHmm.. maybe
git config --global format.attach true
allows binary?Thanks anyway. I was hoping/expecting that "git config" could add any arbitrary command-line options, but I guess it can't. Back to bash aliases again.
Comment #16
chx CreditAttribution: chx commentedgit config can't but read https://git.wiki.kernel.org/index.php/Aliases
Comment #17
pillarsdotnet CreditAttribution: pillarsdotnet commentedchx++