diff mbox series

[RFC,v4,05/19] Makefile: use "generate-perl.sh" to massage Perl library

Message ID eddafe1cf8935fd25d107645168ace3f65e1064c.1729771605.git.ps@pks.im (mailing list archive)
State Superseded
Headers show
Series Modernize the build system | expand

Commit Message

Patrick Steinhardt Oct. 24, 2024, 12:39 p.m. UTC 
Extend "generate-perl.sh" such that it knows to also massage the Perl
library files. There are two major differences:

  - We do not read in the Perl header. This is handled by matching on
    whether or not we have a Perl shebang.

  - We substitute some more variables, which we read in via our
    GIT-BUILD-OPTIONS.

Adapt both our Makefile and the CMake build instructions to use this.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 GIT-BUILD-OPTIONS.in                |  2 ++
 Makefile                            | 10 ++++------
 contrib/buildsystems/CMakeLists.txt | 20 ++++++--------------
 generate-perl.sh                    | 14 ++++++++++++--
 4 files changed, 24 insertions(+), 22 deletions(-)

Comments

Phillip Wood Nov. 11, 2024, 10:53 a.m. UTC | #1 
Hi Patrick

On 24/10/2024 13:39, Patrick Steinhardt wrote:
> diff --git a/generate-perl.sh b/generate-perl.sh
> index 12e116b76e5..cb1629857c6 100755
> --- a/generate-perl.sh
> +++ b/generate-perl.sh
> @@ -17,10 +17,20 @@ OUTPUT="$5"
>   . "$GIT_BUILD_OPTIONS"

I need to add

case "$OUTPUT" in
*.pm)
	dir="$(dirname $OUTPUT)"
	if ! test -d "$dir"
	then
		mkdir -p "$dir"
	fi
     ;;
esac

to create the output directories when building out of tree using CMake 
on Linux. I'm not sure why it works on Windows without this, or why we 
don't need to create the leading directories when generating 
clar-decls.h or clar.suite as CMake seems to do it for us when it 
initializes the build directory.

>   sed -e '1{' \
> +    -e "	/^#!.*perl/!b" \
>       -e "	s|#!.*perl|#!$PERL_PATH|" \
>       -e "	r $PERL_HEADER" \
>       -e '	G' \
>       -e '}' \
> -    -e "s/@GIT_VERSION@/$GIT_VERSION/g" \
> +    -e "s|@GIT_VERSION@|$GIT_VERSION|g" \
> +    -e "s|@LOCALEDIR@|$PERL_LOCALEDIR|g" \
> +    -e "s|@NO_GETTEXT@|$NO_GETTEXT|g" \
> +    -e "s|@NO_PERL_CPAN_FALLBACKS@|$NO_PERL_CPAN_FALLBACKS|g" \
>       "$INPUT" >"$OUTPUT"
> -chmod a+x "$OUTPUT"
> +
> +case "$(basename "$INPUT")" in

Nit: there's no need to call "basename" here as "*" will match directory 
separators in a case statement.

Best Wishes

Phillip

> +*.perl)
> +	chmod a+x "$OUTPUT";;
> +*)
> +	;;
> +esac
Patrick Steinhardt Nov. 11, 2024, 2:13 p.m. UTC | #2 
On Mon, Nov 11, 2024 at 10:53:58AM +0000, Phillip Wood wrote:
> Hi Patrick
> 
> On 24/10/2024 13:39, Patrick Steinhardt wrote:
> > diff --git a/generate-perl.sh b/generate-perl.sh
> > index 12e116b76e5..cb1629857c6 100755
> > --- a/generate-perl.sh
> > +++ b/generate-perl.sh
> > @@ -17,10 +17,20 @@ OUTPUT="$5"
> >   . "$GIT_BUILD_OPTIONS"
> 
> I need to add
> 
> case "$OUTPUT" in
> *.pm)
> 	dir="$(dirname $OUTPUT)"
> 	if ! test -d "$dir"
> 	then
> 		mkdir -p "$dir"
> 	fi
>     ;;
> esac
> 
> to create the output directories when building out of tree using CMake on
> Linux. I'm not sure why it works on Windows without this, or why we don't
> need to create the leading directories when generating clar-decls.h or
> clar.suite as CMake seems to do it for us when it initializes the build
> directory.

Ah, indeed, I can reproduce this when using Makefiles. I'll solve this
via `file(MAKE_DIRECTORY)`.

Patrick
Paul Smith Nov. 11, 2024, 2:18 p.m. UTC | #3 
On Mon, 2024-11-11 at 15:13 +0100, Patrick Steinhardt wrote:
> > case "$OUTPUT" in
> > *.pm)
> >   dir="$(dirname $OUTPUT)"
> >   if ! test -d "$dir"
> >   then
> >   mkdir -p "$dir"
> >   fi
> >      ;;
> > esac

This seems kind of complicated to me.  Why not just simply:

  case "$OUTPUT" in
    (*.pm) mkdir -p "$(dirname $OUTPUT)" ;;
  esac

There's no point in testing for existence of the directory before
invoking mkdir -p IMO.

Maybe the git environment has some rules about this?
Patrick Steinhardt Nov. 11, 2024, 2:48 p.m. UTC | #4 
On Mon, Nov 11, 2024 at 09:18:35AM -0500, Paul Smith wrote:
> On Mon, 2024-11-11 at 15:13 +0100, Patrick Steinhardt wrote:
> > > case "$OUTPUT" in
> > > *.pm)
> > >   dir="$(dirname $OUTPUT)"
> > >   if ! test -d "$dir"
> > >   then
> > >   mkdir -p "$dir"
> > >   fi
> > >      ;;
> > > esac
> 
> This seems kind of complicated to me.  Why not just simply:
> 
>   case "$OUTPUT" in
>     (*.pm) mkdir -p "$(dirname $OUTPUT)" ;;
>   esac
> 
> There's no point in testing for existence of the directory before
> invoking mkdir -p IMO.
> 
> Maybe the git environment has some rules about this?

I won't use either of these. Our Makefile already knows to create the
parent directory anyway:

    perl/build/lib/%.pm: perl/%.pm generate-perl.sh GIT-BUILD-OPTIONS GIT-PERL-DEFINES
        $(call mkdir_p_parent_template)
        $(QUIET_GEN)$(SHELL_PATH) generate-perl.sh ./GIT-BUILD-OPTIONS $(GIT_VERSION) GIT-PERL-HEADER "$<" "$@"

The issue is rather that CMake didn't know to do the same, so I'm
solving this in CMake rather than in the script.

Patrick
diff mbox series

Patch

diff --git a/GIT-BUILD-OPTIONS.in b/GIT-BUILD-OPTIONS.in
index f0ca240493c..050432f9fc4 100644
--- a/GIT-BUILD-OPTIONS.in
+++ b/GIT-BUILD-OPTIONS.in
@@ -1,6 +1,8 @@ 
 SHELL_PATH=@SHELL_PATH@
 TEST_SHELL_PATH=@TEST_SHELL_PATH@
 PERL_PATH=@PERL_PATH@
+PERL_LOCALEDIR=@PERL_LOCALEDIR@
+NO_PERL_CPAN_FALLBACKS=@NO_PERL_CPAN_FALLBACKS@
 DIFF=@DIFF@
 PYTHON_PATH=@PYTHON_PATH@
 TAR=@TAR@
diff --git a/Makefile b/Makefile
index e04a381e8f0..fc13d5bb01c 100644
--- a/Makefile
+++ b/Makefile
@@ -3093,13 +3093,9 @@  endif
 NO_PERL_CPAN_FALLBACKS_SQ = $(subst ','\'',$(NO_PERL_CPAN_FALLBACKS))
 endif
 
-perl/build/lib/%.pm: perl/%.pm GIT-PERL-DEFINES
+perl/build/lib/%.pm: perl/%.pm generate-perl.sh GIT-BUILD-OPTIONS GIT-PERL-DEFINES
 	$(call mkdir_p_parent_template)
-	$(QUIET_GEN) \
-	sed -e 's|@LOCALEDIR@|$(perl_localedir_SQ)|g' \
-	    -e 's|@NO_GETTEXT@|$(NO_GETTEXT_SQ)|g' \
-	    -e 's|@NO_PERL_CPAN_FALLBACKS@|$(NO_PERL_CPAN_FALLBACKS_SQ)|g' \
-	< $< > $@
+	$(QUIET_GEN)$(SHELL_PATH) generate-perl.sh ./GIT-BUILD-OPTIONS $(GIT_VERSION) GIT-PERL-HEADER "$<" "$@"
 
 perl/build/man/man3/Git.3pm: perl/Git.pm
 	$(call mkdir_p_parent_template)
@@ -3160,6 +3156,8 @@  GIT-BUILD-OPTIONS: FORCE
 		-e "s|@SHELL_PATH@|\'$(SHELL_PATH_SQ)\'|" \
 		-e "s|@TEST_SHELL_PATH@|\'$(TEST_SHELL_PATH_SQ)\'|" \
 		-e "s|@PERL_PATH@|\'$(PERL_PATH_SQ)\'|" \
+		-e "s|@PERL_LOCALEDIR@|\'$(perl_localedir_SQ)\'|" \
+		-e "s|@NO_PERL_CPAN_FALLBACKS@|\'$(NO_PERL_CPAN_FALLBACKS_SQ)\'|" \
 		-e "s|@DIFF@|\'$(DIFF)\'|" \
 		-e "s|@PYTHON_PATH@|\'$(PYTHON_PATH_SQ)\'|" \
 		-e "s|@TAR@|\'$(TAR)\'|" \
diff --git a/contrib/buildsystems/CMakeLists.txt b/contrib/buildsystems/CMakeLists.txt
index 7fb6a149f21..ddf39dc90e7 100644
--- a/contrib/buildsystems/CMakeLists.txt
+++ b/contrib/buildsystems/CMakeLists.txt
@@ -849,6 +849,9 @@  endforeach()
 
 #perl scripts
 parse_makefile_for_scripts(git_perl_scripts "SCRIPT_PERL" "")
+#perl modules
+file(GLOB_RECURSE perl_modules "${CMAKE_SOURCE_DIR}/perl/*.pm")
+list(TRANSFORM perl_modules REPLACE "${CMAKE_SOURCE_DIR}/" "")
 
 #create perl header
 file(STRINGS ${CMAKE_SOURCE_DIR}/perl/header_templates/fixed_prefix.template.pl perl_header )
@@ -856,7 +859,7 @@  string(REPLACE "@PATHSEP@" ":" perl_header "${perl_header}")
 string(REPLACE "@INSTLIBDIR@" "${INSTLIBDIR}" perl_header "${perl_header}")
 file(WRITE ${CMAKE_BINARY_DIR}/PERL-HEADER ${perl_header})
 
-foreach(script ${git_perl_scripts})
+foreach(script ${git_perl_scripts} ${perl_modules})
 	string(REPLACE ".perl" "" perl_gen_path "${script}")
 
 	add_custom_command(OUTPUT ${CMAKE_BINARY_DIR}/${perl_gen_path}
@@ -877,19 +880,6 @@  file(STRINGS ${CMAKE_SOURCE_DIR}/git-p4.py content NEWLINE_CONSUME)
 string(REPLACE "#!/usr/bin/env python" "#!/usr/bin/python" content "${content}")
 file(WRITE ${CMAKE_BINARY_DIR}/git-p4 ${content})
 
-#perl modules
-file(GLOB_RECURSE perl_modules "${CMAKE_SOURCE_DIR}/perl/*.pm")
-
-foreach(pm ${perl_modules})
-	string(REPLACE "${CMAKE_SOURCE_DIR}/perl/" "" file_path ${pm})
-	file(STRINGS ${pm} content NEWLINE_CONSUME)
-	string(REPLACE "@LOCALEDIR@" "${LOCALEDIR}" content "${content}")
-	string(REPLACE "@NO_PERL_CPAN_FALLBACKS@" "" content "${content}")
-	file(WRITE ${CMAKE_BINARY_DIR}/perl/build/lib/${file_path} ${content})
-#test-lib.sh requires perl/build/lib to be the build directory of perl modules
-endforeach()
-
-
 #templates
 file(GLOB templates "${CMAKE_SOURCE_DIR}/templates/*")
 list(TRANSFORM templates REPLACE "${CMAKE_SOURCE_DIR}/templates/" "")
@@ -1131,6 +1121,8 @@  file(STRINGS ${CMAKE_SOURCE_DIR}/GIT-BUILD-OPTIONS.in git_build_options NEWLINE_
 string(REPLACE "@SHELL_PATH@" "${SHELL_PATH}" git_build_options "${git_build_options}")
 string(REPLACE "@TEST_SHELL_PATH@" "${TEST_SHELL_PATH}" git_build_options "${git_build_options}")
 string(REPLACE "@PERL_PATH@" "${PERL_PATH}" git_build_options "${git_build_options}")
+string(REPLACE "@PERL_LOCALEDIR@" "${LOCALEDIR}" git_build_options "${git_build_options}")
+string(REPLACE "@NO_PERL_CPAN_FALLBACKS@" "" git_build_options "${git_build_options}")
 string(REPLACE "@DIFF@" "${DIFF}" git_build_options "${git_build_options}")
 string(REPLACE "@PYTHON_PATH@" "${PYTHON_PATH}" git_build_options "${git_build_options}")
 string(REPLACE "@TAR@" "${TAR}" git_build_options "${git_build_options}")
diff --git a/generate-perl.sh b/generate-perl.sh
index 12e116b76e5..cb1629857c6 100755
--- a/generate-perl.sh
+++ b/generate-perl.sh
@@ -17,10 +17,20 @@  OUTPUT="$5"
 . "$GIT_BUILD_OPTIONS"
 
 sed -e '1{' \
+    -e "	/^#!.*perl/!b" \
     -e "	s|#!.*perl|#!$PERL_PATH|" \
     -e "	r $PERL_HEADER" \
     -e '	G' \
     -e '}' \
-    -e "s/@GIT_VERSION@/$GIT_VERSION/g" \
+    -e "s|@GIT_VERSION@|$GIT_VERSION|g" \
+    -e "s|@LOCALEDIR@|$PERL_LOCALEDIR|g" \
+    -e "s|@NO_GETTEXT@|$NO_GETTEXT|g" \
+    -e "s|@NO_PERL_CPAN_FALLBACKS@|$NO_PERL_CPAN_FALLBACKS|g" \
     "$INPUT" >"$OUTPUT"
-chmod a+x "$OUTPUT"
+
+case "$(basename "$INPUT")" in
+*.perl)
+	chmod a+x "$OUTPUT";;
+*)
+	;;
+esac