[Buildroot] [PATCH v2 0/2] fix build using old versions of patch

[Buildroot] [PATCH v2 0/2] fix build using old versions of patch

[Ricardo Martincoski]

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            | 17 +++++++++++++++++
 package/libsoil/0001-fix-makefile.patch |  6 +++---
 package/libsoil/libsoil.mk              |  8 +++++++-
 3 files changed, 27 insertions(+), 4 deletions(-)

--- 
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)
-- 
1.9.1

[Buildroot] [PATCH v2 1/2] package/libsoil: fix build using old versions of patch

[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?
[snap]
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 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/

Signed-off-by: Ricardo Martincoski <ricardo.martincoski@gmail.com>
Cc: Bernd Kuhls <bernd.kuhls@t-online.de>
Cc: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
---
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 used POST_EXTRACT instead of PRE_PATCH because some developer could
use 'make package-extract', create a copy of the extracted directory,
edit the needed files and then use diff to create the patch.

I hand-edited the patch, but it could also be generated following
the procedure added to the manual by [PATCH v2 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(-)

diff --git a/package/libsoil/0001-fix-makefile.patch b/package/libsoil/0001-fix-makefile.patch
index 3b80048..310d264 100644
--- a/package/libsoil/0001-fix-makefile.patch
+++ b/package/libsoil/0001-fix-makefile.patch
@@ -5,9 +5,9 @@ http://anonscm.debian.org/cgit/pkg-games/libsoil.git/tree/debian/patches/linking
 
 Signed-off-by: Bernd Kuhls <bernd.kuhls@t-online.de>
 
-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
diff --git a/package/libsoil/libsoil.mk b/package/libsoil/libsoil.mk
index eb8c2ce..9ef0498 100644
--- a/package/libsoil/libsoil.mk
+++ b/package/libsoil/libsoil.mk
@@ -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
-- 
1.9.1

[Buildroot] [PATCH v2 2/2] docs/manual: patches that change files with spaces in the name

[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.

Signed-off-by: Ricardo Martincoski <ricardo.martincoski@gmail.com>
Cc: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
Cc: Thomas De Schampheleire <patrickdepinguin@gmail.com>
Cc: "Yann E. MORIN" <yann.morin.1998@free.fr>
---
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 | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/docs/manual/patch-policy.txt b/docs/manual/patch-policy.txt
index 0b4604e..471ded5 100644
--- a/docs/manual/patch-policy.txt
+++ b/docs/manual/patch-policy.txt
@@ -134,6 +134,23 @@ 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 hook to +<packagename>_POST_EXTRACT_HOOKS+ that renames the file to be
+  patched, replacing spaces with underscores;
+
+. Create the patch 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 hook to
+  +<packagename>_POST_PATCH_HOOKS+ that renames 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
-- 
1.9.1

[Buildroot] [PATCH v2 1/2] package/libsoil: fix build using old versions of patch

[Arnout Vandecappelle]

On 24-01-16 00:53, Ricardo Martincoski wrote:
> 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?
> [snap]
> 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 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.

 My first reaction was: this is going to be too complicated. But now I see it,
it's not so bad.


 However, I have a few small comments.

> 
> Fixes:
> http://autobuild.buildroot.net/results/8ff/8ff91ab8e52000eb34dd8f662520cf1b31490cf5/
> http://autobuild.buildroot.net/results/ea7/ea77d6b23aca0cb1cf527e6c16ddf5eba957a69c/
> 
> Signed-off-by: Ricardo Martincoski <ricardo.martincoski@gmail.com>
> Cc: Bernd Kuhls <bernd.kuhls@t-online.de>
> Cc: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
> ---
> 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 used POST_EXTRACT instead of PRE_PATCH because some developer could
> use 'make package-extract', create a copy of the extracted directory,
> edit the needed files and then use diff to create the patch.

 You should always use package-patch before creating a patch for a package,
because you have to be sure that it still applies together with all other patches.

 That said, I think post extract is in this case a bit cleaner.

> 
> I hand-edited the patch, but it could also be generated following
> the procedure added to the manual by [PATCH v2 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
> ---
[snip]
> diff --git a/package/libsoil/libsoil.mk b/package/libsoil/libsoil.mk
> index eb8c2ce..9ef0498 100644
> --- a/package/libsoil/libsoil.mk
> +++ b/package/libsoil/libsoil.mk
> @@ -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"

 The quotes could be removed now.

>  
>  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

 Here I would prefer quotes rather than backslash. It is not at all obvious that
make will not interpret the backslash.


 Regards,
 Arnout

> +endef
> +LIBSOIL_POST_EXTRACT_HOOKS += REMOVE_SPACE_FROM_FILENAME
> +
>  define LIBSOIL_BUILD_CMDS
>  	$(MAKE) $(TARGET_CONFIGURE_OPTS) -f $(LIBSOIL_MAKEFILE) \
>  		-C $(@D)/src
> 


-- 
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

[Buildroot] [PATCH v2 2/2] docs/manual: patches that change files with spaces in the name

[Arnout Vandecappelle]

On 24-01-16 00:53, Ricardo Martincoski wrote:
> 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.

 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.

 Regards,
 Arnout

> 
> Signed-off-by: Ricardo Martincoski <ricardo.martincoski@gmail.com>
> Cc: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
> Cc: Thomas De Schampheleire <patrickdepinguin@gmail.com>
> Cc: "Yann E. MORIN" <yann.morin.1998@free.fr>
> ---
> 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 | 17 +++++++++++++++++
>  1 file changed, 17 insertions(+)
> 
> diff --git a/docs/manual/patch-policy.txt b/docs/manual/patch-policy.txt
> index 0b4604e..471ded5 100644
> --- a/docs/manual/patch-policy.txt
> +++ b/docs/manual/patch-policy.txt
> @@ -134,6 +134,23 @@ 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 hook to +<packagename>_POST_EXTRACT_HOOKS+ that renames the file to be
> +  patched, replacing spaces with underscores;
> +
> +. Create the patch 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 hook to
> +  +<packagename>_POST_PATCH_HOOKS+ that renames 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
> 


-- 
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