Re: [Rpm-maint] [rpm-software-management/rpm] Dynamic Spec generation (#1485)

2022-11-09 Thread Panu Matilainen
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)

2022-11-09 Thread Panu Matilainen
@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)

2022-11-09 Thread Florian Festi
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)

2022-11-09 Thread Florian Festi
@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)

2022-11-09 Thread Michal Domonkos
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)

2022-11-09 Thread Michal Domonkos
> 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)

2022-11-09 Thread Florian Festi
@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)

2022-11-08 Thread Panu Matilainen
@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)

2022-11-08 Thread Panu Matilainen
@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)

2022-11-08 Thread Florian Festi
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)

2022-11-08 Thread Florian Festi
@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)

2022-11-08 Thread Florian Festi
@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)

2022-11-08 Thread Florian Festi
@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)

2022-11-08 Thread Florian Festi
- 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)

2022-11-08 Thread Florian Festi
@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)

2022-11-08 Thread Florian Festi
@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)

2022-11-08 Thread Florian Festi
@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)

2022-11-08 Thread Panu Matilainen
@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)

2022-11-07 Thread Panu Matilainen
@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)

2022-11-07 Thread Panu Matilainen
@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)

2022-11-07 Thread Panu Matilainen
@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)

2022-11-07 Thread Florian Festi
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)

2022-11-07 Thread Florian Festi
@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)

2022-11-07 Thread Florian Festi
@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)

2022-11-07 Thread Florian Festi
@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)

2022-11-07 Thread Florian Festi
@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)

2022-11-07 Thread Florian Festi
@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)

2022-11-07 Thread Florian Festi
@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)

2022-11-07 Thread Florian Festi
@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)

2022-11-07 Thread Florian Festi
@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)

2022-11-07 Thread Panu Matilainen
@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)

2022-11-07 Thread Michal Domonkos
@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)

2022-11-07 Thread Panu Matilainen
@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)

2022-11-07 Thread Panu Matilainen
@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)

2022-11-07 Thread Panu Matilainen
@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)

2022-11-07 Thread Panu Matilainen
@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)

2022-11-06 Thread Panu Matilainen
@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)

2022-11-06 Thread Florian Festi
@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)

2022-11-06 Thread Panu Matilainen
> 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)

2022-11-04 Thread Michal Domonkos
@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)

2022-11-04 Thread Florian Festi
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)

2022-11-04 Thread Florian Festi
@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)

2022-11-03 Thread Panu Matilainen
@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)

2022-11-03 Thread Panu Matilainen
...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)

2022-11-03 Thread Panu Matilainen
> 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)

2022-11-03 Thread Panu Matilainen
@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)

2022-11-03 Thread Panu Matilainen
@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)

2022-11-03 Thread Panu Matilainen
@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)

2022-10-31 Thread Michal Domonkos
@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)

2022-10-31 Thread Michal Domonkos
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)

2022-10-31 Thread Michal Domonkos
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)

2022-10-31 Thread Michal Domonkos
> 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)

2022-10-31 Thread Michal Domonkos
@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)

2022-10-31 Thread Florian Festi
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)

2022-10-31 Thread Michal Domonkos
... 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)

2022-10-31 Thread Michal Domonkos
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)

2022-10-31 Thread Florian Festi
- 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)

2022-10-31 Thread Florian Festi
@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)

2022-10-31 Thread Florian Festi
@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)

2022-10-26 Thread Panu Matilainen
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)

2022-10-26 Thread Panu Matilainen
> 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)

2022-10-26 Thread Florian Festi
> > 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)

2022-10-26 Thread Panu Matilainen
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)

2022-10-26 Thread Panu Matilainen
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)

2022-10-26 Thread Panu Matilainen
@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)

2022-10-26 Thread Panu Matilainen
@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)

2022-10-26 Thread Panu Matilainen
> 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)

2022-10-24 Thread Florian Festi
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)

2022-10-24 Thread Florian Festi
@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)

2022-10-18 Thread Panu Matilainen
@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)

2021-03-03 Thread Sundeep Anand
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)

2021-02-23 Thread Panu Matilainen
> 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)

2021-02-23 Thread Florian Festi
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)

2021-02-23 Thread Panu Matilainen
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)

2021-02-23 Thread Florian Festi
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)

2021-02-23 Thread Panu Matilainen
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)

2021-02-04 Thread Panu Matilainen
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)

2021-02-04 Thread Panu Matilainen
@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)

2021-01-21 Thread Jens Petersen
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)

2021-01-19 Thread Florian Festi
@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)

2021-01-15 Thread Florian Festi
> 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)

2021-01-14 Thread Florian Festi
> 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)

2021-01-14 Thread ニール・ゴンパ
> * 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)

2021-01-14 Thread Dan Čermák
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)

2021-01-14 Thread ニール・ゴンパ
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)

2021-01-14 Thread Panu Matilainen
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)

2021-01-13 Thread Panu Matilainen
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)

2021-01-12 Thread Panu Matilainen
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)

2021-01-12 Thread Igor Raits
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)

2021-01-12 Thread Florian Festi
@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)

2021-01-12 Thread Florian Festi
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