On Fri, Nov 20, 2020 at 12:17:02AM -0000, GitLab Bridge on behalf of bcrocker wrote:
> From: Ben Crocker <bcrocker@xxxxxxxxxx>
>
> GIT ?= git
Hi Ben I have some comments, please see them below and inline in the code.
>
> and replace literal occurrences of 'git' with $(GIT).
> This change enables us to override 'git' with, e.g., some
> arbitrary shell script that prints additional information
> and/or does additional processing before and/or after (or
> even instead of) invoking /usr/bin/git.
>
> Add dist-dump-variables for dynamically deriving variables
> from Makefile.common and dumping them.
>
> Add a dist-clean-scripts target to clean up generated scripts.
I would split the addition of the dist-dump-variables in a new patch, because
it's not related to the git replacement, it's a different topic. The
dist-clean-scripts change is related to the dist-dump-variables, so it can be in
the same dist-dump-variables new patch.
>
> Add a description of the new dist-self-test target to dist-full-help.
You are adding the dist-self-test help in this patch, instead of patch 2,
which really adds the dist-self-test target. I think you should move this to
patch 2.
>
> Signed-off-by: Ben Crocker <bcrocker@xxxxxxxxxx>
> ---
> redhat/Makefile | 42 +++++++++++++++++++++++++++---------------
> redhat/Makefile.common | 16 ++++++++--------
> 2 files changed, 35 insertions(+), 23 deletions(-)
>
> diff --git a/redhat/Makefile b/redhat/Makefile
> index 834704a1a0a7..fd10ac8f145d 100644
> --- a/redhat/Makefile
> +++ b/redhat/Makefile
> @@ -1,3 +1,4 @@
> +GIT ?= git
> include Makefile.common
> include Makefile.rhpkg
>
> @@ -32,8 +33,8 @@ endif
> BUILD_TARGET ?= --scratch $(BUILD_SCRATCH_TARGET)
> FLAVOR =
>
> -RHGITURL?=$(shell git config rhg.url || git config remote.origin.url)
> -RHGITCOMMIT?=$(shell git log -1 --pretty=format:%H)
> +RHGITURL?=$(shell $(GIT) config rhg.url || $(GIT) config remote.origin.url)
> +RHGITCOMMIT?=$(shell $(GIT) log -1 --pretty=format:%H)
>
> # this section is needed in order to make O= to work
> _OUTPUT := ..
> @@ -165,7 +166,10 @@ dist-clean-rpmdirs:
> rm -rf $$i; \
> done;
>
> -dist-clean: dist-clean-sources dist-clean-configs dist-clean-rpmdirs
> +dist-clean-scripts:
> + @if [ -f dist-dump-variables.sh ]; then rm dist-dump-variables.sh; fi
This is subject to dist-dump-variables code and should be in the
dist-dump-variables.sh patch as I suggested above.
> +
> +dist-clean: dist-clean-sources dist-clean-configs dist-clean-rpmdirs dist-clean-scripts
>
> dist-stub-key:
> @echo "Copying pre-generated keys";
> @@ -199,7 +203,7 @@ $(KABIDW_TARBALL):
> dist-git-version-check:
> @# genspec.sh uses pathspec magic that wasn't introduced until version 2.13
> @IFS=" ."; \
> - set -- $$(git --version); \
> + set -- $$($(GIT) --version); \
> IFS=; \
> if [ "$$3" -lt 2 -o \( "$$3" -eq 2 -a "$$4" -lt 13 \) ]; then \
> echo "ERROR: You need git version 2.13 or newer to run some setup commands"; \
> @@ -219,7 +223,7 @@ setup-source: dist-git-version-check dist-clean-sources
> sources-rh: $(TARBALL)
> @cp -l $(TARBALL) $(SOURCES)/ || cp $(TARBALL) $(SOURCES)/
> @touch $(TESTPATCH)
> - @git diff --no-renames HEAD > $(TESTPATCH).tmp
> + @$(GIT) diff --no-renames HEAD > $(TESTPATCH).tmp
> @# 1) filterdiff will return crap from the patches it just filtered,
> @# that's why egrep is needed so if there're changes under redhat/
> @# but not everywhere else, it will be empty just like
> @@ -265,7 +269,7 @@ sources-rh: $(TARBALL)
> dist-sources: setup-source dist-configs-check dist-kabi dist-kabi-dup sources-rh
>
> dist-test-patch:
> - @git diff --no-renames HEAD > $(TESTPATCH);
> + @$(GIT) diff --no-renames HEAD > $(TESTPATCH);
> @($(FILTERDIFF) $(TESTPATCH) | egrep -v "^index|^diff" >$(TESTPATCH).tmp; true)
> @mv $(TESTPATCH).tmp $(TESTPATCH);
>
> @@ -300,34 +304,34 @@ dist-release-finish: setup-source
> @cp $(SOURCES)/$(CHANGELOG) $(REDHAT)/$(CHANGELOG)
> @echo $(MARKER) > $(REDHAT)/marker
> @# if neither changelog nor marker was updated, skip bumping a release
> - git diff-index --quiet HEAD && (echo "Nothing changed, skipping updates"; exit 0) || true
> + $(GIT) diff-index --quiet HEAD && (echo "Nothing changed, skipping updates"; exit 0) || true
> $(REDHAT)/scripts/new_release.sh $(REDHAT) $(__YSTREAM) $(__ZSTREAM); \
> - git add $(REDHAT)/$(CHANGELOG); \
> - git add $(REDHAT)/marker; \
> - git commit -s ../Makefile.rhelver $(REDHAT)/marker $(REDHAT)/$(CHANGELOG) $(PACKAGE_NAME).spec.template -m "[redhat] $(PACKAGE_NAME)-$(STAMP_VERSION)-$(PREBUILD)$(BUILD)$(BUILDID)"; \
> + $(GIT) add $(REDHAT)/$(CHANGELOG); \
> + $(GIT) add $(REDHAT)/marker; \
> + $(GIT) commit -s ../Makefile.rhelver $(REDHAT)/marker $(REDHAT)/$(CHANGELOG) $(PACKAGE_NAME).spec.template -m "[redhat] $(PACKAGE_NAME)-$(STAMP_VERSION)-$(PREBUILD)$(BUILD)$(BUILDID)"; \
>
> dist-release: dist-clean-sources
> @$(MAKE) dist-release-finish
> dist-release-tag:
> - @git tag -a -m "$(PACKAGE_NAME)-$(STAMP_VERSION)-$(PKGRELEASE)" $(PACKAGE_NAME)-$(STAMP_VERSION)-$(PKGRELEASE)
> + @$(GIT) tag -a -m "$(PACKAGE_NAME)-$(STAMP_VERSION)-$(PKGRELEASE)" $(PACKAGE_NAME)-$(STAMP_VERSION)-$(PKGRELEASE)
>
> git-tree-check:
> - @if test -n "$(DIST_PUSH)" && test -z "$(shell git remote get-url gitlab 2>/dev/null)"; then \
> + @if test -n "$(DIST_PUSH)" && test -z "$(shell $(GIT) remote get-url gitlab 2>/dev/null)"; then \
> echo -e "Please run 'git remote add gitlab <url>' to enable git-push.\n"; \
> exit 1; \
> fi
> - @git diff-index --quiet HEAD || \
> + @$(GIT) diff-index --quiet HEAD || \
> { echo -e "Dirty tree, please clean before merging.\n"; exit 1; }
>
> DIST_BRANCH ?= "os-build"
> dist-merge-upstream: git-tree-check
> - @if test "$(shell git branch --show-current)" != "$(DIST_BRANCH)"; then \
> + @if test "$(shell $(GIT) branch --show-current)" != "$(DIST_BRANCH)"; then \
> echo -e "Please checkout $(DIST_BRANCH) branch before merging.\n"; \
> exit 1; \
> fi;
>
> @# If TAG is empty, script defaults to master:HEAD
> - @git checkout $(DIST_BRANCH)
> + $(GIT) checkout $(DIST_BRANCH)
> @cd ..; $(REDHAT)/scripts/ci/ark-update-configs.sh $(TAG)
>
> dist-merge-upstream-push: export DIST_PUSH="1"
> @@ -381,6 +385,13 @@ dist-get-latest:
> dist-os-version:
> @echo "OSVERSION: $(RHEL_MAJOR).$(RHEL_MINOR)"
>
> +.EXPORT_ALL_VARIABLES:
> +.PHONY: dist-dump-variables
> +dist-dump-variables:
> + grep "^[ ]*[a-zA-Z_][a-zA-Z_0-9]*[ ]*[:?]*=" $(REDHAT)/Makefile.common | sed -e 's/[ ]*\([a-zA-Z_][a-zA-Z_0-9]*\).*/echo "\1=$$\1"/' | sort | uniq > dist-dump-variables.sh
> + chmod +x $(REDHAT)/dist-dump-variables.sh
> + @$(REDHAT)/dist-dump-variables.sh
> +
This hunk should be in the dist-dump-variables patch I suggested above.
> dist-help:
> @echo 'Cleaning targets:'
> @echo ' dist-clean - Clean redhat/configs/ and redhat/rpm/ directories.'
> @@ -498,4 +509,5 @@ dist-full-help:
> @echo ' development tag.'
> @echo ' dist-os-version - Displays the current Red Hat Enterprise Linux versioni'
> @echo ' target used by the current branch/tree.'
> + @echo ' dist-self-test - Runs self-tests from the redhat/self-test directory'
This hunk should be in patch 2 in current series which is the one which
really adds the dist-self-test target, doesn't belong here.
> @echo ''
> diff --git a/redhat/Makefile.common b/redhat/Makefile.common
> index b335f3c77c7d..80c3b8dfccd4 100644
> --- a/redhat/Makefile.common
> +++ b/redhat/Makefile.common
> @@ -1,4 +1,4 @@
> -TOPDIR:=$(shell git rev-parse --show-toplevel)
> +TOPDIR:=$(shell $(GIT) rev-parse --show-toplevel)
> REDHAT:=$(TOPDIR)/redhat
> include $(TOPDIR)/Makefile.rhelver
>
> @@ -6,11 +6,11 @@ RPMBUILD := $(shell if [ -x "/usr/bin/rpmbuild" ]; then echo rpmbuild; \
> else echo rpm; fi)
>
> MACH := $(shell uname -m)
> -RPMKVERSION:=$(shell git show HEAD:Makefile | sed -ne '/^VERSION\ =\ /{s///;p;q}')
> -RPMKPATCHLEVEL:=$(shell git show HEAD:Makefile | sed -ne '/^PATCHLEVEL\ =\ /{s///;p;q}')
> -RPMKSUBLEVEL:=$(shell git show HEAD:Makefile | sed -ne '/^SUBLEVEL\ =\ /{s///;p;q}')
> -RPMKEXTRAVERSION:=$(shell git show HEAD:Makefile | sed -ne '/^EXTRAVERSION\ =\ /{s///;p;q}')
> -GITID:= $(shell git log --max-count=1 --pretty=format:%H)
> +RPMKVERSION:=$(shell $(GIT) show HEAD:Makefile | sed -ne '/^VERSION\ =\ /{s///;p;q}')
> +RPMKPATCHLEVEL:=$(shell $(GIT) show HEAD:Makefile | sed -ne '/^PATCHLEVEL\ =\ /{s///;p;q}')
> +RPMKSUBLEVEL:=$(shell $(GIT) show HEAD:Makefile | sed -ne '/^SUBLEVEL\ =\ /{s///;p;q}')
> +RPMKEXTRAVERSION:=$(shell $(GIT) show HEAD:Makefile | sed -ne '/^EXTRAVERSION\ =\ /{s///;p;q}')
> +GITID:= $(shell $(GIT) log --max-count=1 --pretty=format:%H)
> # marker is git tag which we base off of for exporting patches
> # Make sure marker uses RPMKPATCHLEVEL and RPMKEXTRAVERSION from the kernel
> # makefile as opposed to any adjusted version for snapshotting.
> @@ -42,8 +42,8 @@ else
> endif
> ifeq ($(VERSION_ON_UPSTREAM),1)
> # master is expected to track mainline.
> - MERGE_BASE:=$(shell git merge-base HEAD master)
> - _TAG:=$(shell git describe $(MERGE_BASE))
> + MERGE_BASE:=$(shell $(GIT) merge-base HEAD master)
> + _TAG:=$(shell $(GIT) describe $(MERGE_BASE))
> # a snapshot off of a tagged git is of the form [tag]-[cnt]-g[hash]
> SNAPSHOT:=$(shell echo $(_TAG) | grep -c '\-g')
> else
> --
> GitLab
> _______________________________________________
> kernel mailing list -- kernel@xxxxxxxxxxxxxxxxxxxxxxx
> To unsubscribe send an email to kernel-leave@xxxxxxxxxxxxxxxxxxxxxxx
> Fedora Code of Conduct: https://docs.fedoraproject.org/en-US/project/code-of-conduct/
> List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines
> List Archives: https://lists.fedoraproject.org/archives/list/kernel@xxxxxxxxxxxxxxxxxxxxxxx
--
[]'s
Herton
_______________________________________________
kernel mailing list -- kernel@xxxxxxxxxxxxxxxxxxxxxxx
To unsubscribe send an email to kernel-leave@xxxxxxxxxxxxxxxxxxxxxxx
Fedora Code of Conduct: https://docs.fedoraproject.org/en-US/project/code-of-conduct/
List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines
List Archives: https://lists.fedoraproject.org/archives/list/kernel@xxxxxxxxxxxxxxxxxxxxxxx
