Re: [Rpm-maint] [rpm-software-management/rpm] Docs: Some formatting fixes for macros.md (PR #2251)
@ferdnyc commented on this pull request. > -All whitespace surrounding \ is removed. Name may be composed -of alphanumeric characters, and the character `_' and must be at least -3 characters in length. A macro without an (opts) field is "simple" in that -only recursive macro expansion is performed. A parameterized macro contains -an (opts) field. "-" as opts disables all option processing, otherwise -the opts (i.e. string between parentheses) are passed +All whitespace surrounding `` is removed. Name may be composed +of alphanumeric characters and the character `_`, and must be at least +3 characters in length. A macro without an `(opts)` field is "simple" +in that only recursive macro expansion is performed. A parameterized +macro contains an `(opts)` field. `-` as opts disables all option +processing, otherwise the opts (i.e. string between parentheses) are passed I snuck in a grammar change here that really should've been a separate commit, moving the comma in this sentence: > Name may be composed of alphanumeric characters, and the character `_` and > must be at least 3 characters in length. to separate the two clauses: > Name may be composed of alphanumeric characters and the character `_`, and > must be at least 3 characters in length. -- Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/2251#pullrequestreview-1162690898 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] Docs: Some formatting fixes for macros.md (PR #2251)
@ferdnyc commented on this pull request. > +…where `` is a legal macro name and `` is the body of the macro. +Multiline macros can be defined by shell-like line continuation, ie ``\`` I'm not sure if this will work, it depends on the renderer. Currently the website is showing \`\\\` un-fenced, implying that it's interpreting the backslash as an escape on the second backtick. This is **unlike** GitHub Flavored Markdown, which properly shows a literal `\`. My change may or may not fix it. The other possibility is that the source should be `` `\\` ``, but in GFM that becomes `\\`. -- Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/2251#pullrequestreview-1162688672 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
[Rpm-maint] [rpm-software-management/rpm] Docs: Some formatting fixes for macros.md (PR #2251)
This started life with the realization that the RPM website Markdown formatter was combining double dashes into an en dash, in the docs. (See, for example, near the end of the second paragraph in the section [Defining a Macro](https://rpm-software-management.github.io/rpm/manual/macros.html)): “–” can be used to separate options from arguments. It kind of grew from there. The primary commit is the one to add backtick fencing to more literals in the body text, in an attempt to combat the issue mentioned above. It _should_ be successful, leading to a clearer statement: `--` can be used to separate options from arguments. The others are just cleanup, and I can drop any on request. (I can drop the fencing one, too, of course.) Im not sure the last commit will work, it depends on the renderer. Currently the website is showing \`\\\` un-fenced, implying that its interpreting the backslash as an escape on the second backtick. This is **unlike** GitHub Flavored Markdown, which properly shows a literal `\`. My change may or may not fix it. The other possibility is that the source should be `` `\\` ``, but in GFM that becomes `\\`. You can view, comment on, or merge this pull request online at: https://github.com/rpm-software-management/rpm/pull/2251 -- Commit Summary -- * Docs: macros: Replace all ... with … * Docs: macros: backtick-fence more literals in body * Docs: Macros: Attempt to fix literal backslash -- File Changes -- M docs/manual/macros.md (187) -- Patch Links -- https://github.com/rpm-software-management/rpm/pull/2251.patch https://github.com/rpm-software-management/rpm/pull/2251.diff -- Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/2251 You are receiving this because you are subscribed to this thread. Message ID: rpm-software-management/rpm/pull/2...@github.com ___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
[Rpm-maint] [rpm-software-management/rpm] /build dir naming, and CMake conventions (Issue #2250)
This is _relatively_ minor, but: There's a long-standing convention -- laziness, really -- in the CMake world, of doing out-of-source builds in a directory named `build`, often located (for convenience) in the top level of the repo dir. `rpmbuild` living in `/build` screws with that convention. It's far from insurmountable (I just built everything in a directory named `_build`, for my current test run), but still it's bound to be confusing, and dumping autotools (#2232) gives us an opportunity to cleanly un-screw with it. Let me just say that I'm willing to do the work of addressing this. I know CMake well and am confident I can talk it into whatever we decide. I just don't want to put a lot of work into a PR that won't be merged, so I thought it best to first discuss an approach. (_If any_, since not addressing it is certainly an option.) Non-solutions * Simply rename `build` to `rpmbuild`. (Conflicts with output binary name, in the build dir.) Possible solutions In no particular order, since I jotted them down as they came to me. 1. Create a `src/` dir, and move the top-level `.c` files, `build`, `fileattrs`, `misc`, `rpmio`, and `sign` into that dir. 1. Rename `build` to `rpmbuild`, **and** have it output the `rpmbuild` binary target _inside_ that directory, rather than at the top level of the builddir. (That's also somewhat more in keeping with CMake conventions, though I think we'd then want to have the rest of the binaries do the same, for consistency. Which makes it a bit of work in terms of both build tooling and subsequent packaging/testing.) 1. Rename `build` to `rpmbuild`, and have all the executable targets (or, all targets?) output into a `bin/` directory inside the build dir. (Same caveats as previous.) 1. Move `build` to `lib/rpmbuild`, since it's **really** the source to `librpmbuild`. (Probably also `rpmio` => `lib/rpmio`, and `sign` => `lib/rpmsign`, again for consistency.) 1. Rename `build` to `librpmbuild`, `rpmio` to `librpmio`, `sign` to `librpmsign`, and -- say it with me, "for consistency" -- `lib` to `librpm`. 1. ??? -- Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/issues/2250 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] Add rpm-sort utility for sorting RPM versions (PR #2249)
Thanks for the review; updated. > It's not a particularly common use-case AFAICS. Well, depends on how you define "common", I suppose - it gets called every time a new kernel is installed, which is pretty common :) > Still, not convinced this needs to be in rpm. I'm not seeing a better place for it - are you? It doesn't belong in grub or kernel-install because adding a build dependency on RPM doesn't make sense upstream. It doesn't belong in rpmdevtools because it needs to be installed on every system and that isn't. Pretty much all it does is provide a shell interface to a function from librpm. Looking at other systems for precendent, a tool like this generally isn't needed because `sort -V` is good enough - e.g., on Debian. Even so, the comparison functionality at least is exposed by the core tool (`dpkg --compare-versions`). -- Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/2249#issuecomment-1297417160 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] Add rpm-sort utility for sorting RPM versions (PR #2249)
@frozencemetery pushed 1 commit. b04fc4d2775470b659801252bd6ca87ccf36f03c Add rpmsort utility for sorting RPM versions -- View it on GitHub: https://github.com/rpm-software-management/rpm/pull/2249/files/b2e6455060f8d4566eed5188e26f4d58c8183427..b04fc4d2775470b659801252bd6ca87ccf36f03c 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] Turn %prep into a normal build script (Issue #2205)
> For some stats, out of 22000+ packages in Fedora, about 6300 have Patch > declarations, of which roughly 3500 are still using the `%patch1` style > syntax. and a whopping 25 are using other variants, including defaulting to 0 > and using a combo of -P and positional arguments etc. The rest are presumably > using `%autosetup` (~8700 packages) Hey, wait... those numbers don't add up. If there are 6300 PatchN-packages, and 3525 use %patchN or %patch N, then at most 2775 are using `%autosetup`. Or is ~8700 the _total_ number of `%autosetup` packages, including those with no PatchN declarations? (If so, that's also depressing: After all this time, only 39.54% of packages are using `%autosetup`? I know packages using conditional patching can't use it, but at **most** that's the other 2775 with Patch declarations.) -- Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/issues/2205#issuecomment-1297337186 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] cmake minimum version requirement for rpm? (Discussion #2248)
Debian isn't exactly a prime target for rpm :smile: but for the record, it seems to be at 3.18.4 from 2020, which is probably newer than rpm can reasonably require. So that should be easily covered. In the meanwhile I realized rpm is already relying on at least >= 3.12 despite what our CMakeLists.txt (for deitys sake who came up with that filename...) claims. 3.12 is from 2018 and that may be more like it. -- Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/discussions/2248#discussioncomment-4017595 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] Add rpm-sort utility for sorting RPM versions (PR #2249)
Yeah there *are* use-cases, such as figuring out the order of stuff in a directory. Still, not convinced this needs to be in rpm. But assuming for a moment we were to merge this, there are some issues to sort (pun not intended) out: - this belongs to tools/ rather than the top-level directory - rpm tools do not have dash in the name, eg it should be called 'rpmsort' - use of alloca() is prohibited - it should use rstrndup() rather than manual malloc+strncpy, possibly other cases where rpm has string helpers - rpmvercmp() on package name seems dubious, the name does not take part in version comparison and rpmvercmp() is known to return 0 for very obviously different strings -- Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/2249#issuecomment-1296800054 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] Add rpm-sort utility for sorting RPM versions (PR #2249)
> I can see how grub* needs such a thing, but I don't really see why this > should be in rpm itself. It's not a particularly common use-case AFAICS. I had to hand-roll something similar when figuring out which was the most recent VM kernel package I had installed in my dom0. -- Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/2249#issuecomment-1296733767 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] Add rpm-sort utility for sorting RPM versions (PR #2249)
I can see how grub* needs such a thing, but I don't really see why this should be in rpm itself. It's not a particularly common use-case AFAICS. -- Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/2249#issuecomment-1296722814 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
