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

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

Reply via email to