diff mbox series

ci: clear and mark MAKEFLAGS exported just once

Message ID xmqqd0o3mac5.fsf_-_@gitster-ct.c.googlers.com (mailing list archive)
State New, archived
Headers show
Series ci: clear and mark MAKEFLAGS exported just once | expand

Commit Message

Junio C Hamano Feb. 7, 2019, 11:45 p.m. UTC
Clearing it once upfront, and turning all the assignment into
appending, would future-proof the code even more, to prevent
mistakes the previous one fixed from happening again.

Also, mark the variable exported just once at the beginning.  There
is no point in marking it exported repeatedly.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
    >> So I'd rather clear `MAKEFLAGS` at the beginning (i.e. where you `export
    >> MAKEFLAGS`, I'd simply append a `=`).

    This time in proper patch form.

 ci/lib.sh | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

Comments

SZEDER Gábor Feb. 8, 2019, 12:17 a.m. UTC | #1
On Thu, Feb 07, 2019 at 03:45:46PM -0800, Junio C Hamano wrote:
> Clearing it once upfront, and turning all the assignment into
> appending, would future-proof the code even more, to prevent
> mistakes the previous one fixed from happening again.
> 
> Also, mark the variable exported just once at the beginning.  There
> is no point in marking it exported repeatedly.
> 
> Signed-off-by: Junio C Hamano <gitster@pobox.com>
> ---
>     >> So I'd rather clear `MAKEFLAGS` at the beginning (i.e. where you `export
>     >> MAKEFLAGS`, I'd simply append a `=`).
> 
>     This time in proper patch form.

Makes sense, and the patch looks good to me.

>  ci/lib.sh | 9 ++++++---
>  1 file changed, 6 insertions(+), 3 deletions(-)
> 
> diff --git a/ci/lib.sh b/ci/lib.sh
> index cee51a4cc4..288a5b3884 100755
> --- a/ci/lib.sh
> +++ b/ci/lib.sh
> @@ -74,6 +74,9 @@ check_unignored_build_artifacts ()
>  	}
>  }
>  
> +# Clear MAKEFLAGS that may come from the outside world.
> +export MAKEFLAGS=
> +
>  # Set 'exit on error' for all CI scripts to let the caller know that
>  # something went wrong.
>  # Set tracing executed commands, primarily setting environment variables
> @@ -101,7 +104,7 @@ then
>  	BREW_INSTALL_PACKAGES="git-lfs gettext"
>  	export GIT_PROVE_OPTS="--timer --jobs 3 --state=failed,slow,save"
>  	export GIT_TEST_OPTS="--verbose-log -x --immediate"
> -	export MAKEFLAGS="--jobs=2"
> +	MAKEFLAGS="$MAKEFLAGS --jobs=2"
>  elif test -n "$SYSTEM_COLLECTIONURI" || test -n "$SYSTEM_TASKDEFINITIONSURI"
>  then
>  	CI_TYPE=azure-pipelines
> @@ -126,7 +129,7 @@ then
>  	BREW_INSTALL_PACKAGES=gcc@8
>  	export GIT_PROVE_OPTS="--timer --jobs 10 --state=failed,slow,save"
>  	export GIT_TEST_OPTS="--verbose-log -x --write-junit-xml"
> -	export MAKEFLAGS="--jobs=10"
> +	MAKEFLAGS="$MAKEFLAGS --jobs=10"
>  	test windows_nt != "$CI_OS_NAME" ||
>  	GIT_TEST_OPTS="--no-chain-lint --no-bin-wrappers $GIT_TEST_OPTS"
>  else
> @@ -185,4 +188,4 @@ GIT_TEST_GETTEXT_POISON)
>  	;;
>  esac
>  
> -export MAKEFLAGS="$MAKEFLAGS CC=${CC:-cc}"
> +MAKEFLAGS="$MAKEFLAGS CC=${CC:-cc}"
> -- 
> 2.21.0-rc0
>
Johannes Schindelin Feb. 8, 2019, 10:11 a.m. UTC | #2
Hi,

On Fri, 8 Feb 2019, SZEDER Gábor wrote:

> On Thu, Feb 07, 2019 at 03:45:46PM -0800, Junio C Hamano wrote:
> > Clearing it once upfront, and turning all the assignment into
> > appending, would future-proof the code even more, to prevent
> > mistakes the previous one fixed from happening again.
> > 
> > Also, mark the variable exported just once at the beginning.  There
> > is no point in marking it exported repeatedly.
> > 
> > Signed-off-by: Junio C Hamano <gitster@pobox.com>
> > ---
> >     >> So I'd rather clear `MAKEFLAGS` at the beginning (i.e. where
> >     >> you `export MAKEFLAGS`, I'd simply append a `=`).
> > 
> >     This time in proper patch form.
> 
> Makes sense, and the patch looks good to me.

To me, too.

Thank you,
Dscho

> 
> >  ci/lib.sh | 9 ++++++---
> >  1 file changed, 6 insertions(+), 3 deletions(-)
> > 
> > diff --git a/ci/lib.sh b/ci/lib.sh
> > index cee51a4cc4..288a5b3884 100755
> > --- a/ci/lib.sh
> > +++ b/ci/lib.sh
> > @@ -74,6 +74,9 @@ check_unignored_build_artifacts ()
> >  	}
> >  }
> >  
> > +# Clear MAKEFLAGS that may come from the outside world.
> > +export MAKEFLAGS=
> > +
> >  # Set 'exit on error' for all CI scripts to let the caller know that
> >  # something went wrong.
> >  # Set tracing executed commands, primarily setting environment variables
> > @@ -101,7 +104,7 @@ then
> >  	BREW_INSTALL_PACKAGES="git-lfs gettext"
> >  	export GIT_PROVE_OPTS="--timer --jobs 3 --state=failed,slow,save"
> >  	export GIT_TEST_OPTS="--verbose-log -x --immediate"
> > -	export MAKEFLAGS="--jobs=2"
> > +	MAKEFLAGS="$MAKEFLAGS --jobs=2"
> >  elif test -n "$SYSTEM_COLLECTIONURI" || test -n "$SYSTEM_TASKDEFINITIONSURI"
> >  then
> >  	CI_TYPE=azure-pipelines
> > @@ -126,7 +129,7 @@ then
> >  	BREW_INSTALL_PACKAGES=gcc@8
> >  	export GIT_PROVE_OPTS="--timer --jobs 10 --state=failed,slow,save"
> >  	export GIT_TEST_OPTS="--verbose-log -x --write-junit-xml"
> > -	export MAKEFLAGS="--jobs=10"
> > +	MAKEFLAGS="$MAKEFLAGS --jobs=10"
> >  	test windows_nt != "$CI_OS_NAME" ||
> >  	GIT_TEST_OPTS="--no-chain-lint --no-bin-wrappers $GIT_TEST_OPTS"
> >  else
> > @@ -185,4 +188,4 @@ GIT_TEST_GETTEXT_POISON)
> >  	;;
> >  esac
> >  
> > -export MAKEFLAGS="$MAKEFLAGS CC=${CC:-cc}"
> > +MAKEFLAGS="$MAKEFLAGS CC=${CC:-cc}"
> > -- 
> > 2.21.0-rc0
> > 
>
diff mbox series

Patch

diff --git a/ci/lib.sh b/ci/lib.sh
index cee51a4cc4..288a5b3884 100755
--- a/ci/lib.sh
+++ b/ci/lib.sh
@@ -74,6 +74,9 @@  check_unignored_build_artifacts ()
 	}
 }
 
+# Clear MAKEFLAGS that may come from the outside world.
+export MAKEFLAGS=
+
 # Set 'exit on error' for all CI scripts to let the caller know that
 # something went wrong.
 # Set tracing executed commands, primarily setting environment variables
@@ -101,7 +104,7 @@  then
 	BREW_INSTALL_PACKAGES="git-lfs gettext"
 	export GIT_PROVE_OPTS="--timer --jobs 3 --state=failed,slow,save"
 	export GIT_TEST_OPTS="--verbose-log -x --immediate"
-	export MAKEFLAGS="--jobs=2"
+	MAKEFLAGS="$MAKEFLAGS --jobs=2"
 elif test -n "$SYSTEM_COLLECTIONURI" || test -n "$SYSTEM_TASKDEFINITIONSURI"
 then
 	CI_TYPE=azure-pipelines
@@ -126,7 +129,7 @@  then
 	BREW_INSTALL_PACKAGES=gcc@8
 	export GIT_PROVE_OPTS="--timer --jobs 10 --state=failed,slow,save"
 	export GIT_TEST_OPTS="--verbose-log -x --write-junit-xml"
-	export MAKEFLAGS="--jobs=10"
+	MAKEFLAGS="$MAKEFLAGS --jobs=10"
 	test windows_nt != "$CI_OS_NAME" ||
 	GIT_TEST_OPTS="--no-chain-lint --no-bin-wrappers $GIT_TEST_OPTS"
 else
@@ -185,4 +188,4 @@  GIT_TEST_GETTEXT_POISON)
 	;;
 esac
 
-export MAKEFLAGS="$MAKEFLAGS CC=${CC:-cc}"
+MAKEFLAGS="$MAKEFLAGS CC=${CC:-cc}"