patching: rewrite: fully stabilize patch index stanzas as well as From lines

[rpardini]

patching: rewrite: fully stabilize patch index stanzas as well as From lines

• patching: rewrite: fully stabilize patch index stanzas as well as From lines
  • git format-patch --zero-commit doesn't affect index xxx...yyy lines, only From:
    • so use the classy "use a regex with a callback" solution as git format-patch doesn't offer one
  • this will make all patches change when rewritten, but hopefully for the last time !
  • we need to preserve index 000000000000..xxx as zeros, which indicate new file creation, thus:
    • new file creations are rewritten as index 000000000000..111111111111
    • non-creations are rewritten as index 111111111111..222222222222
  • this is the final version of RFC: DO NOT MERGE: patching: rewrite: try to stabilize patch index stanzas as well as From lines #6455

[rpardini]

We've a few malformed patches here and there, which are being handled by other people -- due to this I'm holding back a batch of rewrites of the "big 5" kernels edge patches using this implementation for a while.

[ColorfulRhino]

Not quite directly related, but I have discovered that patches created with kernel-patch output patches with 5 additional lines before and after the addition/deletion lines, whereas normal Git patches just have 3 additional lines. While more lines are better to see the patch context, rewrite-kernel-patches does remove/shrink those additional lines, making it basically pointless that kernel-patch added them.

Example:

Patch created with kernel-patch (5 lines before/after the +-):

@@ -400,11 +400,11 @@ $(obj)/%.dtb.S: $(obj)/%.dtb FORCE
 $(obj)/%.dtbo.S: $(obj)/%.dtbo FORCE
 	$(call if_changed,wrap_S_dtb)
 
 quiet_cmd_dtc = DTC     $@
 cmd_dtc = $(HOSTCC) -E $(dtc_cpp_flags) -x assembler-with-cpp -o $(dtc-tmp) $< ; \
-	$(DTC) -o $@ -b 0 \
+	$(DTC) -@ -o $@ -b 0 \
 		$(addprefix -i,$(dir $<) $(DTC_INCLUDE)) $(DTC_FLAGS) \
 		-d $(depfile).dtc.tmp $(dtc-tmp) ; \
 	cat $(depfile).pre.tmp $(depfile).dtc.tmp > $(depfile)
 
 quiet_cmd_fdtoverlay = DTOVL   $@

After doing rewrite-kernel-patches (only 3 lines before/after the +-):

@@ -402,7 +402,7 @@ $(obj)/%.dtbo.S: $(obj)/%.dtbo FORCE
 
 quiet_cmd_dtc = DTC     $@
 cmd_dtc = $(HOSTCC) -E $(dtc_cpp_flags) -x assembler-with-cpp -o $(dtc-tmp) $< ; \
-	$(DTC) -o $@ -b 0 \
+	$(DTC) -@ -o $@ -b 0 \
 		$(addprefix -i,$(dir $<) $(DTC_INCLUDE)) $(DTC_FLAGS) \
 		-d $(depfile).dtc.tmp $(dtc-tmp) ; \
 	cat $(depfile).pre.tmp $(depfile).dtc.tmp > $(depfile)

BUG:
Also as you can see, the line directly in the hunk line stays the same, which is a bug. The line $(obj)/%.dtbo.S: $(obj)/%.dtbo FORCE is not correct anymore after the rewrite.

Maybe I should open an issue about this 😄

[rpardini]

Not quite directly related, but I have discovered that patches created with kernel-patch output patches with 5 additional lines before and after the addition/deletion lines, whereas normal Git patches just have 3 additional lines.

Yes, because https://github.com/armbian/build/blob/main/lib/functions/compilation/patch/patching.sh#L121 (kernel-patch) and https://github.com/armbian/build/blob/main/lib/tools/common/patching_utils.py#L802 (rewrite-*'s).

Should be easy to fix, eg, make them equal -- but really, what is better, 5 or 3, I dunno.

BUG: Also as you can see, the line directly in the hunk line stays the same, which is a bug. The line $(obj)/%.dtbo.S: $(obj)/%.dtbo FORCE is not correct anymore after the rewrite.

Maybe I should open an issue about this 😄

I can't follow; tired eyes. Did this alledged bug happen with this PR or is it on main ?

[ColorfulRhino]

Should be easy to fix, eg, make them equal -- but really, what is better, 5 or 3, I dunno.

5 is better because more context 👍 This way when rewriting patches, it won't remove lines but will also not be able to add new lines I believe (or can it since it can see the context? that would be nice, although would cause lots of commit diff).

I can't follow; tired eyes. Did this alledged bug happen with this PR or is it on main ?

Apologies, I discovered it in main without this PR applied.

[ColorfulRhino]

Seems to work as it should.
Maybe the regex magic could have been avoided using git diff --no-index instead when creating or rewriting patches, but at this point I think your solution is totally fine as well 👍

[rpardini]

git diff --no-index

Hmm. Is there a git format-patch version of this I missed?

[ColorfulRhino]

git diff --no-index

Hmm. Is there a git format-patch version of this I missed?

I don't think so, but actually I think I finally found out how!

Try using the environmental variable: GIT_EXTERNAL_DIFF='git diff --no-index "$2" "$5"' to set how git format-patch uses the diff tool :)
"$2" and "$5" refer to the paths to the temporary files that git creates for the diff.

[rpardini]

Try using the environmental variable: GIT_EXTERNAL_DIFF='git diff --no-index "$2" "$5"' to set how git format-patch uses the diff tool :)
"$2" and "$5" refer to the paths to the temporary files that git creates for the diff.

Oh wow. Will check this out soon, as it's high-hackage but better than regex-replacing index stanzas! Thanks for sharing