Re: [OS-BUILD PATCH] redhat/Makefile: fix default values for dist-brew's DISTRO and DIST

2023-03-01 Thread via Email Bridge
From: Íñigo Huguet on gitlab.com
https://gitlab.com/cki-project/kernel-ark/-/merge_requests/2339#note_1297525299

But if we maintain the default value assignment of DIST in Makefile.variable,
there is no way to know if a value different by default has been defined by
the user in the file.
___
kernel mailing list -- kernel@lists.fedoraproject.org
To unsubscribe send an email to kernel-le...@lists.fedoraproject.org
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@lists.fedoraproject.org
Do not reply to spam, report it: 
https://pagure.io/fedora-infrastructure/new_issue


Re: [OS-BUILD PATCH] redhat/Makefile: fix default values for dist-brew's DISTRO and DIST

2023-03-01 Thread Herton R. Krzesinski (via Email Bridge)
From: Herton R. Krzesinski on gitlab.com
https://gitlab.com/cki-project/kernel-ark/-/merge_requests/2339#note_1297506964

About changing the DISTRO, may be it's be useful, although we default for RHEL
as usually a RHEL developer wants to test the build on brew (rhel) instead of
centos even if we are building/integrating it first on centos. So that's why I
think no one complained so far, the maintainer (eg. me) is usually the one
which uses the DISTRO value (only at the dist-git sync step), but that has
been working on the centos side so far passing through the command line, so I
didn't noticed it was broken too since I don't use Makefile.variables.

To determine the where the value comes, we could use origin like ```ifeq
("$(origin BUILD_TARGET)", "command line")```, but I didn't think what the
logic should be.

I think we should keep DIST as is on Makefile.variables and logic is done in
Makefile? I think could be possible? I'm not sure if there is a problem. Right
now we change the DIST in Makefile.variables when kernel-ark is forked to
RHEL, for example, centos9 kernel src.git sets it to DIST ?= .el9 in
Makefile.variables.
___
kernel mailing list -- kernel@lists.fedoraproject.org
To unsubscribe send an email to kernel-le...@lists.fedoraproject.org
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@lists.fedoraproject.org
Do not reply to spam, report it: 
https://pagure.io/fedora-infrastructure/new_issue


Re: [OS-BUILD PATCH] redhat/Makefile: fix default values for dist-brew's DISTRO and DIST

2023-03-01 Thread via Email Bridge
From: Íñigo Huguet on gitlab.com
https://gitlab.com/cki-project/kernel-ark/-/merge_requests/2339#note_1297458063

Basically, we should need to determine if the user has selected any value for
DISTRO, DIST or BUILD_TARGET, either in command line or in Makefile.variables.
If the user has defined any, respect it, but if not, we choose.

The problem is that, for DIST, the default value is being set in
Makefile.variables, and it depends on the local machine (i.e. if you're in
Fedora, it selects '.fc37'). Maybe we could put `DIST ?=` in
Makefile.variables, and move the current default value to Makefile? Would that
be OK?
___
kernel mailing list -- kernel@lists.fedoraproject.org
To unsubscribe send an email to kernel-le...@lists.fedoraproject.org
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@lists.fedoraproject.org
Do not reply to spam, report it: 
https://pagure.io/fedora-infrastructure/new_issue


Re: [OS-BUILD PATCH] redhat/Makefile: fix default values for dist-brew's DISTRO and DIST

2023-03-01 Thread via Email Bridge
From: Íñigo Huguet on gitlab.com
https://gitlab.com/cki-project/kernel-ark/-/merge_requests/2339#note_1297310135

That won't work, either. In the second level sub-make, BUILD_TARGET won't be
empty, but might have a wrong value, inherited from the first level make...

Also, thanks to your comment, I've seen that dist-brew won't respect the
values of DISTRO and DIST set in Makefile.variables, but neither before these
changes. Is that a problem? I guess that it doesn't make sense to set them to
Fedora values, but maybe yes to Centos values?
___
kernel mailing list -- kernel@lists.fedoraproject.org
To unsubscribe send an email to kernel-le...@lists.fedoraproject.org
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@lists.fedoraproject.org
Do not reply to spam, report it: 
https://pagure.io/fedora-infrastructure/new_issue


Re: [OS-BUILD PATCH] redhat/Makefile: fix default values for dist-brew's DISTRO and DIST

2023-02-28 Thread Herton R. Krzesinski (via Email Bridge)
From: Herton R. Krzesinski on gitlab.com
https://gitlab.com/cki-project/kernel-ark/-/merge_requests/2339#note_1295688724

This doesn't work, because it'll override anything you set inside
redhat/Makefile.variables. For example if I do this:

```
diff --git a/redhat/Makefile.variables b/redhat/Makefile.variables
index c20fef88a468..1381ffbb7c1d 100644
--- a/redhat/Makefile.variables
+++ b/redhat/Makefile.variables
@@ -30,7 +30,7 @@ BUILD_PROFILE ?=
 # default values can be found in redhat/Makefile and are dependent on the
 # target OS.  The target OS can be changed by setting the DIST or DISTRO
 # variables.
-BUILD_TARGET ?=
+BUILD_TARGET ?= rawhide

 # BUMP_RELEASE determines whether the pkgrelease is bumped when you call
 # 'make dist-release'.  It should be set to "yes" if you expect to do
```

Your patch will override it, even if I want to change the value in
Makefile.variables.

Probably what we want to do is to change the ifdefs. For example:

```
  ifndef BUILD_TARGET
```

to

```
ifeq ("$(BUILD_TARGET)", "")
```

It looks like we should be using ifeq instead. I remember something about
ifdef
not being ideal with make and exported variables.

So switch to ifeq, and see if the remaining changes are really needed or not
here.
___
kernel mailing list -- kernel@lists.fedoraproject.org
To unsubscribe send an email to kernel-le...@lists.fedoraproject.org
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@lists.fedoraproject.org
Do not reply to spam, report it: 
https://pagure.io/fedora-infrastructure/new_issue


[OS-BUILD PATCH] redhat/Makefile: fix default values for dist-brew's DISTRO and DIST

2023-02-28 Thread via Email Bridge
From: Íñigo Huguet 

redhat/Makefile: fix default values for dist-brew's DISTRO and DIST

In dist-brew target, target-specific variables to change the default
value of DISTRO and DIST to 'rhel' and '.el9', but they're not useful
because the rest of variables that depends on them, such us BUILD_TARGET,
has been calculated yet.

The most visible error is that `make dist-brew` fails with 'brew: error:
No such build target: rawhide'. Rawhide is the default BUILD_TARGET for
Fedora, not for RHEL.

Fix it by recursively calling `make` so the sub-make has the correct DISTRO
and DIST values since the beginning, thus calculating the rest of values
correctly.

To make this work, get rid of the `ifdef BUILD_TARGET` checks, or the
sub-make will see it as already defined (inherited) and won't change it.
Since variables defined as commandline arguments always override
variables defined inside the Makefile, users still can select a
different one: `make dist-brew BUILD_TARGET=whatever`.

Also, fix a missing closing quote in `ifeq ("$(DIST)", ".eln)`.

Signed-off-by: Íñigo Huguet 

diff --git a/redhat/Makefile b/redhat/Makefile
index blahblah..blahblah 100644
--- a/redhat/Makefile
+++ b/redhat/Makefile
@@ -223,9 +223,7 @@ SRPM:=$(SRPMS)/$(RELEASETAG)$(DIST).src.rpm
 #
 ifeq ("$(DISTRO)", "fedora")
   RHDISTGIT_BRANCH:=rawhide
-  ifndef BUILD_TARGET
-BUILD_TARGET:=rawhide
-  endif
+  BUILD_TARGET:=rawhide
   # The Fedora tarfile name is based on an upstream tag as users may
   # replace the tarball from one with upstream, rebuild, and then deploy
   # without changing anything else in the specfile.
@@ -236,12 +234,8 @@ ifeq ("$(DISTRO)", "fedora")
   RHPKG_BIN:=fedpkg
 else ifeq ("$(DISTRO)", "centos")
   RHDISTGIT_BRANCH:=c$(RHEL_MAJOR)s
-  ifndef BUILD_PROFILE
-BUILD_PROFILE:= -p stream
-  endif
-  ifndef BUILD_TARGET
-BUILD_TARGET:=c$(RHEL_MAJOR)s-candidate
-  endif
+  BUILD_PROFILE:= -p stream
+  BUILD_TARGET:=c$(RHEL_MAJOR)s-candidate
   SPECTARFILE_RELEASE:=$(BASEVERSION)$(DIST)
   SPECKABIVERSION:=$(BASEVERSION)$(DIST)
   DISTRELEASETAG:=$(RELEASETAG)$(DIST)
@@ -250,21 +244,16 @@ else ifeq ("$(DISTRO)", "centos")
   RHPKG_BIN:=centpkg
 else
   RHDISTGIT_BRANCH:=rhel-$(RHEL_MAJOR).$(RHEL_MINOR).0
-  ifndef BUILD_TARGET
-ifeq ("$(DIST)", ".eln")
-  BUILD_TARGET:=eln
-else
-  # This value is used by the dist[g]-targets.  Changing this value has 
significant
-  # consequences for all of RHEL kernel engineering.
-  BUILD_TARGET:=rhel-$(RHEL_MAJOR)-newest-test-pesign
-endif
-  endif
-  ifeq ("$(DIST)", ".eln)
+  ifeq ("$(DIST)", ".eln")
+BUILD_TARGET:=eln
 DISTRELEASETAG:=$(RELEASETAG)
 DISTBASEVERSION:=$(BASEVERSION)
 SPECTARFILE_RELEASE:=$(BASEVERSION)
 SPECKABIVERSION:=$(BASEVERSION)
   else
+# This value is used by the dist[g]-targets.  Changing this value has 
significant
+# consequences for all of RHEL kernel engineering.
+BUILD_TARGET:=rhel-$(RHEL_MAJOR)-newest-test-pesign
 DISTRELEASETAG:=$(RELEASETAG)$(DIST)
 DISTBASEVERSION:=$(BASEVERSION)$(DIST)
 SPECTARFILE_RELEASE:=$(BASEVERSION)$(DIST)
@@ -753,14 +742,18 @@ dist-vr-check:
exit 1; \
fi
 
-dist-brew: DIST=.el9
-dist-brew: DISTRO=rhel
-dist-brew dist-koji: dist-%: dist-vr-check dist-srpm
+# Call as sub-make to select different default DISTRO and DIST for dist-brew.
+# This is because target-specific variables only apply inside the recipe, but 
we
+# need to recalculate some values such as BUILD_TARGET that depends on them.
+dist-brew distg-brew: DISTRO=rhel
+dist-brew distg-brew: DIST=.el9
+dist-brew distg-brew dist-koji distg_koji:
+   $(MAKE) _$@
+
+_dist-brew _dist-koji: _dist-%: dist-vr-check dist-srpm
$* $(BUILD_PROFILE) build $(BUILD_FLAGS) --scratch $(BUILD_TARGET) 
$(SRPMS)/$(RELEASETAG)$(DIST).src.rpm $(OUTPUT_FILE)
 
-distg-brew: DIST=.el9
-distg-brew: DISTRO=rhel
-distg-brew distg-koji: distg-%: dist-vr-check
+_distg-brew _distg-koji: _distg-%: dist-vr-check
$* $(BUILD_PROFILE) build $(BUILD_FLAGS) --scratch $(BUILD_TARGET) 
"$(RHGITURL)?redhat/koji#$(RHGITCOMMIT)"
 
 .PHONY: $(REDHAT)/rpm/SOURCES/$(SPECFILE)

--
https://gitlab.com/cki-project/kernel-ark/-/merge_requests/2339
___
kernel mailing list -- kernel@lists.fedoraproject.org
To unsubscribe send an email to kernel-le...@lists.fedoraproject.org
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@lists.fedoraproject.org
Do not reply to spam, report it: 
https://pagure.io/fedora-infrastructure/new_issue