Re: [Rpm-maint] [rpm-software-management/rpm] Dynamic Spec generation (#1485)
Merged #1485 into master. -- Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/1485#event-7774057590 You are receiving this because you are subscribed to this thread. Message ID: ___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Dynamic Spec generation (#1485)
@pmatilai approved this pull request. Okay the spec parse stuff looks a whole lot nicer now - it actually looks like an improvement. I intended to do some more practical testing with this, but enough is enough :sweat_smile: -- Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/1485#pullrequestreview-1174144690 You are receiving this because you are subscribed to this thread. Message ID: ___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Dynamic Spec generation (#1485)
Rebased on top of #2270 and squashed latest fixes. -- Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/1485#issuecomment-1308683146 You are receiving this because you are subscribed to this thread. Message ID: ___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Dynamic Spec generation (#1485)
@ffesti pushed 3 commits. a1056d0b1487ee921db9de8d53bf693976e1f0b5 Split actual parsing of spec into its own function a2592627c300ddbe9af1632d0681391567213791 Allow starting new spec parts with PART_EMPTY 26de93c844ee13d456542e867cf35a60e574c90b Add Dynamic Spec generation -- View it on GitHub: https://github.com/rpm-software-management/rpm/pull/1485/files/5b4d7065b2f720dd098db92234f496ae59a03882..26de93c844ee13d456542e867cf35a60e574c90b You are receiving this because you are subscribed to this thread. Message ID: ___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Dynamic Spec generation (#1485)
OK, seems like all the comments are now addressed. -- Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/1485#issuecomment-1308622418 You are receiving this because you are subscribed to this thread. Message ID: ___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Dynamic Spec generation (#1485)
> Added alternative error handling. Turns out to be less of a change than I > expected. So it's probably just the way to go. Yup, looks good. -- Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/1485#issuecomment-1308613037 You are receiving this because you are subscribed to this thread. Message ID: ___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Dynamic Spec generation (#1485)
@ffesti pushed 1 commit. 5b4d7065b2f720dd098db92234f496ae59a03882 Dynamic Specs: Add order of specparts reading -- View it on GitHub: https://github.com/rpm-software-management/rpm/pull/1485/files/a2ec843b8dbc11ece43f057092b0f768931c933c..5b4d7065b2f720dd098db92234f496ae59a03882 You are receiving this because you are subscribed to this thread. Message ID: ___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Dynamic Spec generation (#1485)
@pmatilai commented on this pull request. > @@ -1129,3 +1155,29 @@ rpmSpec rpmSpecParse(const char *specFile, > rpmSpecFlags flags, { return parseSpec(specFile, flags, buildRoot, 0); } + +rpmRC parseGeneratedSpecs(rpmSpec spec) +{ +ARGV_t argv = NULL; +int argc = 0; +int i; +rpmRC rc = RPMRC_OK; + +char * specPattern = rpmGenPath("%{specpartsdir}", NULL, "*.specpart"); +if (rpmGlob(specPattern, , ) == 0) { One more thing I realized earlier today but ended up on a long detour... there's a subtle but important thing silently done by rpmGlob() here: it sorts the returned files. Without which, users of this feature may get a pretty weird spec salad of the day. I'd add a comment here because that's a not-so-known feature of rpmGlob(), and also document the order (sorted according to C locale) because it's something users will need to care about. -- Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/1485#pullrequestreview-1172214804 You are receiving this because you are subscribed to this thread. Message ID: ___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Dynamic Spec generation (#1485)
@pmatilai commented on this pull request. > + if (checkForEncoding(pkg->header, 0) != RPMRC_OK) { + badenc = 1; + } + } + if (badenc) + goto errxit; +} + +closeSpec(spec); + +exit: +return spec; + +errxit: +if (!secondary) + rpmSpecFree(spec); Hmm, the API shouldn't get much in the way as parseSpec() and friends is all internal affairs. I'm too cross-eyed to look at this today but glad you found a way forward, -- Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/1485#discussion_r1016688097 You are receiving this because you are subscribed to this thread. Message ID: ___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Dynamic Spec generation (#1485)
Added alternative error handling. Turns out to be less of a change than I expected. So it's probably just the way to go. -- Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/1485#issuecomment-1307049064 You are receiving this because you are subscribed to this thread. Message ID: ___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Dynamic Spec generation (#1485)
@ffesti pushed 1 commit. a2ec843b8dbc11ece43f057092b0f768931c933c Move rpmSpecFree out of parseSpecSection -- View it on GitHub: https://github.com/rpm-software-management/rpm/pull/1485/files/a07501c24c391ed091f72c2933f259bb617c6185..a2ec843b8dbc11ece43f057092b0f768931c933c You are receiving this because you are subscribed to this thread. Message ID: ___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Dynamic Spec generation (#1485)
@ffesti commented on this pull request. > + if (checkForEncoding(pkg->header, 0) != RPMRC_OK) { + badenc = 1; + } + } + if (badenc) + goto errxit; +} + +closeSpec(spec); + +exit: +return spec; + +errxit: +if (!secondary) + rpmSpecFree(spec); I am going to try another variant with the spec being a & parameter and the freeing is done outside of parseSpecSection(). That may be more to the book even if it doesn't really simplify things. -- Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/1485#discussion_r1016461228 You are receiving this because you are subscribed to this thread. Message ID: ___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Dynamic Spec generation (#1485)
@ffesti commented on this pull request. > + if (checkForEncoding(pkg->header, 0) != RPMRC_OK) { + badenc = 1; + } + } + if (badenc) + goto errxit; +} + +closeSpec(spec); + +exit: +return spec; + +errxit: +if (!secondary) + rpmSpecFree(spec); OK, I was able to at least get rid of the `rpmSpecFree` in `parseSpec` which is just not needed. The API sets some hard limits on what we can do here. While parseSpec is returning NULL on error we can't free the spec when parsing dynamic spec parts as the surrounding code does not expect the spec object to go away at that point. -- Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/1485#discussion_r1016441627 You are receiving this because you are subscribed to this thread. Message ID: ___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Dynamic Spec generation (#1485)
- Removed u2p macro - Added _prefix to test case - Removed unnneccessary rpmSpecFree call -- Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/1485#issuecomment-1306984561 You are receiving this because you are subscribed to this thread. Message ID: ___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Dynamic Spec generation (#1485)
@ffesti commented on this pull request. > + +runroot rpmbuild -ba /data/SPECS/dynamic.spec +], +[0], +[ignore], +[ignore]) + +AT_CHECK([ + +runroot rpm -qp --qf "%{Summary}\n" /build/RPMS/noarch/dynamic-docs-1.0-1.noarch.rpm +runroot rpm -ql /build/RPMS/noarch/dynamic-docs-1.0-1.noarch.rpm +], +[0], +[Documentation for dynamic spec +/usr/local/share/doc/dynamic-docs-1.0 +/usr/local/share/doc/dynamic-docs-1.0/FAQ Done. -- Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/1485#discussion_r1016434111 You are receiving this because you are subscribed to this thread. Message ID: ___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Dynamic Spec generation (#1485)
@ffesti commented on this pull request. > @@ -715,7 +715,8 @@ package or when debugging this package.\ RPM_ARCH=\"%{_arch}\"\ RPM_OS=\"%{_os}\"\ RPM_BUILD_NCPUS=\"%{_smp_build_ncpus}\"\ - export RPM_SOURCE_DIR RPM_BUILD_DIR RPM_OPT_FLAGS RPM_ARCH RPM_OS RPM_BUILD_NCPUS\ + RPM_SPECPARTS_DIR=\"%{u2p:%{specpartsdir}}\"\ Done. -- Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/1485#discussion_r1016433878 You are receiving this because you are subscribed to this thread. Message ID: ___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Dynamic Spec generation (#1485)
@ffesti pushed 3 commits. 825d644e5654c1898849c54eb2b7327a9044f7da Split actual parsing of spec into its own function a3f967656ccdec466653a12301c7449f9f746975 Allow starting new spec parts with PART_EMPTY a07501c24c391ed091f72c2933f259bb617c6185 Add Dynamic Spec generation -- View it on GitHub: https://github.com/rpm-software-management/rpm/pull/1485/files/387da564de03e2c12200d09abf746690b103af86..a07501c24c391ed091f72c2933f259bb617c6185 You are receiving this because you are subscribed to this thread. Message ID: ___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Dynamic Spec generation (#1485)
@pmatilai commented on this pull request. > + +runroot rpmbuild -ba /data/SPECS/dynamic.spec +], +[0], +[ignore], +[ignore]) + +AT_CHECK([ + +runroot rpm -qp --qf "%{Summary}\n" /build/RPMS/noarch/dynamic-docs-1.0-1.noarch.rpm +runroot rpm -ql /build/RPMS/noarch/dynamic-docs-1.0-1.noarch.rpm +], +[0], +[Documentation for dynamic spec +/usr/local/share/doc/dynamic-docs-1.0 +/usr/local/share/doc/dynamic-docs-1.0/FAQ This will fail when rpm is built with prefix other than /usr/local, such as distros would normally do. Run the above rpmbuild with `--define "_prefix /usr/local"` to fix. -- Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/1485#pullrequestreview-1171591018 You are receiving this because you are subscribed to this thread. Message ID: ___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Dynamic Spec generation (#1485)
@pmatilai commented on this pull request. > @@ -715,7 +715,8 @@ package or when debugging this package.\ RPM_ARCH=\"%{_arch}\"\ RPM_OS=\"%{_os}\"\ RPM_BUILD_NCPUS=\"%{_smp_build_ncpus}\"\ - export RPM_SOURCE_DIR RPM_BUILD_DIR RPM_OPT_FLAGS RPM_ARCH RPM_OS RPM_BUILD_NCPUS\ + RPM_SPECPARTS_DIR=\"%{u2p:%{specpartsdir}}\"\ Rebase on top of #2267 and drop the u2p here too. Sorry for not spotting this earlier. -- Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/1485#pullrequestreview-1171564549 You are receiving this because you are subscribed to this thread. Message ID: ___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Dynamic Spec generation (#1485)
@pmatilai commented on this pull request. > + if (checkForEncoding(pkg->header, 0) != RPMRC_OK) { + badenc = 1; + } + } + if (badenc) + goto errxit; +} + +closeSpec(spec); + +exit: +return spec; + +errxit: +if (!secondary) + rpmSpecFree(spec); Of course it needs a spec pointer. The point here is to avoid two paths of error cleanup in different functions which is just the kind of murky environment that all manner of unpleasant creatures thrive. We can't fix everything that is crazy in there, but never ever make bad code worse. Find a way, I'm sure there is one or more. My musings above were just ways I'd look into resolving it, there may be others/better ones. -- Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/1485#discussion_r1016217062 You are receiving this because you are subscribed to this thread. Message ID: ___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Dynamic Spec generation (#1485)
@pmatilai commented on this pull request. > @@ -273,6 +273,15 @@ static int doSetupMacro(rpmSpec spec, const char *line) free(buf); } +/* mkdir for dynamic specparts */ +buf = rpmExpand("%{__mkdir} SPECPARTS", NULL); +appendBuf(spec, buf, 1); +free(buf); + +buf = rpmGenPath("%{u2p:%{_builddir}}", "%{buildsubdir}", "SPECPARTS"); Oh, I hadn't realized we have u2p being used someplace still. Lets get rid of those (but not in scope for this of course) -- Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/1485#discussion_r1016200548 You are receiving this because you are subscribed to this thread. Message ID: ___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Dynamic Spec generation (#1485)
OK, addressed all the comments above except the issue with the signature of `parseSpecSection()` where I not quite see what a better solution would be. Fixed for now: - Reworded documentation - Merged spec files for test cases - Failing test case checks rpmbuild output - removed spurious u2p macro -- Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/1485#issuecomment-1305701782 You are receiving this because you are subscribed to this thread. Message ID: ___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Dynamic Spec generation (#1485)
@ffesti commented on this pull request. > + if (checkForEncoding(pkg->header, 0) != RPMRC_OK) { + badenc = 1; + } + } + if (badenc) + goto errxit; +} + +closeSpec(spec); + +exit: +return spec; + +errxit: +if (!secondary) + rpmSpecFree(spec); The issue here is that we want to alter the spec object with parseSpecSection() for the dynamic spec feature. So we need to pass the spec object no matter what. Doing the object creating in parseSpecSection() only bloats that function and increases the different behaviors for the two use cases. Not quite sure what you mean with "handling it inline specParse()". We still need something we can pass the spec object to. While I am fine with moving stuff out of specParse() I think this is a bit out of scope here. -- Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/1485#discussion_r1015488140 You are receiving this because you are subscribed to this thread. Message ID: ___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Dynamic Spec generation (#1485)
@ffesti commented on this pull request. > @@ -273,6 +273,15 @@ static int doSetupMacro(rpmSpec spec, const char *line) free(buf); } +/* mkdir for dynamic specparts */ +buf = rpmExpand("%{__mkdir} SPECPARTS", NULL); +appendBuf(spec, buf, 1); +free(buf); + +buf = rpmGenPath("%{u2p:%{_builddir}}", "%{buildsubdir}", "SPECPARTS"); +rpmPushMacro(spec->macros, "specpartsdir", NULL, buf, RMIL_SPEC); The actual dir name is now removed from the docs but remains hard coded. -- Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/1485#discussion_r1015479632 You are receiving this because you are subscribed to this thread. Message ID: ___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Dynamic Spec generation (#1485)
@ffesti commented on this pull request. > +layout: default +title: rpm.org - Package Build Process +--- +# Dynamic Spec Generation + +Since rpm 4.19 RPM supports parsing dynamically generated specs. This +allows the build scripts (**%build** or **%install**) to create parts +of the spec file. The parts are read in and parsed after **%install** +and before **%check**. Because of this they obviously can't contain +the build scripts but are intended to create sub packages based on the +build results. + +The files need to be placed in the **%{specpartsdir}** (also available +as **$RPM_SPECPARTS_DIR** in the build scripts) and have a +**.specpart** postfix. The directory is created by **%setup** in the +**buildsubdir**. Scripts must fail if it is not present. They should Reworded. There are just a little to many ifs and whens to make this nice to read literature. Or my poetic qualities are just lacking... -- Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/1485#discussion_r1015478404 You are receiving this because you are subscribed to this thread. Message ID: ___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Dynamic Spec generation (#1485)
@ffesti commented on this pull request. > +%description +Simple rpm demonstration. + +%prep +%setup -q -T -c + +%build +echo "Q: Why?\nA: Because we can!" > FAQ + +%install +mkdir -p $RPM_BUILD_ROOT/usr/local/bin +echo " " > $RPM_BUILD_ROOT/usr/local/bin/hello + + +echo "%package docs" >> %{specpartsdir}/docs.specpart +echo "Summary: Documentation for dynamic spec" >> %{specpartsdir}/docs.specpart With both spec files now merged the section is marked by a macro query. -- Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/1485#discussion_r1015477125 You are receiving this because you are subscribed to this thread. Message ID: ___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Dynamic Spec generation (#1485)
@ffesti commented on this pull request. > @@ -273,6 +273,15 @@ static int doSetupMacro(rpmSpec spec, const char *line) free(buf); } +/* mkdir for dynamic specparts */ +buf = rpmExpand("%{__mkdir} SPECPARTS", NULL); +appendBuf(spec, buf, 1); +free(buf); + +buf = rpmGenPath("%{u2p:%{_builddir}}", "%{buildsubdir}", "SPECPARTS"); That's a good question. I could swear there's a line doing a similar thing just above for the buildroot. Turns out there isn't and `u2p` is only used in `macros.in`. Anyway, it gone now. -- Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/1485#discussion_r1015475658 You are receiving this because you are subscribed to this thread. Message ID: ___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Dynamic Spec generation (#1485)
@ffesti commented on this pull request. > + +%prep +%setup -q -T -c + +%build +echo "Q: Why?\nA: Because we can!" > FAQ + +%install +mkdir -p $RPM_BUILD_ROOT/usr/local/bin +echo " " > $RPM_BUILD_ROOT/usr/local/bin/hello + + +echo "%package docs" >> %{specpartsdir}/docs.specpart +echo "Summary: Documentation for dynamic spec" >> %{specpartsdir}/docs.specpart +echo "BuildArch: noarch" >> %{specpartsdir}/docs.specpart +echo "%files docs" >> %{specpartsdir}/docs.specpart Done. I guess I started with a fool prove variant to not debug the test case and the error handling at once. -- Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/1485#discussion_r1015474063 You are receiving this because you are subscribed to this thread. Message ID: ___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Dynamic Spec generation (#1485)
@ffesti commented on this pull request. > @@ -2272,3 +2272,44 @@ runroot rpmbuild \ ], [ignore]) AT_CLEANUP + +# -- +# Check if dynamic spec generation works +AT_SETUP([rpmbuild with dynamic spec generation]) +AT_KEYWORDS([build]) +RPMDB_INIT +AT_CHECK([ + +runroot rpmbuild -ba /data/SPECS/dynamic.spec +], +[0], +[ignore], +[ignore]) Done. -- Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/1485#discussion_r1015472698 You are receiving this because you are subscribed to this thread. Message ID: ___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Dynamic Spec generation (#1485)
@ffesti pushed 1 commit. 387da564de03e2c12200d09abf746690b103af86 Add Dynamic Spec generation -- View it on GitHub: https://github.com/rpm-software-management/rpm/pull/1485/files/ab78270fca71149a42fdead2b8dd14a0f2d0898c..387da564de03e2c12200d09abf746690b103af86 You are receiving this because you are subscribed to this thread. Message ID: ___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Dynamic Spec generation (#1485)
@pmatilai commented on this pull request. > + if (checkForEncoding(pkg->header, 0) != RPMRC_OK) { + badenc = 1; + } + } + if (badenc) + goto errxit; +} + +closeSpec(spec); + +exit: +return spec; + +errxit: +if (!secondary) + rpmSpecFree(spec); Another possibility may be a kind of opposite direction: handle it inline specParse(), just move stuff that doesn't belong there out of the way. I have no idea why I planted the openmp thread fubar right in the middle of specParse(), it sure does not have to be just there, it may just as well be done from rpmSpecParse() instead, probably ditto for various other pieces. -- Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/1485#discussion_r1015128682 You are receiving this because you are subscribed to this thread. Message ID: ___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Dynamic Spec generation (#1485)
@dmnks commented on this pull request. > +%description +Simple rpm demonstration. + +%prep +%setup -q -T -c + +%build +echo "Q: Why?\nA: Because we can!" > FAQ + +%install +mkdir -p $RPM_BUILD_ROOT/usr/local/bin +echo " " > $RPM_BUILD_ROOT/usr/local/bin/hello + + +echo "%package docs" >> %{specpartsdir}/docs.specpart +echo "Summary: Documentation for dynamic spec" >> %{specpartsdir}/docs.specpart Oh, yup, I confused the file :) -- Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/1485#discussion_r1015126857 You are receiving this because you are subscribed to this thread. Message ID: ___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Dynamic Spec generation (#1485)
@pmatilai commented on this pull request. > + if (checkForEncoding(pkg->header, 0) != RPMRC_OK) { + badenc = 1; + } + } + if (badenc) + goto errxit; +} + +closeSpec(spec); + +exit: +return spec; + +errxit: +if (!secondary) + rpmSpecFree(spec); I think just moving the initial spec allocation (+ other setup) to parseSpecSection() should make it a whole lot less lopsided wrt the allocation, and thus less ugly. We can't help the recursing from buildarch, but we're not going to make things *worse*. -- Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/1485#discussion_r1015123195 You are receiving this because you are subscribed to this thread. Message ID: ___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Dynamic Spec generation (#1485)
@pmatilai commented on this pull request. > @@ -2272,3 +2272,44 @@ runroot rpmbuild \ ], [ignore]) AT_CLEANUP + +# -- +# Check if dynamic spec generation works +AT_SETUP([rpmbuild with dynamic spec generation]) +AT_KEYWORDS([build]) +RPMDB_INIT +AT_CHECK([ + +runroot rpmbuild -ba /data/SPECS/dynamic.spec +], +[0], +[ignore], +[ignore]) I'd think the error message should be easy enough to catch here, especially if building with --quiet (as most of the test-suite does). -- Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/1485#pullrequestreview-1169945357 You are receiving this because you are subscribed to this thread. Message ID: ___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Dynamic Spec generation (#1485)
@pmatilai commented on this pull request. > +%description +Simple rpm demonstration. + +%prep +%setup -q -T -c + +%build +echo "Q: Why?\nA: Because we can!" > FAQ + +%install +mkdir -p $RPM_BUILD_ROOT/usr/local/bin +echo " " > $RPM_BUILD_ROOT/usr/local/bin/hello + + +echo "%package docs" >> %{specpartsdir}/docs.specpart +echo "Summary: Documentation for dynamic spec" >> %{specpartsdir}/docs.specpart I suppose this comment was meant for the dynamic-fail.spec instead. But yeah, just having a fail case here depending on a macro would make it more obvious too. -- Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/1485#discussion_r1015110163 You are receiving this because you are subscribed to this thread. Message ID: ___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Dynamic Spec generation (#1485)
@pmatilai commented on this pull request. > + +%prep +%setup -q -T -c + +%build +echo "Q: Why?\nA: Because we can!" > FAQ + +%install +mkdir -p $RPM_BUILD_ROOT/usr/local/bin +echo " " > $RPM_BUILD_ROOT/usr/local/bin/hello + + +echo "%package docs" >> %{specpartsdir}/docs.specpart +echo "Summary: Documentation for dynamic spec" >> %{specpartsdir}/docs.specpart +echo "BuildArch: noarch" >> %{specpartsdir}/docs.specpart +echo "%files docs" >> %{specpartsdir}/docs.specpart This fail case could be trivially handled with the same spec as the success case, just add a macro that swaps the behavior as needed. That's how most of the test-suite is done to avoid adding dozens of specs only differing by a line or two. -- Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/1485#pullrequestreview-1169942445 You are receiving this because you are subscribed to this thread. Message ID: ___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Dynamic Spec generation (#1485)
@pmatilai commented on this pull request. > @@ -273,6 +273,15 @@ static int doSetupMacro(rpmSpec spec, const char *line) free(buf); } +/* mkdir for dynamic specparts */ +buf = rpmExpand("%{__mkdir} SPECPARTS", NULL); +appendBuf(spec, buf, 1); +free(buf); + +buf = rpmGenPath("%{u2p:%{_builddir}}", "%{buildsubdir}", "SPECPARTS"); What's with this u2p thing? %_builddir is just a plain old directory, we really don't want any http:// or ftp:// handling in there. -- Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/1485#pullrequestreview-1169919570 You are receiving this because you are subscribed to this thread. Message ID: ___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Dynamic Spec generation (#1485)
@ffesti commented on this pull request. > @@ -1129,3 +1157,26 @@ rpmSpec rpmSpecParse(const char *specFile, > rpmSpecFlags flags, { return parseSpec(specFile, flags, buildRoot, 0); } + +rpmRC parseGeneratedSpecs(rpmSpec spec) +{ +ARGV_t argv = NULL; +int argc = 0; +int i; + +char * specPattern = rpmGenPath("%{specpartsdir}", NULL, "*.specpart"); +if (rpmGlob(specPattern, , ) == 0) { + for (i = 0; i < argc; i++) { + rpmlog(RPMLOG_NOTICE, "Reading %s\n", argv[i]); + pushOFI(spec, argv[i]); + snprintf(spec->fileStack->readBuf, spec->fileStack->readBufLen, +"# Spec part read from %s\n\n", argv[i]); + if (parseSpecSection(spec, 1)) { > Do stuff for others to clean up is not an attitude us maintainers can afford > you know. Yes, I know. I do not actually expect anyone to clean this up. I expect this to stay that way as long as we have BUILDARCH - which is forever. -- Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/1485#discussion_r1015091472 You are receiving this because you are subscribed to this thread. Message ID: ___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Dynamic Spec generation (#1485)
> Yes, this is super ugly but nothing I am touching while adding this new > feature. Anyone feel free to clean that up afterwards. Do stuff for others to clean up is not an attitude us maintainers can afford you know. -- Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/1485#issuecomment-1305187602 You are receiving this because you are subscribed to this thread. Message ID: ___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Dynamic Spec generation (#1485)
@dmnks commented on this pull request. LGTM now, only little nitpicks, see comments. > +layout: default +title: rpm.org - Package Build Process +--- +# Dynamic Spec Generation + +Since rpm 4.19 RPM supports parsing dynamically generated specs. This +allows the build scripts (**%build** or **%install**) to create parts +of the spec file. The parts are read in and parsed after **%install** +and before **%check**. Because of this they obviously can't contain +the build scripts but are intended to create sub packages based on the +build results. + +The files need to be placed in the **%{specpartsdir}** (also available +as **$RPM_SPECPARTS_DIR** in the build scripts) and have a +**.specpart** postfix. The directory is created by **%setup** in the +**buildsubdir**. Scripts must fail if it is not present. They should This sentence sounds contradictory to what follows it, i.e. that they in fact don't have to fail if they do some kind of fallback. > +%description +Simple rpm demonstration. + +%prep +%setup -q -T -c + +%build +echo "Q: Why?\nA: Because we can!" > FAQ + +%install +mkdir -p $RPM_BUILD_ROOT/usr/local/bin +echo " " > $RPM_BUILD_ROOT/usr/local/bin/hello + + +echo "%package docs" >> %{specpartsdir}/docs.specpart +echo "Summary: Documentation for dynamic spec" >> %{specpartsdir}/docs.specpart Nitpick, but I'd add a comment saying that a `%description` is missing here which makes the build fail (otherwise it's not that much apparent from the looks of it). -- Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/1485#pullrequestreview-1168693722 You are receiving this because you are subscribed to this thread. Message ID: ___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Dynamic Spec generation (#1485)
OK, fixed the error handling and added a failing test case. Added $RPM_SPECPARTS_DIR to the build scripts and removed the true directory name from the docs. Turns out there is a reason that `parseSpecSection()` returns a `Spec` object: The BUILDARCHITECTURES magic may return a different Spec object. Yes, this is super ugly but nothing I am touching while adding this new feature. Anyone feel free to clean that up afterwards. -- Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/1485#issuecomment-1303566982 You are receiving this because you are subscribed to this thread. Message ID: ___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Dynamic Spec generation (#1485)
@ffesti pushed 3 commits. a63662e1558a086a850a2da382bd7c5759a1d7ca Split actual parsing of spec into its own function 05e784b9b576aefd88522c88ab06133048c7904d Allow starting new spec parts with PART_EMPTY ab78270fca71149a42fdead2b8dd14a0f2d0898c Add Dynamic Spec generation -- View it on GitHub: https://github.com/rpm-software-management/rpm/pull/1485/files/06c764a2625fe2daea62598075bac2d10fc42317..ab78270fca71149a42fdead2b8dd14a0f2d0898c You are receiving this because you are subscribed to this thread. Message ID: ___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Dynamic Spec generation (#1485)
@pmatilai commented on this pull request. > @@ -273,6 +273,15 @@ static int doSetupMacro(rpmSpec spec, const char *line) free(buf); } +/* mkdir for dynamic specparts */ +buf = rpmExpand("%{__mkdir} SPECPARTS", NULL); +appendBuf(spec, buf, 1); +free(buf); + +buf = rpmGenPath("%{u2p:%{_builddir}}", "%{buildsubdir}", "SPECPARTS"); +rpmPushMacro(spec->macros, "specpartsdir", NULL, buf, RMIL_SPEC); Hmm, except maybe that's a step too far, we don't have separate macros for SPECS, SOURCES and the like either. Internal uses we can always fix if necessary, what matters is that packages should never hardcode the value. And for that, we must export it in the build scriptlet environment (similar to $RPM_BUILD_ROOT) and document that as the thing to use. -- Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/1485#discussion_r1012656185 You are receiving this because you are subscribed to this thread. Message ID: ___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Dynamic Spec generation (#1485)
...or maybe we should get into the habit of including these design docs in a directory someplace in the source tree. -- Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/1485#issuecomment-1301768925 You are receiving this because you are subscribed to this thread. Message ID: ___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Dynamic Spec generation (#1485)
> Final patch might still deserve a bit longer commit message. Otoh there is > docs on this and there is no need to duplicated that in the patch. Yep, there's no point duplicating the user docs in the commit message. What belongs in the commit message is a condensed version of the design spec: https://github.com/rpm-software-management/rpm/discussions/2032 Because, commit messages are forever, a discussion on a proprietary platform is less so. -- Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/1485#issuecomment-1301767497 You are receiving this because you are subscribed to this thread. Message ID: ___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Dynamic Spec generation (#1485)
@pmatilai commented on this pull request. > @@ -1129,3 +1157,26 @@ rpmSpec rpmSpecParse(const char *specFile, > rpmSpecFlags flags, { return parseSpec(specFile, flags, buildRoot, 0); } + +rpmRC parseGeneratedSpecs(rpmSpec spec) +{ +ARGV_t argv = NULL; +int argc = 0; +int i; + +char * specPattern = rpmGenPath("%{specpartsdir}", NULL, "*.specpart"); +if (rpmGlob(specPattern, , ) == 0) { + for (i = 0; i < argc; i++) { + rpmlog(RPMLOG_NOTICE, "Reading %s\n", argv[i]); + pushOFI(spec, argv[i]); + snprintf(spec->fileStack->readBuf, spec->fileStack->readBufLen, +"# Spec part read from %s\n\n", argv[i]); + if (parseSpecSection(spec, 1)) { Yup. Since parseSpecSection() doesn't actually allocate the spec structure, it should return a simple 0/1 error code instead of a pointer. -- Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/1485#discussion_r1012592935 You are receiving this because you are subscribed to this thread. Message ID: ___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Dynamic Spec generation (#1485)
@pmatilai commented on this pull request. > + if (checkForEncoding(pkg->header, 0) != RPMRC_OK) { + badenc = 1; + } + } + if (badenc) + goto errxit; +} + +closeSpec(spec); + +exit: +return spec; + +errxit: +if (!secondary) + rpmSpecFree(spec); Indeed. The way it gets called, it probably doesn't matter as parseSpecSection() returns an explicit NULL on error, but freeing the struct is the responsibility of the caller. There's also something broken in the error handling: an invalid spec part does not abort the build, although an error message can be seen in the log. Do add a test for that too, just reuse the same spec but leave a mandatory part out or something. -- Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/1485#discussion_r1012591860 You are receiving this because you are subscribed to this thread. Message ID: ___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Dynamic Spec generation (#1485)
@pmatilai commented on this pull request. > @@ -273,6 +273,15 @@ static int doSetupMacro(rpmSpec spec, const char *line) free(buf); } +/* mkdir for dynamic specparts */ +buf = rpmExpand("%{__mkdir} SPECPARTS", NULL); +appendBuf(spec, buf, 1); +free(buf); + +buf = rpmGenPath("%{u2p:%{_builddir}}", "%{buildsubdir}", "SPECPARTS"); +rpmPushMacro(spec->macros, "specpartsdir", NULL, buf, RMIL_SPEC); Indeed, I was thinking about a macro for SPECPARTS specifically, and drop any references to the explicit value from both the code *and documentation*. -- Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/1485#discussion_r1012579408 You are receiving this because you are subscribed to this thread. Message ID: ___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Dynamic Spec generation (#1485)
@dmnks commented on this pull request. > @@ -273,6 +273,15 @@ static int doSetupMacro(rpmSpec spec, const char *line) free(buf); } +/* mkdir for dynamic specparts */ +buf = rpmExpand("%{__mkdir} SPECPARTS", NULL); +appendBuf(spec, buf, 1); +free(buf); + +buf = rpmGenPath("%{u2p:%{_builddir}}", "%{buildsubdir}", "SPECPARTS"); +rpmPushMacro(spec->macros, "specpartsdir", NULL, buf, RMIL_SPEC); This defines the `specpartsdir` macro for use in a specpart file, however the directory name `SPECPARTS` is still hardcoded. Is that what we want? -- Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/1485#pullrequestreview-1162144946 You are receiving this because you are subscribed to this thread. Message ID: ___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Dynamic Spec generation (#1485)
Oh, and apologies for the noise here, I should've raised this a while ago in the design [document](https://github.com/rpm-software-management/rpm/discussions/2032) instead, and have it answered there. -- Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/1485#issuecomment-1297316049 You are receiving this because you are subscribed to this thread. Message ID: ___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Dynamic Spec generation (#1485)
There's another reason *not* to mess with the semantics of `%include`, which is, most people likely understand it as a C preprocessor-like counterpart to `#include`, rather than some kind of dynamic feature. Therefore separating those two (also by using different naming conventions) is arguably better. -- Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/1485#issuecomment-1297311967 You are receiving this because you are subscribed to this thread. Message ID: ___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Dynamic Spec generation (#1485)
> There are a few reasons why this is not wired into `%include`. The first > being that `%include` includes a file in place and then continues to read, > expand and parse the rest of the file. This is something that just cannot be > done afterwards. All kind of things could change by re-parsing the spec file. > This is a mess we really don't want (and still have with the BuildArch > parsing mess). Yup, what I meant was, have an `%include` variant that does *not* get expanded during the first pass but rather registers a filename that gets parsed afterwards. So basically, from the user's perspective, it's a special flavour of `%include` that just gets expanded at a later time. The implementation would basically look the same as yours now, with a few tweaks. That said, I'm not advocating for that, just wanted to understand the implications, and you've explained those, so thanks! > > This solution allows to not list the specparts in the spec file itself. While > this is not a big benefit with the current feature set in the future this may > allow system wide policies to add stuff without the package needing to know - > as is already the case with the debuginfo packages (even if they are > implemented in a different way). Oh, right, this is a good reason alone to *not* use SPEC-hardcoded paths. Thanks! -- Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/1485#issuecomment-1297286784 You are receiving this because you are subscribed to this thread. Message ID: ___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Dynamic Spec generation (#1485)
@dmnks commented on this pull request. > + if (checkForEncoding(pkg->header, 0) != RPMRC_OK) { + badenc = 1; + } + } + if (badenc) + goto errxit; +} + +closeSpec(spec); + +exit: +return spec; + +errxit: +if (!secondary) + rpmSpecFree(spec); Wouldn't this cause a double-free in `parseSpec()`? We do the same freeing there in `errxit`. Moreover, it seems like freeing the passed structure should really be the responsibility of the caller...? > @@ -1129,3 +1157,26 @@ rpmSpec rpmSpecParse(const char *specFile, > rpmSpecFlags flags, { return parseSpec(specFile, flags, buildRoot, 0); } + +rpmRC parseGeneratedSpecs(rpmSpec spec) +{ +ARGV_t argv = NULL; +int argc = 0; +int i; + +char * specPattern = rpmGenPath("%{specpartsdir}", NULL, "*.specpart"); +if (rpmGlob(specPattern, , ) == 0) { + for (i = 0; i < argc; i++) { + rpmlog(RPMLOG_NOTICE, "Reading %s\n", argv[i]); + pushOFI(spec, argv[i]); + snprintf(spec->fileStack->readBuf, spec->fileStack->readBufLen, +"# Spec part read from %s\n\n", argv[i]); + if (parseSpecSection(spec, 1)) { This should've probably been `!parseSpecSection(spec, 1)` -- Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/1485#pullrequestreview-1162077690 You are receiving this because you are subscribed to this thread. Message ID: ___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Dynamic Spec generation (#1485)
There are a few reasons why this is not wired into `%include`. The first being that `%include` includes a file in place and then continues to read, expand and parse the rest of the file. This is something that just cannot be done afterwards. All kind of things could change by re-parsing the spec file. This is a mess we really don't want (and still have with the BuildArch parsing mess). This solution allows to not list the specparts in the spec file itself. While this is not a big benefit with the current feature set in the future this may allow system wide policies to add stuff without the package needing to know - as is already the case with the debuginfo packages (even if they are implemented in a different way). -- Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/1485#issuecomment-1297275256 You are receiving this because you are subscribed to this thread. Message ID: ___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Dynamic Spec generation (#1485)
... Point being, this is just another `%include`-like mechanism, one that happens at a very specific point during the build, yet it's completely separate from `%include`. OTOH, as I'm writing this, I realize that maybe that's a good thing, since the semantics of `%include` really is "include this text during spec parsing" whereas specparts is a *dynamic* mechanism. -- Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/1485#issuecomment-1297247510 You are receiving this because you are subscribed to this thread. Message ID: ___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Dynamic Spec generation (#1485)
I'm a bit late to the party, but couldn't we "just" add an `%include` alternative (e.g. as a parameter) such that it would be parsed post-`%install`? I'm pretty sure I'm missing some obvious reason why it couldn't be done (easily), but I meant to ask anyway :smile:. Of course, the current approach in PR is totally fine, too. -- Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/1485#issuecomment-1297235335 You are receiving this because you are subscribed to this thread. Message ID: ___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Dynamic Spec generation (#1485)
- Rebased - Added `specpartsdir` macro - Changed name of dir to `SPECPARTS` - Fixed white space issues - Use noarch package in test case - Merged patches Final patch might still deserve a bit longer commit message. Otoh there is docs on this and there is no need to duplicated that in the patch. -- Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/1485#issuecomment-1296951683 You are receiving this because you are subscribed to this thread. Message ID: ___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Dynamic Spec generation (#1485)
@ffesti pushed 1 commit. 06c764a2625fe2daea62598075bac2d10fc42317 Read in spec pieces generated during builds -- View it on GitHub: https://github.com/rpm-software-management/rpm/pull/1485/files/7e43a675d518fbe41fab8c456cb73d26a95c8af3..06c764a2625fe2daea62598075bac2d10fc42317 You are receiving this because you are subscribed to this thread. Message ID: ___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Dynamic Spec generation (#1485)
@ffesti pushed 3 commits. ae3a952f96aa84c7eed1cd10ee86d56017a8968b Split actual parsing of spec into its own function 28834422d628f5ad7381d18bdbb13e91dc3fc223 Allow starting new spec parts with PART_EMPTY 7e43a675d518fbe41fab8c456cb73d26a95c8af3 Read in spec pieces generated during builds -- View it on GitHub: https://github.com/rpm-software-management/rpm/pull/1485/files/d149c6a7dcb6005f892988fdc638bc1f9b5a21e5..7e43a675d518fbe41fab8c456cb73d26a95c8af3 You are receiving this because you are subscribed to this thread. Message ID: ___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Dynamic Spec generation (#1485)
Hmm, so the SPECPARTS directory (whether its empty or not at the end of %install) could be used for determining whether the feature was used during a build, so it could be recorded as an rpmlib() dependnecy on src.rpm files similar to DynamicBuildRequires. If we want to - we obviously do not track every new build feature that way. -- Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/1485#issuecomment-1291952787 You are receiving this because you are subscribed to this thread. Message ID: ___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Dynamic Spec generation (#1485)
> Well, the sub directory is mainly there so placing specparts on older > rpmbuild versions will fail and they won't just be silently ignored. Yup, I got that. > What about SPECPARTS as dir name. It's loud, but at least it's to the point. Add a %_specpartsdir macro, point it to that and we can move forward. We still got several months to change it if a better name comes to us in a flash of brilliance :laughing: -- Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/1485#issuecomment-1291906218 You are receiving this because you are subscribed to this thread. Message ID: ___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Dynamic Spec generation (#1485)
> > The patch now uses a SPECS sub dir in the buildsubdir > > Names are hard, but this directory is nothing at all like SPECS in > %{_topdir}, I don't think it should share that name. SPEC, maybe. Except, I > think we may want to use such a directory for other purposes too, which is > why the spec snippets have *.specpart suffix. Well, the sub directory is mainly there so placing specparts on older rpmbuild versions will fail and they won't just be silently ignored. What about `SPECPARTS` as dir name. -- Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/1485#issuecomment-1291897664 You are receiving this because you are subscribed to this thread. Message ID: ___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Dynamic Spec generation (#1485)
As for the directory name: add a macro for it, and document *that* instead of the hardcoded value. Otherwise, two weeks from the release of 4.19 we'll have a ticket requesting for one. Been there. Also that gives us at least a fleeting chance to rename it should such a thing be needed someday, no matter what we end up calling it here. Other misc observations: - the last three commits should be merged into one, it's a single feature being added there, the earlier ones are obviously just pre-requisite infra work - there's some excess/trailing whitespace in the "Read in spec pieces" commit - it needs a rebase due to #2243 -- Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/1485#issuecomment-1291896568 You are receiving this because you are subscribed to this thread. Message ID: ___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Dynamic Spec generation (#1485)
On the positive side, spec parse with and without this produce identical results on texlive.spec and kernel.spec (both fairly complicated beasts), except for the expected mkdir on the SPECS dir. -- Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/1485#issuecomment-1291881156 You are receiving this because you are subscribed to this thread. Message ID: ___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Dynamic Spec generation (#1485)
@pmatilai commented on this pull request. > +# Check if dynamic spec generation works +AT_SETUP([rpmbuild with dynamic spec generation]) +AT_KEYWORDS([build]) +RPMDB_INIT +AT_CHECK([ + +run rpmbuild --noclean\ + -ba "${abs_srcdir}"/data/SPECS/dynamic.spec +], +[0], +[ignore], +[ignore]) + +AT_CHECK([ + +runroot rpm -qp --qf "%{Summary}\n" /build/RPMS/*/hello-docs-1.0-1.*.rpm ...and it avoids globs like these as well. -- Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/1485#pullrequestreview-1156314815 You are receiving this because you are subscribed to this thread. Message ID: ___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Dynamic Spec generation (#1485)
@pmatilai commented on this pull request. > @@ -2272,3 +2272,27 @@ runroot rpmbuild \ ], [ignore]) AT_CLEANUP + +# -- +# Check if dynamic spec generation works +AT_SETUP([rpmbuild with dynamic spec generation]) +AT_KEYWORDS([build]) +RPMDB_INIT +AT_CHECK([ + +run rpmbuild --noclean\ + -ba "${abs_srcdir}"/data/SPECS/dynamic.spec Use a simpler, noarch package that you can run under runroot instead, what's being tested here doesn't require a compiler and make to be present at all. It makes things simpler when you're always operating inside the root and don't need these abs_srcdir kind of things at all. -- Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/1485#pullrequestreview-1156314094 You are receiving this because you are subscribed to this thread. Message ID: ___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Dynamic Spec generation (#1485)
> The patch now uses a SPECS sub dir in the buildsubdir Names are hard, but this directory is nothing at all like SPECS in %{_topdir}, I don't think it should use this name. SPEC, maybe. Except, I think we may want to use such a directory for other purposes too, which is why the spec snippets have *.specpart suffix. -- Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/1485#issuecomment-1291860636 You are receiving this because you are subscribed to this thread. Message ID: ___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Dynamic Spec generation (#1485)
OK, added test case and some docs. In addition I stripped the find-lang patch as this needs to g into it's own PR and was only there as a demonstration piece. The patch now uses a `SPECS` sub dir in the `buildsubdir` to allow detecting whether the feature is supported. I also removed the code to print out the content of the specparts to the terminal as we now have the expanded spec file in the SRPM. -- Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/1485#issuecomment-1289163826 You are receiving this because you are subscribed to this thread. Message ID: ___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Dynamic Spec generation (#1485)
@ffesti pushed 5 commits. 4320abf21b4d228b446fb679f0fa2e9f992e5985 Split actual parsing of the spec into its own function d8f830518e70e1bae31fab10c75720185446c5ba Allow starting new spec parts with PART_EMPTY 29708e32c8e2fb9f2b758d97fda24f23e1506ca6 Read in spec pieces generated during builds 201d8ce5a73c6f675a3b4310f9e14f57d2b0b58e Dynamic spec docs 68e83b3c83e8c127998b272e38d0f0eb5efa9a90 Add test case for dynamic spec generation -- View it on GitHub: https://github.com/rpm-software-management/rpm/pull/1485/files/ce3c6e97be69c6febcd3fd2f6424099eb8e7bdd2..68e83b3c83e8c127998b272e38d0f0eb5efa9a90 You are receiving this because you are subscribed to this thread. Message ID: ___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Dynamic Spec generation (#1485)
@pmatilai requested changes on this pull request. Okay so to recoup the on-off discussions over many moons: there are further details to sort out. While we can work on the details later, the bare minimum requirements for merging are: - documentation - a test-case or two For clarity, flagging as changes-needed for these two. -- Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/1485#pullrequestreview-1145177165 You are receiving this because you are subscribed to this thread. Message ID: ___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Dynamic Spec generation (#1485)
thanks @ffesti this groups `.mo` files by `language_id`, for example, in `__rpmbuild_gettext-runtime.lang.specpart` :+1: ```%package lang-en@quot Summary: en@quot translations for %{name} %description lang-en@quot Translations for package %{name} in en@quot %files lang-en@quot %lang(en@quot) /usr/share/locale/en@quot/LC_MESSAGES/gettext-runtime.mo ``` ```%package lang-pt Summary: pt translations for %{name} %description lang-pt Translations for package %{name} in pt %files lang-pt %lang(pt) /usr/share/locale/pt_BR/LC_MESSAGES/gettext-runtime.mo %lang(pt) /usr/share/locale/pt/LC_MESSAGES/gettext-runtime.mo ``` ```%package lang-zh Summary: zh translations for %{name} %description lang-zh Translations for package %{name} in zh %files lang-zh %lang(zh) /usr/share/locale/zh_CN/LC_MESSAGES/gettext-runtime.mo %lang(zh) /usr/share/locale/zh_HK/LC_MESSAGES/gettext-runtime.mo %lang(zh) /usr/share/locale/zh_TW/LC_MESSAGES/gettext-runtime.mo ``` -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/1485#issuecomment-789710406___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Dynamic Spec generation (#1485)
> I guess I really need to write a proper design document... Please do. I'm clearly thinking on somewhat different lines from you. Which is not to say either one is wrong, just that there are so many possible scenarios that they're easy to miss entirely, and/or don't easily fit inside ones head simultaneously :sweat_smile: -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/1485#issuecomment-784128757___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Dynamic Spec generation (#1485)
I guess I really need to write a proper design document... When I think about distribution level sub package creation I think about something very close to an brp-script - most likely a literal brp-script. Not that I am a big fan of the way brp scripts are currently run. But I'd rather improve the brp mechanism to something more modular and easier to control from the spec file than adding a new mechanism to run these "distro sub package creators". -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/1485#issuecomment-784125993___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Dynamic Spec generation (#1485)
I'd think there needs to be a global switch for at least case 2, but in addition individual scripts could/should be possible to disable case-by-case. Which might get tricky. Of course, just `rm -f` the bits you dislike/break at end of `%install` achieves that, but then you might want to prevent the script from running in the first place. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/1485#issuecomment-784116563___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Dynamic Spec generation (#1485)
When it comes to the distribution automagically generating sub packages I agree that the package need to be able to over write the behaviour. But I wonder if this really should be done on the "automatic sub packages off/on" level. Similar to what we have with debuginfo sub packages I'd rather expect each such feature providing some switch on its own. So packages would not switch off automatic sub package creation as a whole but the particular type that breaks. E.g. not splitting out precompliled (Python) files. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/1485#issuecomment-784110960___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Dynamic Spec generation (#1485)
Wrt implicit/explicitness of it all, it occurred to me that there are two quite distinct cases: 1. spec invoking a script that does something - such as invoke %find_lang with extra flags 2. upstream build system (makefiles etc) doing something For 1. you want it to "just work" without further ado (it's already explicit in the spec afterall), but for 2. that needs to be a packager decision. One crude but relatively simple way would be using different directories for these, with the directory for 1. being outside the package builddir so upstream make system has "no access" (because it doesn't know where it is), and 2. in a well-known location in the build directory, that would only be used if explicitly enabled in the spec somehow. That conceptual split seems resolve the implicit/explicit problem for me (actual implementation is another question). You'd still want to programmatically tell whether a spec relies on this for sub-package generation though. One could easily flag it in the SRPM one way or the other, but not the spec alone for 1. One possibility could be (going back to) a new spec section that simply executes after %install, but without any special parsing. Then you can easily tell at spec parse whether this is used or not, like with `%generate_requires`. Only something there doesn't feel right :thinking: -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/1485#issuecomment-784046850___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Dynamic Spec generation (#1485)
And yeah, docs + a test-case or two needed. Once that is there we can do a fine-comb review, but generally this is so remarkably tiny amount of code that I wouldn't expect any major issues there. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/1485#issuecomment-773216642___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Dynamic Spec generation (#1485)
@pmatilai commented on this pull request. > @@ -1118,3 +1146,25 @@ rpmSpec rpmSpecParse(const char *specFile, > rpmSpecFlags flags, { return parseSpec(specFile, flags, buildRoot, 0); } + +rpmRC parseGeneratedSpecs(rpmSpec spec) +{ + +ARGV_t argv = NULL; +int argc = 0; +int i; + +char * specPattern = rpmGenPath("%{u2p:%{_builddir}}", spec->buildSubdir, "__rpmbuild_*.specpart"); "__rpmbuild_*.specpart" still looks quite the eyesore to me. These are not rpm internal things, but files we expect others to produce. I'd think "*.specpart" would be enough to differentiate from other stuff. Also there's still the option of creating a special directory (".rpmbuild" or such in the builddir) which could be used for such things, including but not limited to debuginfo filelists, to avoid cluttering the main build directory (that gets requested separately every now and then). Such a thing doesn't have to be part of this PR, but should need to be done before a public release. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/1485#pullrequestreview-583255087___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Dynamic Spec generation (#1485)
It would be nice to see some simple example(s) :-) -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/1485#issuecomment-764533895___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Dynamic Spec generation (#1485)
@ffesti pushed 3 commits. b7b472b3e4ff284e260a581749c6fb30f24f38cc Parse spec snippets before running %check 3e4d486e415f603b2403eae7faa8a5325eedf39e Use __rpmbuild_*.specpart in BUILDDIR instead of __rpm sub dir ce3c6e97be69c6febcd3fd2f6424099eb8e7bdd2 Add support for --generate-subpackages to find_lang.sh -- You are receiving this because you are subscribed to this thread. View it on GitHub: https://github.com/rpm-software-management/rpm/pull/1485/files/89402a09e08d19c93cbd800e7ba53814f0c90aac..ce3c6e97be69c6febcd3fd2f6424099eb8e7bdd2 ___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Dynamic Spec generation (#1485)
> Is there a reason we can't just push it back into `rpmbuild` itself directly? > Like perhaps by exposing something via Lua that lets you programmatically > define binary packages? Yes, we could. I have thinking about this quite a lot. But this is much more complicated and harder to use. Generating a piece of spec file is easy and can be done from any programming language. This is a huge benefit if you want to do language specific solutions as you have easy access to all the domain specific tooling. But there are a few use cases that will benefit from direct access to the rpmbuild internals. Debuginfo being one of them. Otoh I dislike the fact that the internal state of rpmbuild getting altered without an artefact that documents that. Yes, this already is the case for debuginfo packages. But I would rather have less of that than more. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/1485#issuecomment-760837284___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Dynamic Spec generation (#1485)
> Do I understand it correctly that you'd essentially do a `my_spec_generator > > %_builddir/__rpm/subpac.spec` which would e.g. contain a new subpackage > definition? Basically yes. As this is run in the %build or %install section you would need to modify your spec generator to not output these. Note that this is not the only possible use. I just did a POC for find_lang.sh to generate language sub packages. This is surprisingly simple and can easily be done in bash. Long term this may also be used to generate the debuginfo sub packages. But as they are more closely coupled to the rpmbuild internals there are some more pieces needed to make this feasible. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/1485#issuecomment-760205838___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Dynamic Spec generation (#1485)
> * The generated spec fragments need to have some other suffix than *.spec to > avoid confusion. Call it .specpart or whatever, but they are not spec files > so they must not be mistaken as such, either by humans or the computer (eg > multiple .spec files in a tarball will confuse rpm) > * If the spec fragments are to be in some special new directory, we'll need > to have a macro for it. Up to now, various things have just dropped things > into the per-package build directory root, which would still be feasible if a > unique suffix is used. Some people dislike that practise as it is, but then > I'm not too hot about the `__rpm` directory either. Is there a reason we can't just push it back into `rpmbuild` itself directly? Like perhaps by exposing something via Lua that lets you programmatically define binary packages? -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/1485#issuecomment-760205293___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Dynamic Spec generation (#1485)
This looks intriguing! Do I understand it correctly that you'd essentially do a `my_spec_generator > %_builddir/__rpm/subpac.spec` which would e.g. contain a new subpackage definition? -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/1485#issuecomment-760200619___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Dynamic Spec generation (#1485)
cc: @dcermak -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/1485#issuecomment-760169523___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Dynamic Spec generation (#1485)
More random thoughts as they trickle through this old-but-not-obsolete (yet) processor of mine... :brain: - This is too implicit as it is. Existing carefully hand-crafted packages can't start behaving differently because somebody else somewhere did something (unless that something creates new files that it owns, like debuginfo does). Also, it needs to be clear from the outset whether a spec relies on dynamically generated parts or not, this will also serve as a clue to rpmspec that all is not what it seems. So given all the pre-existing content out there, I think this needs to be explicit opt-in behavior in the spec, one way or the other. If rpm was new today, it'd probably be the other way around. - The generated spec fragments need to have some other suffix than *.spec to avoid confusion. Call it .specpart or whatever, but they are not spec files so they must not be mistaken as such, either by humans or the computer (eg multiple .spec files in a tarball will confuse rpm) - If the spec fragments are to be in some special new directory, we'll need to have a macro for it. Up to now, various things have just dropped things into the per-package build directory root, which would still be feasible if a unique suffix is used. Some people dislike that practise as it is, but then I'm not too hot about the `__rpm` directory either. - For the generic case of including built content in src.rpm (such as the parsed spec), we already have a way, sort of: flag such files as RPMFILE_ARTIFACT in the src.rpm, and filter out on the regular src.rpm usage paths. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/1485#issuecomment-759993705___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Dynamic Spec generation (#1485)
A couple of random thoughts: - I think we'd want the generated specs parse occur before `%check` because you want to get possible packaging errors as soon as possible, and test-suites can take significant amount of time. And actually, I don't think we should allow `%check` to affect the actual packaging. - I think we need to come up with a (new) way to include generated content in src.rpm's. That content is not sources, patches or specs so it needs to be flagged differently, but it probably should be included in the name of debuggability and reproducability. The need for such a thing isn't specific to this PR at all, we've come across the topic in multiple cases in the last couple of years. As the spec macro usage gets ever more complex, it'd probably be a good idea to stash a fully expanded copy of the spec someplace for people to see, and optionally use for reproducing a build. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/1485#issuecomment-759322114___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Dynamic Spec generation (#1485)
Didn't look with a microscope yet (some whitespace and spelling issues there at least) but on the whole this looks really neat set of patches. Which is usually a good sign :+1: Do add some test-case(s) though, no matter how artificial. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/1485#issuecomment-758620899___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Dynamic Spec generation (#1485)
cc @davide125 -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/1485#issuecomment-758602337___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Dynamic Spec generation (#1485)
@ffesti pushed 1 commit. 89402a09e08d19c93cbd800e7ba53814f0c90aac Remove stupid warning -- You are receiving this because you are subscribed to this thread. View it on GitHub: https://github.com/rpm-software-management/rpm/pull/1485/files/4514e0436ea9897d0e2f9c802f6e40fb6b9a3c1f..89402a09e08d19c93cbd800e7ba53814f0c90aac ___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
[Rpm-maint] [rpm-software-management/rpm] Dynamic Spec generation (#1485)
This is an alternative to #1239 Instead of extending the spec syntax with dynamic elements allow the build process to generate their own spec file snippets that get read in after executing `%build` (technically after `%check`) from `$BUILDDIR/__rpm/*.spec`. This allows tricks like generating sub packages from brp scripts - although I might need to think this through a bit more. While this could be used to implement debuginfo packages as brp script that might not be a great idea as they dont have easy access to the rpmbuiild internals. But for simpler use cases (e.g. language sub packages) this should be perfectly fine. Another use case would be executing some X2rpm script that extracts the package structure from the manifest file of some scripting language module/project. You can view, comment on, or merge this pull request online at: https://github.com/rpm-software-management/rpm/pull/1485 -- Commit Summary -- * Split actual parsing of the spec into its own function * Allow starting new spec parts with PART_EMPTY * Read in spec pieces generated during builds -- File Changes -- M build/build.c (4) M build/parseSpec.c (152) M build/rpmbuild_internal.h (9) -- Patch Links -- https://github.com/rpm-software-management/rpm/pull/1485.patch https://github.com/rpm-software-management/rpm/pull/1485.diff -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/1485 ___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint