On Mon, Aug 29, 2016 at 2:30 PM, Lukas Slebodnik <[email protected]> wrote:
> On (29/08/16 14:22), Fabiano Fidêncio wrote:
>>On Mon, Aug 29, 2016 at 1:39 PM, Lukas Slebodnik <[email protected]> wrote:
>>> On (17/08/16 22:14), Fabiano Fidêncio wrote:
>>>>Howdy!
>>>>
>>>>I've been using a lot the "make prerelease-rpms" command and I've
>>>>found out the fact that all the resulting rpms of 2 (or more builds)
>>>>are put in the same buildroot/RPMS/{x86_64,noarch}/ directory a bit
>>>>unfortunate. It's not exactly handy to find out which are the packages
>>>>I want to copy to my guest and deleting the buildroot before doing the
>>>>build is also not so cool, as I may want to check stuff from two
>>>>different builds.
>>>>
>>>>With this problem in mind, I came with this patchset that generates
>>>>the rpms/srpm in a specific folder for each build, which makes my life
>>>>way easier when I want to copy those rpms to my guest and do my tests
>>>>there.
>>>>
>>>>Please, see the attached patches.
>>>>
>>>>Best Regards,
>>>>--
>>>>Fabiano Fidêncio
>>>
>>> >From e5f87e198a6a42f003ed9ce78f91db8cab070539 Mon Sep 17 00:00:00 2001
>>>>From: =?UTF-8?q?Fabiano=20Fid=C3=AAncio?= <[email protected]>
>>>>Date: Wed, 17 Aug 2016 21:08:23 +0200
>>>>Subject: [PATCH 1/3] BUILD: Clean up prerelease targets
>>>>MIME-Version: 1.0
>>>>Content-Type: text/plain; charset=UTF-8
>>>>Content-Transfer-Encoding: 8bit
>>>>
>>>>As I'm already going to do some modifications in the prerelease targets,
>>>>let's clean them up a little bit before the modifications by introducing
>>>>a few variables and (trying to) not exceed 80 characters per line (which
>>>>ended up not being possible for all lines, but the overall result is
>>>>still okay).
>>>>
>>>>Signed-off-by: Fabiano Fidêncio <[email protected]>
>>>>---
>>>> Makefile.am | 13 +++++++++++--
>>>> 1 file changed, 11 insertions(+), 2 deletions(-)
>>>>
>>>>diff --git a/Makefile.am b/Makefile.am
>>>>index 8b9240f..7e14d5c 100644
>>>>--- a/Makefile.am
>>>>+++ b/Makefile.am
>>>>@@ -4189,6 +4189,15 @@ rpmroot:
>>>>       $(MKDIR_P) $(RPMBUILD)/SPECS
>>>>       $(MKDIR_P) $(RPMBUILD)/SRPMS
>>>>
>>>>+# pre-release related vars
>>>>+
>>>>+PR_VERSION_DATE := $(shell date +%Y%m%d.%H%M)
>>>>+PR_VERSION_COMMIT_HASH := $(shell git log -1 --pretty=format:%h)
>>>>+PR_VERSION_NUMBER = $(PR_VERSION_DATE).git$(PR_VERSION_COMMIT_HASH)
>>>>+PR_VERSION_REGEX = m4_define(\[PRERELEASE_VERSION_NUMBER\], \[.*\])
>>>>+PR_VERSION_REPL = m4_define(\[PRERELEASE_VERSION_NUMBER\], 
>>>>\[.$(PR_VERSION_NUMBER)\])
>>>>+PR_SED = sed -e "s/$(PR_VERSION_REGEX)/$(PR_VERSION_REPL)/"
>>>>+
>>>> rpmbrprep: dist-gzip rpmroot
>>>> if GIT_CHECKOUT
>>>> # When we're building RPMs from a git checkout,
>>>>@@ -4206,7 +4215,7 @@ rpms: rpmbrprep
>>>> if GIT_CHECKOUT
>>>> prerelease-rpms:
>>>>       cp $(srcdir)/version.m4 $(srcdir)/version.m4.orig
>>>>-      sed -e "s/m4_define(\[PRERELEASE_VERSION_NUMBER\], 
>>>>\[.*\])/m4_define(\[PRERELEASE_VERSION_NUMBER\], \[.`date 
>>>>+%Y%m%d.%H%M`.git`git log -1 --pretty=format:%h`\])/" < 
>>>>$(srcdir)/version.m4.orig > $(srcdir)/version.m4
>>>>+      $(PR_SED) < $(srcdir)/version.m4.orig > $(srcdir)/version.m4
>>>>       $(MAKE) rpms
>>>>       mv $(srcdir)/version.m4.orig $(srcdir)/version.m4
>>>> endif
>>>>@@ -4221,7 +4230,7 @@ srpm: rpmbrprep
>>>> if GIT_CHECKOUT
>>>> prerelease-srpm:
>>>>       cp $(srcdir)/version.m4 $(srcdir)/version.m4.orig
>>>>-      sed -e "s/m4_define(\[PRERELEASE_VERSION_NUMBER\], 
>>>>\[.*\])/m4_define(\[PRERELEASE_VERSION_NUMBER\], \[.`date 
>>>>+%Y%m%d.%H%M`.git`git log -1 --pretty=format:%h`\])/" < 
>>>>$(srcdir)/version.m4.orig > $(srcdir)/version.m4
>>>>+      $(PR_SED) < $(srcdir)/version.m4.orig > $(srcdir)/version.m4
>>>>       $(MAKE) srpm
>>>>       mv $(srcdir)/version.m4.orig $(srcdir)/version.m4
>>>> endif
>>>>--
>>>>2.7.4
>>>>
>>> I would prefer to have sed directly rather then PR_SED.
>>> -       $(PR_SED) < $(srcdir)/version.m4.orig > $(srcdir)/version.m4
>>> +       sed -e "s/$(PR_VERSION_REGEX)/$(PR_VERSION_REPL)/" \
>>> +               < $(srcdir)/version.m4.orig > $(srcdir)/version.m4
>>>
>>> IMHO, it is more obvious what code want to do.
>>>
>>> >From 22d8574e66273f60a519fb0e230852c58c39e7f8 Mon Sep 17 00:00:00 2001
>>>>From: =?UTF-8?q?Fabiano=20Fid=C3=AAncio?= <[email protected]>
>>>>Date: Wed, 17 Aug 2016 21:29:56 +0200
>>>>Subject: [PATCH 2/3] BUILD: Improve prerelease targets
>>>>MIME-Version: 1.0
>>>>Content-Type: text/plain; charset=UTF-8
>>>>Content-Transfer-Encoding: 8bit
>>>>
>>>>By generating the rpms/srpm resulting of the prerelease targets on its
>>>>own specific directory, $(BUILDROOT)/{RPMS,SRPM}/$(PR_VERSION_NUMBER),
>>>>makes easier for the person who is building these packages to just copy
>>>>the whole content of that directory instead of having to search/select
>>>>whichever are the files generated by the last or penultimate build.
>>>>
>>>>A big note here is a must: the format of the rpms/srpm names did not
>>>>change, what changes is just the final directory they can be found in.
>>>>
>>>>Signed-off-by: Fabiano Fidêncio <[email protected]>
>>>>---
>>>> Makefile.am | 11 ++++++++++-
>>>> 1 file changed, 10 insertions(+), 1 deletion(-)
>>>>
>>>>diff --git a/Makefile.am b/Makefile.am
>>>>index 7e14d5c..0902d83 100644
>>>>--- a/Makefile.am
>>>>+++ b/Makefile.am
>>>>@@ -4197,6 +4197,8 @@ PR_VERSION_NUMBER = 
>>>>$(PR_VERSION_DATE).git$(PR_VERSION_COMMIT_HASH)
>>>> PR_VERSION_REGEX = m4_define(\[PRERELEASE_VERSION_NUMBER\], \[.*\])
>>>> PR_VERSION_REPL = m4_define(\[PRERELEASE_VERSION_NUMBER\], 
>>>> \[.$(PR_VERSION_NUMBER)\])
>>>> PR_SED = sed -e "s/$(PR_VERSION_REGEX)/$(PR_VERSION_REPL)/"
>>>>+PR_RPMS_EXTRA_ARGS =
>>>>+PR_SRPMS_EXTRA_ARGS =
>>>>
>>>> rpmbrprep: dist-gzip rpmroot
>>>> if GIT_CHECKOUT
>>>>@@ -4210,9 +4212,15 @@ endif
>>>>
>>>> rpms: rpmbrprep
>>>>       cd $(RPMBUILD); \
>>>>-      rpmbuild --define "_topdir $(RPMBUILD)" -ba SPECS/sssd.spec
>>>>+      rpmbuild --define "_topdir $(RPMBUILD)" \
>>>>+               $(PR_RPMS_EXTRA_ARGS) \
>>>>+               $(PR_SRPMS_EXTRA_ARGS) \
>>>>+               -ba SPECS/sssd.spec
>>>>
>>>> if GIT_CHECKOUT
>>>>+PR_RPMS_EXTRA_ARGS += --define "_rpmdir 
>>>>$(RPMBUILD)/RPMS/$(PR_VERSION_NUMBER)"
>>>>+PR_SRPMS_EXTRA_ARGS += --define "_srcrpmdir 
>>>>$(RPMBUILD)/SRPMS/$(PR_VERSION_NUMBER)"
>>>>+
>>> According to the patch I assume you want to store
>>> in rpms in subdirectory "$(PR_VERSION_NUMBER)"
>>> only for prerelease-* targets.
>>>
>>> However it's stored there also for
>>> simple targets as well
>>> e.g
>>>
>>> make srpm
>>> //snip
>>>
>>> rpmbuild --define "_topdir ~/sssd/rpmbuild" \
>>>          --define "_srcrpmdir 
>>> ~/sssd/rpmbuild/SRPMS/20160829.1328.gitb6a95cc" \
>>>          -bs SPECS/sssd.spec
>>> Wrote: 
>>> ~/sssd/rpmbuild/SRPMS/20160829.1328.gitb6a95cc/sssd-1.14.2-0.fc25.src.rpm
>>>> prerelease-rpms:
>>>>       cp $(srcdir)/version.m4 $(srcdir)/version.m4.orig
>>>>       $(PR_SED) < $(srcdir)/version.m4.orig > $(srcdir)/version.m4
>>>>@@ -4225,6 +4233,7 @@ endif
>>>> srpm: rpmbrprep
>>>>       cd $(RPMBUILD); \
>>>>       rpmbuild --define "_topdir $(RPMBUILD)" \
>>>>+               $(PR_SRPMS_EXTRA_ARGS) \
>>>>                -bs SPECS/sssd.spec
>>>>
>>>> if GIT_CHECKOUT
>>>>--
>>>>2.7.4
>>>>
>>>
>>> >From 383b2ebc1d442a3668ffce48c945b4038dd2b4a7 Mon Sep 17 00:00:00 2001
>>>>From: =?UTF-8?q?Fabiano=20Fid=C3=AAncio?= <[email protected]>
>>>>Date: Wed, 17 Aug 2016 21:53:44 +0200
>>>>Subject: [PATCH 3/3] BUILD: Consider seconds when building PR_VERSION_DATE
>>>>MIME-Version: 1.0
>>>>Content-Type: text/plain; charset=UTF-8
>>>>Content-Transfer-Encoding: 8bit
>>>>
>>>>As the "srpm" target may be fast enough to be run twice in the same
>>>>minute, let's also add the seconds to the PR_VERSION_DATE thus avoiding
>>>>to override the penultime first build.
>>>>
>>>>Upgrading packages from a prerelease target that didn't takes the
>>>>seconds into account to a prerelease target that does is not a problem
>>>>and it has been tested beforehand.
>>>>
>>>>Signed-off-by: Fabiano Fidêncio <[email protected]>
>>>>---
>>>> Makefile.am | 2 +-
>>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>>
>>>>diff --git a/Makefile.am b/Makefile.am
>>>>index 0902d83..30d874e 100644
>>>>--- a/Makefile.am
>>>>+++ b/Makefile.am
>>>>@@ -4191,7 +4191,7 @@ rpmroot:
>>>>
>>>> # pre-release related vars
>>>>
>>>>-PR_VERSION_DATE := $(shell date +%Y%m%d.%H%M)
>>>>+PR_VERSION_DATE := $(shell date +%Y%m%d.%H%M%S)
>>>> PR_VERSION_COMMIT_HASH := $(shell git log -1 --pretty=format:%h)
>>>> PR_VERSION_NUMBER = $(PR_VERSION_DATE).git$(PR_VERSION_COMMIT_HASH)
>>>> PR_VERSION_REGEX = m4_define(\[PRERELEASE_VERSION_NUMBER\], \[.*\])
>>>
>>> -1
>>>
>>> fedora packaging guidelines recommend just a YYYYMMDD
>>> https://fedoraproject.org/w/index.php?title=Packaging:Versioning
>>>
>>> I'm file with "%H%M" because it's sometimes simpler to
>>> compare 4 numbers rather than 7 characters hash.
>>> Comparing 6 numers for date would be almost the &&  same
>>> as comparing 7 characters for hash. So "%H%M%S" would
>>> be redundant.
>>>
>>> BTW even though there can be 2 prerelease rpms done
>>> within one minute than git hash should be different.
>>> Otherwise there is something weird in developers workflow :-)
>>>
>>> LS
>>
>>Being completely honest, at this point I feel like we can drop this
>>whole series.
>>I just got used to "rm -rf rpmbuild && make prerelease-rpms"
>>
>>Lukaš, is there something that is of your interest in this series? If
>>yes, I'll re-work and re-send the patches, otherwise we can just leave
>>everything as it is nowadays.
>>
>
> I think we can merge the 1st patch after replacing sed :-)
> The current version with 214 columns in a line is not very nice

Here we go then.
See the attached patch.

>
> LS
> _______________________________________________
> sssd-devel mailing list
> [email protected]
> https://lists.fedorahosted.org/admin/lists/[email protected]
From 35a567ced9971f3c2e4fe75311262e4b92436f18 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Fabiano=20Fid=C3=AAncio?= <[email protected]>
Date: Wed, 17 Aug 2016 21:08:23 +0200
Subject: [PATCH v2] BUILD: Clean up prerelease targets
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Clean up the pre-release targets in order to avoid lines exceeding 80
characters.

Signed-off-by: Fabiano Fidêncio <[email protected]>
---
 Makefile.am | 14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/Makefile.am b/Makefile.am
index 6299ac7..4a56d8b 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -4202,6 +4202,14 @@ rpmroot:
 	$(MKDIR_P) $(RPMBUILD)/SPECS
 	$(MKDIR_P) $(RPMBUILD)/SRPMS
 
+# pre-release related vars
+
+PR_VERSION_DATE := $(shell date +%Y%m%d.%H%M)
+PR_VERSION_COMMIT_HASH := $(shell git log -1 --pretty=format:%h)
+PR_VERSION_NUMBER = $(PR_VERSION_DATE).git$(PR_VERSION_COMMIT_HASH)
+PR_VERSION_REGEX = m4_define(\[PRERELEASE_VERSION_NUMBER\], \[.*\])
+PR_VERSION_REPL = m4_define(\[PRERELEASE_VERSION_NUMBER\], \[.$(PR_VERSION_NUMBER)\])
+
 rpmbrprep: dist-gzip rpmroot
 if GIT_CHECKOUT
 # When we're building RPMs from a git checkout,
@@ -4219,7 +4227,8 @@ rpms: rpmbrprep
 if GIT_CHECKOUT
 prerelease-rpms:
 	cp $(srcdir)/version.m4 $(srcdir)/version.m4.orig
-	sed -e "s/m4_define(\[PRERELEASE_VERSION_NUMBER\], \[.*\])/m4_define(\[PRERELEASE_VERSION_NUMBER\], \[.`date +%Y%m%d.%H%M`.git`git log -1 --pretty=format:%h`\])/" < $(srcdir)/version.m4.orig > $(srcdir)/version.m4
+	sed -e "s/$(PR_VERSION_REGEX)/$(PR_VERSION_REPL)/" \
+		< $(srcdir)/version.m4.orig > $(srcdir)/version.m4
 	$(MAKE) rpms
 	mv $(srcdir)/version.m4.orig $(srcdir)/version.m4
 endif
@@ -4234,7 +4243,8 @@ srpm: rpmbrprep
 if GIT_CHECKOUT
 prerelease-srpm:
 	cp $(srcdir)/version.m4 $(srcdir)/version.m4.orig
-	sed -e "s/m4_define(\[PRERELEASE_VERSION_NUMBER\], \[.*\])/m4_define(\[PRERELEASE_VERSION_NUMBER\], \[.`date +%Y%m%d.%H%M`.git`git log -1 --pretty=format:%h`\])/" < $(srcdir)/version.m4.orig > $(srcdir)/version.m4
+	sed -e "s/$(PR_VERSION_REGEX)/$(PR_VERSION_REPL)/" \
+		< $(srcdir)/version.m4.orig > $(srcdir)/version.m4
 	$(MAKE) srpm
 	mv $(srcdir)/version.m4.orig $(srcdir)/version.m4
 endif
-- 
2.7.4

_______________________________________________
sssd-devel mailing list
[email protected]
https://lists.fedorahosted.org/admin/lists/[email protected]

Reply via email to