[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH 5/6] Don't leak developer GREP, SED etc into distribution fil
From: |
Ralf Wildenhues |
Subject: |
Re: [PATCH 5/6] Don't leak developer GREP, SED etc into distribution file. |
Date: |
Tue, 31 Aug 2010 20:12:02 +0200 |
User-agent: |
Mutt/1.5.20 (2010-04-22) |
* Gary V. Vaughan wrote on Tue, Aug 31, 2010 at 08:43:19AM CEST:
> From: Gary V. Vaughan <address@hidden>
>
> * bootstrap (rebuild): Set the shell variable `revision' rather
> than `correctver' for clarity.
> (edit): Split into two parts...
> (bootstrap_edit): ...substitutions that should happen at bootstrap
> time...
> (configure_edit): ...and substitution that should not happen until
> configure time.
> * bootstrap: Move the bootstrap related section up towards the top
> of the file.
> (libltdl/m4/ltversion.m4): Extract `$macro_revision' from this
> file for direct comparison with `$revision', rather than munging
> the file's serial number. Use `bootstrap_edit' for substitutions
> when generating from `ltversion.in'.
> (libltdl/config/ltmain.sh): Similarly, extract `$package_revision'
> for comparison with rebuild's `$revision' setting, and make
> bootstrap time substitutions with `bootstrap_edit'.
> (libtool): Likewise.
> (libtoolize): Use `configure_edit' for substitutions at configure
> time.
> (tests/package.m4, tests/defs): Likewise.
> * HACKING (Release Procedure): Remove the note to workaround the
> bug fixed by this changeset.
> * NEWS (Bug fixes): Mention that this bug is now fixed.
Please credit Joerg Sonnenberger for finding the bug. Thanks.
I usually do VPATH builds exclusively, except when trying out patches
where I think in-tree builds could be broken. In my VPATH tree, when I
make -f ../libtool/Makefile.main announce-gen SHELL='/bin/sh -x'
that contains the following output:
+ -e 's,@TIMESTAMP\@, 1.3257 2010-08-30,g' -e 's,@LASTRELEASE\@,2.2.11,g' -e
's,@top_srcdir\@,../libtool,g' announce-gen.in
/bin/sh: line 4: -e: command not found
Please, once and for all, whenever you rename some identifier,
use some method to verify that you've renamed all instances;
git grep '\<edit\>'
is quite a good approximation to the perfect method, and costs maybe a
couple of seconds on your part. Then submit that as separate patch.
When you do pure code move-arounds, that also is much easier to review
when done as a separate patch, without any actual changes to the text
that was moved. I remember that we took more than dozen iterations on
the makefile rules to get them halfway correct, I have no intentions of
going there again.
I have stopped reviewing at this point. I would gladly re-review a
minimal patch based on minimal prerequisites that has been properly
tested. The Makefile.am changes should be tested with both GNU and
non-GNU make, and both VPATH and in-tree.
> --- a/HACKING
> +++ b/HACKING
> @@ -620,15 +620,6 @@ or obtained by writing to the Free Software Foundation,
> Inc.,
>
> * Update NEWS, ChangeLog.
>
> -* Until the bug that leaks developer tool paths into the release tarballs
> - from ./bootstrap is fixed, make sure the right tools are first in your
> - PATH and then:
> - export EGREP=egrep
> - export FGREP=fgrep
> - export GREP=grep
> - export MAKE=make
> - export SED=sed
> -
> * Run ./bootstrap.
>
> * Run ./configure (or create a build directory first and run configure
> diff --git a/Makefile.am b/Makefile.am
> index de3eafe..f6c0a9a 100644
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -57,89 +57,13 @@ timestamp = set dummy `$(MKSTAMP) $(srcdir)`; shift; \
> *) TIMESTAMP="" ;; \
> esac
>
> -rebuild = rebuild=:; $(timestamp); correctver=$$1
> -
> -
> -## ---------------- ##
> -## Libtool scripts. ##
> -## ---------------- ##
> -
> -# The libtool distributor and the standalone libtool script.
> -bin_SCRIPTS = libtoolize libtool
> -
> -libtoolize: $(srcdir)/libtoolize.in $(top_builddir)/config.status
> - rm -f libtoolize.tmp libtoolize
> - $(timestamp); \
> - input="libtoolize.m4sh"; \
> - $(edit) -e "s,@TIMESTAMP\@,$$TIMESTAMP,g" \
> - -e 's,@aclocal_DATA\@,$(aclocalfiles),g' \
> - -e "s,@pkgltdl_files\@,$(ltdldatafiles),g" \
> - -e "s,@pkgconfig_files\@,$(auxfiles),g" \
> - $(srcdir)/libtoolize.in > libtoolize.tmp
> - chmod a+x libtoolize.tmp
> - chmod a-w libtoolize.tmp
> - mv -f libtoolize.tmp libtoolize
> -
> -# Use `$(srcdir)' for the benefit of non-GNU makes: this is
> -# how libtoolize.in appears in our dependencies.
> -EXTRA_DIST += libtoolize.m4sh
> -$(srcdir)/libtoolize.in: $(sh_files) libtoolize.m4sh Makefile.am
> - cd $(srcdir); \
> - rm -f libtoolize.in; \
> - $(M4SH) -B $(auxdir) libtoolize.m4sh > libtoolize.in
> -
> -# We used to do this with a 'stamp-vcl' file, but non-gmake builds
> -# would rerun configure on every invocation, so now we manually
> -# check the version numbers from the build rule when necessary.
> -libtool: $(top_builddir)/config.status $(srcdir)/$(auxdir)/ltmain.sh
> ChangeLog
> - @target=libtool; $(rebuild); \
> - if test -f "$$target"; then \
> - set dummy `./$$target --version | sed 1q`; actualver="$$5"; \
> - test "$$actualver" = "$$correctver" && rebuild=false; \
> - fi; \
> - for prereq in $?; do \
> - case $$prereq in *ChangeLog);; *) rebuild=:;; esac; \
> - done; \
> - if $$rebuild; then \
> - echo $(SHELL) ./config.status $$target; \
> - cd $(top_builddir) && $(SHELL) ./config.status $$target; \
> - fi
> -
> -.PHONY: configure-subdirs
> -configure-subdirs distdir: $(DIST_MAKEFILE_LIST)
> address@hidden@:
> - dir=`echo $@ | sed 's,^[^/]*$$,.,;s,/[^/]*$$,,'`; \
> - test -d $$dir || mkdir $$dir || exit 1; \
> - abs_srcdir=`$(lt__cd) $(srcdir) && pwd`; \
> - (cd $$dir && $$abs_srcdir/$$dir/configure --with-dist) || exit 1
> +rebuild = rebuild=:; $(timestamp); revision=$$1
>
>
> # ---------- #
> # Bootstrap. #
> # ---------- #
>
> -edit = sed \
> - -e 's,@EGREP\@,$(EGREP),g' \
> - -e 's,@FGREP\@,$(FGREP),g' \
> - -e 's,@GREP\@,$(GREP),g' \
> - -e 's,@LN_S\@,$(LN_S),g' \
> - -e 's,@MACRO_VERSION\@,$(VERSION),g' \
> - -e 's,@PACKAGE\@,$(PACKAGE),g' \
> - -e 's,@PACKAGE_BUGREPORT\@,$(PACKAGE_BUGREPORT),g' \
> - -e 's,@PACKAGE_URL\@,$(PACKAGE_URL),g' \
> - -e 's,@PACKAGE_NAME\@,$(PACKAGE_NAME),g' \
> - -e 's,@PACKAGE_STRING\@,$(PACKAGE_NAME) $(VERSION),g' \
> - -e 's,@PACKAGE_TARNAME\@,$(PACKAGE),g' \
> - -e 's,@PACKAGE_VERSION\@,$(VERSION),g' \
> - -e 's,@SED\@,$(SED),g' \
> - -e 's,@VERSION\@,$(VERSION),g' \
> - -e 's,@aclocaldir\@,$(aclocaldir),g' \
> - -e 's,@datadir\@,$(datadir),g' \
> - -e 's,@pkgdatadir\@,$(pkgdatadir),g' \
> - -e 's,@host_triplet\@,$(host_triplet),g' \
> - -e 's,@prefix\@,$(prefix),g' \
> - -e "s,@configure_input\@,Generated from $$input.,g"
> -
> sh_files = $(auxdir)/general.m4sh $(auxdir)/getopt.m4sh
> EXTRA_DIST += bootstrap $(srcdir)/libtoolize.in $(auxdir)/ltmain.m4sh \
> $(auxdir)/mkstamp $(sh_files) \
> @@ -151,6 +75,25 @@ EXTRA_DIST += bootstrap $(srcdir)/libtoolize.in
> $(auxdir)/ltmain.m4sh \
> CLEANFILES += libtool libtoolize libtoolize.tmp \
> $(auxdir)/ltmain.tmp $(m4dir)/ltversion.tmp
>
> +## These are the replacements that need to be made at bootstrap time,
> +## because they must be static in distributed files, and not accidentally
> +## changed by configure running on the build machine.
> +bootstrap_edit = sed \
> + -e "s,@configure_input\@,Generated from $$input.,g" \
> + -e 's,@MACRO_VERSION\@,$(VERSION),g' \
> + -e "s,@MACRO_REVISION\@,$$revision,g" \
> + -e "s,@MACRO_SERIAL\@,$$serial,g" \
> + -e 's,@PACKAGE\@,$(PACKAGE),g' \
> + -e 's,@PACKAGE_BUGREPORT\@,$(PACKAGE_BUGREPORT),g' \
> + -e 's,@PACKAGE_URL\@,$(PACKAGE_URL),g' \
> + -e 's,@PACKAGE_NAME\@,$(PACKAGE_NAME),g' \
> + -e "s,@package_revision\@,$$revision,g" \
> + -e 's,@PACKAGE_STRING\@,$(PACKAGE_NAME) $(VERSION),g' \
> + -e 's,@PACKAGE_TARNAME\@,$(PACKAGE),g' \
> + -e 's,@PACKAGE_VERSION\@,$(VERSION),g' \
> + -e "s,@TIMESTAMP\@,$$TIMESTAMP,g" \
> + -e 's,@VERSION\@,$(VERSION),g'
> +
> ## We build ltversion.m4 here, instead of from config.status,
> ## because config.status is rerun each time one of configure's
> ## dependencies change and ltversion.m4 happens to be a configure
> @@ -159,15 +102,14 @@ CLEANFILES += libtool libtoolize libtoolize.tmp \
> ## We used to do this with a 'stamp-vcl' file, but non-gmake builds
> ## would rerun configure on every invocation, so now we manually
> ## check the version numbers from the build rule when necessary.
> -## Use `$(srcdir)/m4' for the benefit of non-GNU makes: this is
> +## Use `$(srcdir)/$(m4dir)' for the benefit of non-GNU makes: this is
> ## how ltversion.m4 appears in our dependencies.
> EXTRA_DIST += $(m4dir)/ltversion.in $(srcdir)/$(m4dir)/ltversion.m4
> $(srcdir)/$(m4dir)/ltversion.m4: $(m4dir)/ltversion.in configure.ac ChangeLog
> @target='$(srcdir)/$(m4dir)/ltversion.m4'; $(rebuild); \
> if test -f "$$target"; then \
> - set dummy `sed -n '/^# serial /p' "$$target"`; shift; \
> - actualver=1.$$3; \
> - test "$$actualver" = "$$correctver" && rebuild=false; \
> + eval `sed -n '/^macro_revision=/p' "$$target"`; \
> + test "$$macro_revision" = "$$revision" && rebuild=false; \
> fi; \
> for prereq in $?; do \
> case $$prereq in *ChangeLog | *configure.ac);; *) rebuild=:;; esac; \
> @@ -175,14 +117,12 @@ $(srcdir)/$(m4dir)/ltversion.m4: $(m4dir)/ltversion.in
> configure.ac ChangeLog
> if $$rebuild; then \
> cd $(srcdir); \
> rm -f $(m4dir)/ltversion.tmp; \
> - serial=`echo "$$correctver" | sed 's,^1[.],,g'`; \
> + serial=`echo "$$revision" | sed 's,^[1-9][0-9]*[.],,g'`; \
> input="ltversion.in"; \
> - echo $(edit) -e "s,@MACRO_REVISION\@,$$correctver,g" \
> - -e "s,@MACRO_SERIAL\@,$$serial,g" \
> + echo $(bootstrap_edit) \
> $(srcdir)/$(m4dir)/ltversion.in \> $(srcdir)/$(m4dir)/ltversion.m4;
> \
> - $(edit) -e "s,@MACRO_REVISION\@,$$correctver,g" \
> - -e "s,@MACRO_SERIAL\@,$$serial,g" \
> - $(m4dir)/ltversion.in > $(m4dir)/ltversion.tmp; \
> + $(bootstrap_edit) \
> + $(m4dir)/ltversion.in > $(m4dir)/ltversion.tmp; \
> chmod a-w $(m4dir)/ltversion.tmp; \
> mv -f $(m4dir)/ltversion.tmp $(m4dir)/ltversion.m4; \
> fi
> @@ -192,7 +132,7 @@ $(srcdir)/$(m4dir)/ltversion.m4: $(m4dir)/ltversion.in
> configure.ac ChangeLog
> ## would rerun configure on every invocation, so now we manually
> ## check the version numbers from the build rule when necessary.
> ## !WARNING! If you edit this rule to change the contents of ltmain.sh,
> -## you must `touch $(srcdir)/$(auxdir)/ltmain.in' from the
> +## you must `touch $(srcdir)/$(auxdir)/ltmain.m4sh' from the
> ## shell if you need ltmain.sh to be regenerated. Ideally, we
> ## should make this rule depend on Makefile but that will break
> ## distcheck (at least) by rebuilding ltmain.sh in the source
> @@ -202,8 +142,7 @@ $(srcdir)/$(auxdir)/ltmain.sh: $(sh_files)
> $(auxdir)/ltmain.m4sh configure.ac Ch
> @target='$(srcdir)/$(auxdir)/ltmain.sh'; $(rebuild); \
> if test -f "$$target"; then \
> eval `sed -n '/^package_revision=/p' "$$target"`; \
> - actualver=$$package_revision; \
> - test "$$actualver" = "$$correctver" && rebuild=false; \
> + test "$$package_revision" = "$$revision" && rebuild=false; \
> fi; \
> for prereq in $?; do \
> case $$prereq in *ChangeLog);; *) rebuild=:;; esac; \
> @@ -217,12 +156,10 @@ $(srcdir)/$(auxdir)/ltmain.sh: $(sh_files)
> $(auxdir)/ltmain.m4sh configure.ac Ch
> $(M4SH) -B $(auxdir) $(auxdir)/ltmain.m4sh \
> > $(auxdir)/ltmain.in; \
> input="ltmain.m4sh"; \
> - echo $(edit) -e "s,@TIMESTAMP\@,$$TIMESTAMP,g" \
> - -e "s,@package_revision\@,$$correctver," \
> + echo $(bootstrap_edit) \
> $(srcdir)/$(auxdir)/ltmain.in "> $$target"; \
> - $(edit) -e "s,@TIMESTAMP\@,$$TIMESTAMP,g" \
> - -e "s,@package_revision\@,$$1,g" \
> - $(auxdir)/ltmain.in > $(auxdir)/ltmain.tmp; \
> + $(bootstrap_edit) \
> + $(auxdir)/ltmain.in > $(auxdir)/ltmain.tmp; \
> rm -f $(auxdir)/ltmain.in; \
> chmod a-w $(auxdir)/ltmain.tmp; \
> mv -f $(auxdir)/ltmain.tmp $(auxdir)/ltmain.sh; \
> @@ -264,6 +201,79 @@ LTDL_BOOTSTRAP_DEPS = $(srcdir)/libltdl/aclocal.m4 \
>
> all-local: $(LTDL_BOOTSTRAP_DEPS)
>
> +
> +## ---------------- ##
> +## Libtool scripts. ##
> +## ---------------- ##
> +
> +configure_edit = $(bootstrap_edit) \
> + -e 's,@aclocal_DATA\@,$(aclocalfiles),g' \
> + -e 's,@aclocaldir\@,$(aclocaldir),g' \
> + -e 's,@datadir\@,$(datadir),g' \
> + -e 's,@EGREP\@,$(EGREP),g' \
> + -e 's,@FGREP\@,$(FGREP),g' \
> + -e 's,@GREP\@,$(GREP),g' \
> + -e 's,@host_triplet\@,$(host_triplet),g' \
> + -e 's,@LN_S\@,$(LN_S),g' \
> + -e "s,@pkgconfig_files\@,$(auxfiles),g" \
> + -e 's,@pkgdatadir\@,$(pkgdatadir),g' \
> + -e "s,@pkgltdl_files\@,$(ltdldatafiles),g" \
> + -e 's,@prefix\@,$(prefix),g' \
> + -e 's,@SED\@,$(SED),g'
> +
> +# The libtool distributor and the standalone libtool script.
> +bin_SCRIPTS = libtoolize libtool
> +
> +libtoolize: $(srcdir)/libtoolize.in $(top_builddir)/config.status
> + rm -f libtoolize.tmp libtoolize
> + $(timestamp); \
> + input="libtoolize.m4sh"; \
> + $(configure_edit) \
> + $(srcdir)/libtoolize.in > libtoolize.tmp
> + chmod a+x libtoolize.tmp
> + chmod a-w libtoolize.tmp
> + mv -f libtoolize.tmp libtoolize
> +
> +# Use `$(srcdir)' for the benefit of non-GNU makes: this is
> +# how libtoolize.in appears in our dependencies.
> +EXTRA_DIST += libtoolize.m4sh
> +$(srcdir)/libtoolize.in: $(sh_files) libtoolize.m4sh Makefile.am
> + cd $(srcdir); \
> + rm -f libtoolize.in; \
> + $(M4SH) -B $(auxdir) libtoolize.m4sh > libtoolize.in
> +
> +# We used to do this with a 'stamp-vcl' file, but non-gmake builds
> +# would rerun configure on every invocation, so now we manually
> +# check the version numbers from the build rule when necessary.
> +libtool: $(top_builddir)/config.status $(srcdir)/$(auxdir)/ltmain.sh
> ChangeLog
> + @target=libtool; $(rebuild); \
> + if test -f "$$target"; then \
> + eval `sed -n '/^package_revision=/p' "$$target"`; \
> + test "$$package_revision" = "$$revision" && rebuild=false; \
> + fi; \
> + for prereq in $?; do \
> + case $$prereq in *ChangeLog);; *) rebuild=:;; esac; \
> + done; \
> + if $$rebuild; then \
> + echo $(SHELL) ./config.status $$target; \
> + cd $(top_builddir) && $(SHELL) ./config.status $$target; \
> + fi
> +
> +.PHONY: configure-subdirs
> +configure-subdirs distdir: $(DIST_MAKEFILE_LIST)
> address@hidden@:
> + dir=`echo $@ | sed 's,^[^/]*$$,.,;s,/[^/]*$$,,'`; \
> + test -d $$dir || mkdir $$dir || exit 1; \
> + abs_srcdir=`$(lt__cd) $(srcdir) && pwd`; \
> + (cd $$dir && $$abs_srcdir/$$dir/configure --with-dist) || exit 1
> +
> +
> +## -------- ##
> +## Libltdl. ##
> +## -------- ##
> +
> +include libltdl/Makefile.inc
> +
> EXTRA_DIST += $(srcdir)/libltdl/stamp-mk $(m4dir)/lt~obsolete.m4
>
> $(srcdir)/libltdl/Makefile.in: $(srcdir)/libltdl/Makefile.am \
> @@ -301,13 +311,6 @@ $(srcdir)/libltdl/config-h.in: $(sub_configure_deps)
> touch $@
>
>
> -## -------- ##
> -## Libltdl. ##
> -## -------- ##
> -
> -include libltdl/Makefile.inc
> -
> -
> ## -------------- ##
> ## Documentation. ##
> ## -------------- ##
> @@ -551,7 +554,7 @@ $(srcdir)/tests/package.m4: $(srcdir)/configure.ac
> Makefile.am
> echo 'm4_define([AT_PACKAGE_STRING], address@hidden@])'; \
> echo 'm4_define([AT_PACKAGE_BUGREPORT], address@hidden@])'; \
> echo 'm4_define([AT_PACKAGE_URL], address@hidden@])'; \
> - } | $(edit) > $(srcdir)/tests/package.m4
> + } | $(configure_edit) > $(srcdir)/tests/package.m4
>
> tests/atconfig: $(top_builddir)/config.status
> $(SHELL) ./config.status tests/atconfig
> @@ -897,7 +900,7 @@ check-recursive: tests/defs
> tests/defs: $(srcdir)/tests/defs.in
> rm -f tests/defs.tmp tests/defs; \
> input="defs.m4sh"; \
> - $(edit) $(srcdir)/tests/defs.in > tests/defs.tmp; \
> + $(configure_edit) $(srcdir)/tests/defs.in > tests/defs.tmp; \
> mv -f tests/defs.tmp tests/defs
>
> # Use `$(srcdir)/tests' for the benefit of non-GNU makes: this is
> diff --git a/NEWS b/NEWS
> index 688bdca..90a5e6e 100644
> --- a/NEWS
> +++ b/NEWS
> @@ -40,6 +40,8 @@ New in 2.2.12 2010-08-??: git version 2.2.11a, Libtool team:
> - Warnings from Autoconf v2.67-36-g1e604ec about incomplete programs
> passed to AC_*_IFELSE tests have been fixed.
> - On IRIX, the test for -Wl,-exported_symbol now also works with gfortran.
> + - The bug that leaked developer tool paths into the release tarballs
> + from ./bootstrap is fixed.
>
> New in 2.2.10 2010-06-10: git version 2.2.9a, Libtool team:
- [PATCH 3/6] Support missing detached signatures, like gnulib implementation., (continued)
- [PATCH 3/6] Support missing detached signatures, like gnulib implementation., Gary V. Vaughan, 2010/08/31
- [PATCH 4/6] Rewrite bootstrap script for consistency with our other shell code., Gary V. Vaughan, 2010/08/31
- [PATCH 6/6] Simplify and improve safety of bootstrap process., Gary V. Vaughan, 2010/08/31
- [PATCH 5/6] Don't leak developer GREP, SED etc into distribution file., Gary V. Vaughan, 2010/08/31
- Re: [PATCH 5/6] Don't leak developer GREP, SED etc into distribution file.,
Ralf Wildenhues <=
- Re: Backport non-gnulib dependent parts of my use-gnulib topic branch, Ralf Wildenhues, 2010/08/31