Skip to contentSkip to conversation

fix build using old versions of patch

PATCH New Ricardo Martincoski 5 participants 9 messages Jan 24, 2016 – Feb 20, 2016

lore
Share & tools
lore
#0 lore raw

v3 cover fix build using old versions of patch

This patch series makes libsoil (currently the only package that patches a file with spaces in the name) to build fine even in very old distros. I tested using patch 2.5 (from 1997) compiled from its tarball in a Ubuntu 14.04 host system.
The patch series also updates the manual to describe the workaround needed in such cases (when a patch need to be applied to a file with spaces in the name).

Anyone willing to help by testing on a RHEL4 or RHEL5 after the code-review? See
http://autobuild.buildroot.net/results/ea7/ea77d6b23aca0cb1cf527e6c16ddf5eba957a69c/

Some background:

patch version 2.7 or later is required to apply patches (generated by
diff v3.3) that change a file with space in the name

The output of "diff -purN" when there are spaces in the name of a changed file depends on the tool version:
diff <= v3.2 does not add double quotes to filenames with spaces diff >= v3.3 adds double quotes to filenames with spaces
Tests were performed using the versions 3.2 and 3.3 from
http://ftp.gnu.org/gnu/diffutils/

The tool 'patch' can handle patches with spaces in the name of a changed file depending on the tool version:
patch <= 2.5.4 cannot handle spaces in filenames
patch >= 2.5.9 can handle unquoted filenames with spaces patch >= 2.7 can handle both quoted and unquoted filenames
Tests were performed using all versions with tarball available at
http://ftp.gnu.org/gnu/patch/

If the patch file is just edited to remove the quote (added by
diff >= v3.3) around the filename, it would become applicable by
patch >= 2.5.9

RHEL5 and RHEL4 use patch 2.5.4. (Thomas P)
There are still users that build using these distros. (Thomas DS)

Symlink and hard link cannot be used because patch unlinks them. But the file can be renamed by a hook before patched.
And the patched file can be renamed back by another hook, if needed.

Ricardo Martincoski (2):
package/libsoil: fix build using old versions of patch docs/manual: patches that change files with spaces in the name

 docs/manual/patch-policy.txt            | 16 ++++++++++++++++
 package/libsoil/0001-fix-makefile.patch |  6 +++---
 package/libsoil/libsoil.mk              |  8 +++++++-
 3 files changed, 26 insertions(+), 4 deletions(-)

---
Changes v2 -> v3:
- remove quotes (Arnout) (I missed the same suggestion from Thomas P)
- say "post-extract" instead of "POST_EXTRACT"
- prefer quotes rather than backslash in the rename hook (Arnout)
- there is no strong argument to use post-extract instead of pre-patch (Arnout)
- fix minor spelling mistakes
- Arnout suggested to drop the patch to the manual:
"I think that this situation is so exotic that it doesn't warrant a mention in the manual. The manual is already long and complicated enough."
I updated the manual for now because it is a separate patch that can be Rejected if others agree with Arnout.

Changes v1 -> v2:
- remove prerequisite patch >= 2.5.9 (added by v1)
- use 1 or 2 hooks to rename the file that contains spaces (based on the suggestion to use a symlink from Thomas DS)
- change the patch file to be applied on the renamed file
- update the manual entry
- do not use the 2nd rename hook for libsoil (Thomas P)

Signature · 1 line
1.9.1
#1 lore raw

PATCH v3 1/2 package/libsoil: fix build using old versions of patch

Well-formed patch fails to apply
- patch v2.6:
Hunk #1 FAILED at 1.
Hunk #2 FAILED at 23.
- patch v2.6.1:
can't find file to patch at input line 11 Perhaps you used the wrong -p or --strip option? [snip]
No file to patch. Skipping patch.
2 out of 2 hunks ignored
Patch failed! Please fix 0001-fix-makefile.patch!

Old versions of the tool "patch" cannot handle spaces in filenames. The same does not occur using "patch" v2.7 or any later.

Workaround: when a file with space in the name needs to be patched, one or two hooks must be used.
A pre-patch or post-extract hook renames the file to replace spaces with underscores.
The patch file must be generated using diff between two source-trees that have the file renamed with spaces replaced by underscores. A post-patch hook could rename the file to its original name if needed.

Fixes:
http://autobuild.buildroot.net/results/8ff/8ff91ab8e52000eb34dd8f662520cf1b31490cf5/
http://autobuild.buildroot.net/results/ea7/ea77d6b23aca0cb1cf527e6c16ddf5eba957a69c/

Cc Bernd Kuhls <[email protected]>
Cc Thomas Petazzoni <[email protected]>
Cc Arnout Vandecappelle (Essensium/Mind) <[email protected]>

---
Changes v2 -> v3:
- remove quotes (Arnout) (I missed the same suggestion from Thomas P)
- say "post-extract" instead of "POST_EXTRACT"
- prefer quotes rather than backslash in the rename hook (Arnout)
- there is no strong argument to use post-extract instead of pre-patch (Arnout)
- fix minor spelling mistakes

Changes v1 -> v2:
- use 1 or 2 hooks to rename the file that contains spaces (based on the suggestion to use a symlink from Thomas DS)
- change the patch file to be applied on the renamed file
- do not use the 2nd rename hook for libsoil (Thomas P)

Symlink and hard link cannot be used because patch unlinks them. So I renamed the file before patching it.

I hand-edited the patch, but it could also be generated following the procedure added to the manual by [PATCH v3 2/2]

I tested by inspection of the logs produced by:
for V in 2.5 2.5.4 2.5.9 2.6 2.6.1 2.7 2.7.1 2.7.2 2.7.3 2.7.4 2.7.5 ; do \
rm -rf build/libsoil-20080707/ ; \
PATH=/home/ricardo/src/patch-$V:/home/ricardo/src/patch-$V/src:$PATH patch -v | tee loghook-$V ; \
PATH=/home/ricardo/src/patch-$V:/home/ricardo/src/patch-$V/src:$PATH make libsoil-patch 2>&1 | tee -a loghook-$V ; \
grep -H fPIC build/libsoil-20080707/projects/makefile/alternate* | tee -a loghook-$V ; \
echo '----------' ; \
done

---
 package/libsoil/0001-fix-makefile.patch | 6 +++---
 package/libsoil/libsoil.mk              | 8 +++++++-
 2 files changed, 10 insertions(+), 4 deletions(-)
package/libsoil/0001-fix-makefile.patch +3 -3
index 3b80048..310d264 100644
@@ -5,9 +5,9 @@ http://anonscm.debian.org/cgit/pkg-games/libsoil.git/tree/debian/patches/linking
 
 Signed-off-by: Bernd Kuhls <[email protected]>
 
-diff -uNr "soil.org/projects/makefile/alternate Makefile.txt" "soil/projects/makefile/alternate Makefile.txt"
---- "soil.org/projects/makefile/alternate Makefile.txt"	2008-07-07 18:13:28.000000000 +0200
-+++ "soil/projects/makefile/alternate Makefile.txt"	2015-11-07 11:15:04.140106336 +0100
+diff -uNr soil.org/projects/makefile/alternate_Makefile.txt soil/projects/makefile/alternate_Makefile.txt
+--- soil.org/projects/makefile/alternate_Makefile.txt	2008-07-07 18:13:28.000000000 +0200
++++ soil/projects/makefile/alternate_Makefile.txt	2015-11-07 11:15:04.140106336 +0100
 @@ -1,8 +1,8 @@
  MAKE = make
 -CC = gcc
package/libsoil/libsoil.mk +7 -1
index eb8c2ce..99a47f6 100644
@@ -11,13 +11,19 @@ LIBSOIL_INSTALL_STAGING = YES
 LIBSOIL_DEPENDENCIES = libgl
 LIBSOIL_LICENSE = Public Domain, MIT
 LIBSOIL_LICENSE_FILES = src/stb_image_aug.c src/image_helper.c
-LIBSOIL_MAKEFILE = "../projects/makefile/alternate Makefile.txt"
+LIBSOIL_MAKEFILE = ../projects/makefile/alternate_Makefile.txt
 
 define LIBSOIL_EXTRACT_CMDS
 	$(UNZIP) -d $(@D) $(DL_DIR)/$(LIBSOIL_SOURCE)
 	mv $(@D)/Simple\ OpenGL\ Image\ Library/* $(@D)
 endef
 
+define REMOVE_SPACE_FROM_FILENAME
+	cd $(@D)/projects/makefile/ && \
+		mv "alternate Makefile.txt" alternate_Makefile.txt
+endef
+LIBSOIL_POST_EXTRACT_HOOKS += REMOVE_SPACE_FROM_FILENAME
+
 define LIBSOIL_BUILD_CMDS
 	$(MAKE) $(TARGET_CONFIGURE_OPTS) -f $(LIBSOIL_MAKEFILE) \
 		-C $(@D)/src
Signature · 1 line
1.9.1
#2 lore raw

PATCH v3 1/2 package/libsoil: fix build using old versions of patch

Quoted 23 lines · Ricardo Martincoski

Well-formed patch fails to apply
- patch v2.6:
Hunk #1 FAILED at 1.
Hunk #2 FAILED at 23.
- patch v2.6.1:
can't find file to patch at input line 11 Perhaps you used the wrong -p or --strip option? [snip]
No file to patch. Skipping patch.
2 out of 2 hunks ignored
Patch failed! Please fix 0001-fix-makefile.patch!

Old versions of the tool "patch" cannot handle spaces in filenames. The same does not occur using "patch" v2.7 or any later.

Workaround: when a file with space in the name needs to be patched, one or two hooks must be used.
A pre-patch or post-extract hook renames the file to replace spaces with underscores.
The patch file must be generated using diff between two source-trees that have the file renamed with spaces replaced by underscores. A post-patch hook could rename the file to its original name if needed.

Quoted 8 lines

Fixes:
http://autobuild.buildroot.net/results/8ff/8ff91ab8e52000eb34dd8f662520cf1b31490cf5/
http://autobuild.buildroot.net/results/ea7/ea77d6b23aca0cb1cf527e6c16ddf5eba957a69c/

Cc Bernd Kuhls <[email protected]>
Cc Thomas Petazzoni <[email protected]>
Cc Arnout Vandecappelle (Essensium/Mind) <[email protected]>
Signature · 6 lines
Arnout Vandecappelle                          arnout at mind be
Senior Embedded Software Architect            +32-16-286500
Essensium/Mind                                http://www.mind.be
G.Geenslaan 9, 3001 Leuven, Belgium           BE 872 984 063 RPR Leuven
LinkedIn profile: http://www.linkedin.com/in/arnoutvandecappelle
GPG fingerprint:  7493 020B C7E3 8618 8DEC 222C 82EB F404 F9AC 0DDF
#3 lore raw

PATCH v3 1/2 package/libsoil: fix build using old versions of patch

In reply to Ricardo Martincoski

Dear Ricardo Martincoski,

Quoted 23 lines · Ricardo Martincoski

Well-formed patch fails to apply
- patch v2.6:
Hunk #1 FAILED at 1.
Hunk #2 FAILED at 23.
- patch v2.6.1:
can't find file to patch at input line 11 Perhaps you used the wrong -p or --strip option? [snip]
No file to patch. Skipping patch.
2 out of 2 hunks ignored
Patch failed! Please fix 0001-fix-makefile.patch!

Old versions of the tool "patch" cannot handle spaces in filenames. The same does not occur using "patch" v2.7 or any later.

Workaround: when a file with space in the name needs to be patched, one or two hooks must be used.
A pre-patch or post-extract hook renames the file to replace spaces with underscores.
The patch file must be generated using diff between two source-trees that have the file renamed with spaces replaced by underscores. A post-patch hook could rename the file to its original name if needed.

Quoted 8 lines

Fixes:
http://autobuild.buildroot.net/results/8ff/8ff91ab8e52000eb34dd8f662520cf1b31490cf5/
http://autobuild.buildroot.net/results/ea7/ea77d6b23aca0cb1cf527e6c16ddf5eba957a69c/

Cc Bernd Kuhls <[email protected]>
Cc Thomas Petazzoni <[email protected]>
Cc Arnout Vandecappelle (Essensium/Mind) <[email protected]>

Applied, thanks.

Thomas

Signature · 3 lines
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
#4 lore raw

PATCH v3 1/2 package/libsoil: fix build using old versions of patch

Quoted 31 lines
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line

"Thomas" == Thomas Petazzoni <[email protected]> writes:

Dear Ricardo Martincoski,

Quoted 27 lines · Ricardo Martincoski

Well-formed patch fails to apply
- patch v2.6:
Hunk #1 FAILED at 1.
Hunk #2 FAILED at 23.
- patch v2.6.1:
can't find file to patch at input line 11 Perhaps you used the wrong -p or --strip option? [snip]
No file to patch. Skipping patch.
2 out of 2 hunks ignored
Patch failed! Please fix 0001-fix-makefile.patch!

Old versions of the tool "patch" cannot handle spaces in filenames. The same does not occur using "patch" v2.7 or any later.

Workaround: when a file with space in the name needs to be patched, one or two hooks must be used.
A pre-patch or post-extract hook renames the file to replace spaces with underscores.
The patch file must be generated using diff between two source-trees that have the file renamed with spaces replaced by underscores. A post-patch hook could rename the file to its original name if needed.

Fixes:
http://autobuild.buildroot.net/results/8ff/8ff91ab8e52000eb34dd8f662520cf1b31490cf5/
http://autobuild.buildroot.net/results/ea7/ea77d6b23aca0cb1cf527e6c16ddf5eba957a69c/

Quoted 6 lines
Quoted 4 lines
Cc Bernd Kuhls <[email protected]>
Cc Thomas Petazzoni <[email protected]>
Cc Arnout Vandecappelle (Essensium/Mind) <[email protected]>

Applied, thanks.

I think we should add a comment in the .mk to explain why we're doing this odd rename. I'll do so now.

Signature · 1 line
Bye, Peter Korsgaard
#5 lore raw

PATCH v3 2/2 docs/manual: patches that change files with spaces in the name

In reply to Ricardo Martincoski

When a patch changes one or more files with spaces in the name, a workaround must be applied to ensure the build using old versions of patch in the host system.

Cc Thomas Petazzoni <[email protected]>
Cc Thomas De Schampheleire <[email protected]>
Cc "Yann E. MORIN" <[email protected]>
Cc Arnout Vandecappelle (Essensium/Mind) <[email protected]>

---
Changes v2 -> v3:
- say "post-extract" instead of "POST_EXTRACT"
- there is no strong argument to use post-extract instead of pre-patch (Arnout)
- Arnout suggested to drop the patch to the manual:
"I think that this situation is so exotic that it doesn't warrant a mention in the manual. The manual is already long and complicated enough."
I updated the manual for now because it is a separate patch that can be Rejected if others agree with Arnout.

Changes v1 -> v2:
- update the manual entry
- use 1 or 2 hooks to rename the file that contains spaces (based on the suggestion to use a symlink from Thomas DS)
- change the patch file to be applied on the renamed file

Symlink and hard link cannot be used because patch unlinks them. So I renamed the file before patching it.
The file can be renamed back when needed.

Probably this patch will need some rewording.
It's my first patch to the manual.
I tested using 'make manual-html'

---
 docs/manual/patch-policy.txt | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)
docs/manual/patch-policy.txt +16 -0
index 0b4604e..8e6b3ee 100644
@@ -134,6 +134,22 @@ AC_PROG_MAKE_SET
 +AM_CONDITIONAL([CXX_WORKS], [test "x$rw_cv_prog_cxx_works" = "xyes"])
 ---------------
 
+==== Patches that change files with spaces in the name
+
+When a patch changes one or more files with spaces in the name, a workaround
+must be applied to ensure the build using old versions of +patch+ in the
+host system.
+
+. Add a pre-patch or a post-extract hook to rename the file to be patched,
+  replacing spaces with underscores;
+
+. Create the patch file using the usual diff command, but both
+  +package-version.orig/+ and +package-version/+ must have the file renamed
+  to replace the spaces with underscores;
+
+. If the file must keep its name, add a post-patch hook to rename the patched
+  file to its original name;
+
 === Integrating patches found on the Web
 
 When integrating a patch of which you are not the author, you have to
Signature · 1 line
1.9.1
#6 lore raw

PATCH v3 2/2 docs/manual: patches that change files with spaces in the name

Ricardo, All,

On 2016-01-24 11:19 -0200, Ricardo Martincoski spake thusly:

Quoted 9 lines

When a patch changes one or more files with spaces in the name, a workaround must be applied to ensure the build using old versions of patch in the host system.

Cc Thomas Petazzoni <[email protected]>
Cc Thomas De Schampheleire <[email protected]>
Cc "Yann E. MORIN" <[email protected]>
Cc Arnout Vandecappelle (Essensium/Mind) <[email protected]>
Quoted 11 lines

---
Changes v2 -> v3:
- say "post-extract" instead of "POST_EXTRACT"
- there is no strong argument to use post-extract instead of pre-patch (Arnout)
- Arnout suggested to drop the patch to the manual:
"I think that this situation is so exotic that it doesn't warrant a mention in the manual. The manual is already long and complicated enough."
I updated the manual for now because it is a separate patch that can be Rejected if others agree with Arnout.

Well, like Arnout said, the number of cases that this would bite us is very limited (as in: there is only one case).

The most problematic issue here is not doing the fix, but recognising the symptoms. This is such a rare occurence that I'm afraid we'll again scratch our heads when we next encounter this issue. That is, if there's ever a "next time"...

So I don't think it is necessary that the manual has a blurb about that.

Regards,
Yann E. MORIN.

Quoted 47 lines

Changes v1 -> v2:
- update the manual entry
- use 1 or 2 hooks to rename the file that contains spaces (based on the suggestion to use a symlink from Thomas DS)
- change the patch file to be applied on the renamed file

Symlink and hard link cannot be used because patch unlinks them. So I renamed the file before patching it.
The file can be renamed back when needed.

Probably this patch will need some rewording.
It's my first patch to the manual.
I tested using 'make manual-html'

---
 docs/manual/patch-policy.txt | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)
docs/manual/patch-policy.txt +16 -0
index 0b4604e..8e6b3ee 100644
@@ -134,6 +134,22 @@ AC_PROG_MAKE_SET
 +AM_CONDITIONAL([CXX_WORKS], [test "x$rw_cv_prog_cxx_works" = "xyes"])
 ---------------
 
+==== Patches that change files with spaces in the name
+
+When a patch changes one or more files with spaces in the name, a workaround
+must be applied to ensure the build using old versions of +patch+ in the
+host system.
+
+. Add a pre-patch or a post-extract hook to rename the file to be patched,
+  replacing spaces with underscores;
+
+. Create the patch file using the usual diff command, but both
+  +package-version.orig/+ and +package-version/+ must have the file renamed
+  to replace the spaces with underscores;
+
+. If the file must keep its name, add a post-patch hook to rename the patched
+  file to its original name;
+
 === Integrating patches found on the Web
 
 When integrating a patch of which you are not the author, you have to
Signature · 1 line
1.9.1
Signature · 6 lines
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 223 225 172 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'
#7 lore raw

PATCH v3 2/2 docs/manual: patches that change files with spaces in the name

Arnout, Yann E. MORIN,

On Sun, Jan 24, 2016 at 02:20 PM, Yann E. MORIN <[email protected]> wrote:
[snip]

Quoted 10 lines
Quoted 6 lines

- Arnout suggested to drop the patch to the manual:
"I think that this situation is so exotic that it doesn't warrant a mention in the manual. The manual is already long and complicated enough."
I updated the manual for now because it is a separate patch that can be Rejected if others agree with Arnout.

Well, like Arnout said, the number of cases that this would bite us is very limited (as in: there is only one case).

Quoted 6 lines

The most problematic issue here is not doing the fix, but recognising the symptoms. This is such a rare occurence that I'm afraid we'll again scratch our heads when we next encounter this issue. That is, if there's ever a "next time"...

So I don't think it is necessary that the manual has a blurb about that.

[snip]

Thank you both for your time.
I will set this patch as Rejected.

All,

I guess I don't need to send a v4 for the series (now it became a single patch) while no change is requested in
http://patchwork.ozlabs.org/patch/572263/
If someone disagrees, just let me know and I will send it.

Regards,
Ricardo

#8 lore raw

PATCH v3 2/2 docs/manual: patches that change files with spaces in the name

Ricardo, All,

On 2016-01-24 18:48 -0200, Ricardo Martincoski spake thusly:

Quoted 25 lines

Arnout, Yann E. MORIN,

On Sun, Jan 24, 2016 at 02:20 PM, Yann E. MORIN <[email protected]> wrote:
[snip]

Quoted 10 lines
Quoted 6 lines

- Arnout suggested to drop the patch to the manual:
"I think that this situation is so exotic that it doesn't warrant a mention in the manual. The manual is already long and complicated enough."
I updated the manual for now because it is a separate patch that can be Rejected if others agree with Arnout.

Well, like Arnout said, the number of cases that this would bite us is very limited (as in: there is only one case).

Quoted 6 lines

The most problematic issue here is not doing the fix, but recognising the symptoms. This is such a rare occurence that I'm afraid we'll again scratch our heads when we next encounter this issue. That is, if there's ever a "next time"...

So I don't think it is necessary that the manual has a blurb about that.

[snip]

Thank you both for your time.
I will set this patch as Rejected.

Quoted 6 lines

All,

I guess I don't need to send a v4 for the series (now it became a single patch) while no change is requested in
http://patchwork.ozlabs.org/patch/572263/
If someone disagrees, just let me know and I will send it.

Indeed, no need to resubmit.

Thanmks! :-)

Regards,
Yann E. MORIN.

Signature · 6 lines
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 223 225 172 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'