Re: [Rpm-maint] [rpm-software-management/rpm] Make %{buildsubdir} usable without being a side effect of %setup. (#860)
nwnk approved this pull request.
> @@ -329,8 +333,8 @@ static int doSetupMacro(rpmSpec spec, const char *line)
/* if necessary, create and cd into the proper dir */
if (createDir) {
- buf = rpmExpand("%{__mkdir_p} ", spec->buildSubdir, "\n",
- "cd '", spec->buildSubdir, "'", NULL);
+ buf = rpmExpand("%{?_buildsubdir:%{__mkdir_p} '%{_buildsubdir}'\n}",
+ "cd '%{_buildsubdir}'}", NULL);
The CI build caught this:
```
/srv/rpm/rpm-4.15.90/_build/sub/tests/testing/tmp/rpm-tmp.5Xq9OQ: line 31: cd:
test-1.0}: No such file or directory
```
You have an extra `}` in the `cd` command here.
--
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/860#pullrequestreview-292520184___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Make %{buildsubdir} usable without being a side effect of %setup. (#860)
@vathpela pushed 1 commit.
43c0982a97a5c600f00b8108ef1765bc15ee563f Make %{buildsubdir} usable without
being a side effect of %setup.
--
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/rpm-software-management/rpm/pull/860/files/57b69b312d551a33fcbae47c965893da1ec5a687..43c0982a97a5c600f00b8108ef1765bc15ee563f
___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Make %{buildsubdir} usable without being a side effect of %setup. (#860)
> Other than the typo this looks good to me. > > `Reviewed-by: Adam Jackson ` Yeah, that's what I get for doing make check and then going "oh that looks dangerous, but it's an easy fix" while re-reading the patch before pushing. Anyway, new version that passes pushed. -- 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/860#issuecomment-534763882___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
[Rpm-maint] [rpm-software-management/rpm] Support uncompressed/reconstructed payloads (#861)
#163 / commit 91aa078 added `RPMTAG_PAYLOADDIGEST` and `RPMTAG_PAYLOADDIGESTALGO`, so RPM now verifies the integrity of the payload. But there are tools (e.g. `deltarpm`) that reconstruct RPM payloads from individual parts. Given an RPM header and the individual file contents, the original (uncompressed) payload can be easily reconstructed by adding the appropriate CPIO headers, but there's no way to verify the integrity of the reconstructed payload other than re-compressing it and letting RPM verify PAYLOADDIGEST, which wastes a bunch of CPU & disk i/o and then sometimes fails randomly because of minor, unpredictable differences in compressor output. To fix this I propose adding a second digest (`RPMTAG_PAYLOADDIGEST_UNCOMPRESSED`?) for the _uncompressed_ payload, and then either: 1. Fall back to uncompressing the payload and checking the uncompressed digest if the original verification fails (unsafe, slow) 2. Add another tag (maybe `SIGTAG_PAYLOAD_UNCOMPRESSED`?) which directs RPM to assume the payload is already uncompressed; external programs could manually set that flag when reconstructing an RPM, or 3. Add a new tag (`RPMTAG_PAYLOAD_MAGIC`?) that gives magic bytes (e.g. the first 4 bytes) for the compressed and uncompressed payload, so RPM can identify uncompressed/reconstructed payloads. Either way, RPM would also need to override/ignore `RPMTAG_PAYLOADCOMPRESSOR` when the "uncompressed payload" flag is set. But that only happens in 3 places that I can see, so that's doable. -- 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/issues/861___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Make %{buildsubdir} usable without being a side effect of %setup. (#860)
Other than the typo this looks good to me. `Reviewed-by: Adam Jackson ` -- 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/860#issuecomment-534619869___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Make %setup use %{__tar_opts} to set tar options. (#859)
`Reviewed-by: Adam Jackson ` -- 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/859#issuecomment-534613072___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
[Rpm-maint] [rpm-software-management/rpm] Make %{buildsubdir} usable without being a side effect of %setup. (#860)
This patch makes a couple of related changes:
- lets you set %{_buildsubdir} as a global to expose it everywhere,
rather than just specific parts of %prep (%setup and %patch*)
- lets you choose what path is used independently of the unpack
options in %setup
- allows you to use a different %{_buildsubdir} in different parts of
the .spec, for instance if you have multiple builds of the same code
with different compile options
- renames it to %{_buildsubdir} since its now exposed
Signed-off-by: Peter Jones pjo...@redhat.com
You can view, comment on, or merge this pull request online at:
https://github.com/rpm-software-management/rpm/pull/860
-- Commit Summary --
* Make %{buildsubdir} usable without being a side effect of %setup.
-- File Changes --
M build/build.c (13)
M build/files.c (8)
M build/pack.c (2)
M build/parsePrep.c (24)
M build/policies.c (2)
M build/rpmbuild_internal.h (1)
M build/spec.c (2)
M macros.in (4)
M tests/data/macros.debug (2)
-- Patch Links --
https://github.com/rpm-software-management/rpm/pull/860.patch
https://github.com/rpm-software-management/rpm/pull/860.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/860
___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint
[Rpm-maint] [rpm-software-management/rpm] Make %setup use %{__tar_opts} to set tar options. (#859)
This patch adds __tar_opts and __tar_opts_verbose macros, which can be
overriden to change the default tar behavior when called from %setup
while building packages.
Signed-off-by: Peter Jones pjo...@redhat.com
You can view, comment on, or merge this pull request online at:
https://github.com/rpm-software-management/rpm/pull/859
-- Commit Summary --
* Make %setup use %{__tar_opts} to set tar options.
-- File Changes --
M build/parsePrep.c (4)
M macros.in (2)
-- Patch Links --
https://github.com/rpm-software-management/rpm/pull/859.patch
https://github.com/rpm-software-management/rpm/pull/859.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/859
___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint
[Rpm-maint] [rpm-software-management/rpm] Make copyNextLineFromOFI() aware of the new %[] syntax (#858)
Thats the third loop of that form in the code... You can view, comment on, or merge this pull request online at: https://github.com/rpm-software-management/rpm/pull/858 -- Commit Summary -- * Make copyNextLineFromOFI() aware of the new %[] syntax -- File Changes -- M build/parseSpec.c (7) -- Patch Links -- https://github.com/rpm-software-management/rpm/pull/858.patch https://github.com/rpm-software-management/rpm/pull/858.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/858 ___ 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 a %cnl (continue next line) marker (#787)
Here's another solution: ``` %global godocs docs examples code-of-conduct.md %dnl\ README.md ``` This uses the new %dnl macro to eat away the newline generated by the trailing `\`. -- 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/issues/787#issuecomment-534565402___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Logical op improvements (#855)
Okay then, thanks for the patches! -- 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/855#issuecomment-534517798___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Logical op improvements (#855)
Merged #855 into master. -- 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/855#event-2657706957___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Ternary op discussions (#852)
Closed #852 via #855. -- 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/issues/852#event-2657706960___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Ternary op discussions (#852)
Kinda funny, I'm not familiar with that form at all. Not that it means much, except that maybe I'm not the best judge here :) I can certainly live without ```?``` and ```?:```, for the latter I know gcc supports it but I don't recall ever using it. So it's not exactly of life-support importance... -- 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/issues/852#issuecomment-534517331___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Logical op improvements (#855)
(I've got no plans for another commit currently.) -- 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/855#issuecomment-534516042___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Codify built-in macro argument acceptance (#853)
Merged #853 into master. -- 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/853#event-2657687248___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Correct description of %verbose and %getconfdir in the macro manual (#856)
Merged #856 into master. -- 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/856#event-2657674654___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Correct description of %verbose and %getconfdir in the macro manual (#856)
pmatilai approved this pull request. Fine now, thanks. -- 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/856#pullrequestreview-292348903___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Ternary op discussions (#852)
With the referenced pull request you can use && and || for alternate/default: `%foo && %bar` is `%bar` if `%foo` is true, otherwise `0` `%foo || %bar` is `%foo` if `%foo` is true, otherwise `%bar` I think people are somewhat more used to this than the degenerate `?` and `?!` forms that aren't used in any other programming language. -- 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/issues/852#issuecomment-534512964___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Codify built-in macro argument acceptance (#853)
The PR looks good to me. -- 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/853#issuecomment-534511744___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Correct description of %verbose and %getconfdir in the macro manual (#856)
Pushed. -- 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/856#issuecomment-534511549___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Correct description of %verbose and %getconfdir in the macro manual (#856)
@pavlinamv pushed 1 commit. 9ae7eb4858f381cad3925c96a0ec1b4d7d9f36cc Correct description of %verbose and %getconfdir in the macro manual -- You are receiving this because you are subscribed to this thread. View it on GitHub: https://github.com/rpm-software-management/rpm/pull/856/files/857558f100343a971af19c720dea71749f53456b..9ae7eb4858f381cad3925c96a0ec1b4d7d9f36cc ___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Correct description of %verbose and %getconfdir in the macro manual (#856)
Forgot to push? -- 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/856#issuecomment-534509060___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Correct description of %verbose and %getconfdir in the macro manual (#856)
Changed according to the comment. -- 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/856#issuecomment-534508114___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
[Rpm-maint] [rpm-software-management/rpm] Remove "support" for loading keyring from filesystem (#857)
This is basically an abandoned and forgotten development path from 11 years ago that arguably shouldve been removed long ago, and one that has potential security implications and doesnt play well with existing API users who rely on gpg-pubkey headers being in the rpmdb (RhBug:1393586) You can view, comment on, or merge this pull request online at: https://github.com/rpm-software-management/rpm/pull/857 -- Commit Summary -- * Remove support for loading keyring from filesystem -- File Changes -- M lib/rpmts.c (56) -- Patch Links -- https://github.com/rpm-software-management/rpm/pull/857.patch https://github.com/rpm-software-management/rpm/pull/857.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/857 ___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Correct description of %verbose and %getconfdir in the macro manual (#856)
Good spotting, but please move them to the appropriate groups as well: the
first group of macros takes no argument, so %getconfdir belongs there, and
%{verbose:...} belongs to the group that does.
The grouping is pretty arbitrary of course, %define/%undefine/%global are in
the middle of different types of macros for no good reason.
--
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/856#issuecomment-534500534___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint
[Rpm-maint] [rpm-software-management/rpm] Correct description of %verbose and %getconfdir in the macro manual (#856)
I looked into rpm/doc/manual/macros to check 0 and 1 added into the builtinmacros[] in PR #853. Values added in PR #853 are correct, but description of macros %verbose and %getconfdir in manual is confusing. So that is why I created this PR. You can view, comment on, or merge this pull request online at: https://github.com/rpm-software-management/rpm/pull/856 -- Commit Summary -- * Correct description of %verbose and %getconfdir in the macro manual -- File Changes -- M doc/manual/macros (5) -- Patch Links -- https://github.com/rpm-software-management/rpm/pull/856.patch https://github.com/rpm-software-management/rpm/pull/856.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/856 ___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Logical op improvements (#855)
Makes the code nicer while enhancing it, what's not to like. One thing I forgot to mention in some of the earlier patches is that I certainly don't see any need to add those empty comments above functions as the common style in that function is. It's not a showstopper by any means but I'd basically rather see them all eliminated than more added, as we've done elsewhere (and ditto with the ```@param state expression parser state``` kind of wholly redundant "API docs", its pretty damn obvious what it is...) I'm fine with merging this as-is though, unless you planning on pushing more commits here? -- 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/855#issuecomment-534489577___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Ternary op discussions (#852)
As for 3), I think both variants would be kinda nice to have. In particular, people are used to ```%foo ? %bar``` from macros, and might be even a bit surprised if that doesn't work. And default value case of ```%foo ?: %bar``` is quite a common pattern, so why not? -- 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/issues/852#issuecomment-534486010___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Codify built-in macro argument acceptance (#853)
After poking around a bit, I ended up dropping the argument length checking entirely: it's not an *error* to echo an empty string, and similarly it is not an *error* to ask for dirname of an empty string. And so on - seems better left alone in reality. On-disk filenames cannot be empty so those cases could raise an error, but leaving that classification effort to some other day. -- 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/853#issuecomment-534467660___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
[Rpm-maint] [rpm-software-management/rpm] Logical op improvements (#855)
This commits change the handling of the logical operators. It allows to use strings as condition and also changes || and to return the last evaluated term like in perl/python/ruby. Fixes #852 You can view, comment on, or merge this pull request online at: https://github.com/rpm-software-management/rpm/pull/855 -- Commit Summary -- * Add boolifyValue helper and use it in rpmExprBool * Disallow ternary operator with different types on the rhs * Use boolifyValue in the not operator * Change || and operators to behave like perl/python/ruby -- File Changes -- M rpmio/expression.c (72) M tests/rpmmacro.at (34) -- Patch Links -- https://github.com/rpm-software-management/rpm/pull/855.patch https://github.com/rpm-software-management/rpm/pull/855.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/855 ___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Codify built-in macro argument acceptance (#853)
Yup, the thing with g vs gn is that g reflects whether ':' is there or not, and
gn reflects whether there's something after it: macros that do not accept
arguments must not have g, and macros that do need to have both (but non-zero
gn implies non-NULL g).
And yeah, ```%{echo:%{nil}}``` would work and that's perfectly fine. Which is
exactly the reason that makes me wonder if empty argument should be permitted
for echo, warn and error.
Good point on doFoo() expansion, but note that doFoo() is not the only function
of that kind. Perhaps the central check should be only for g (ie ':') and only
check for the non-empty argument in the places that actually care. Which
probably would leave doOutput() naturally out of the argument checking - making
output of empty string does feel kinda wrong.
I'll change that at least to see how it'd look like. Thanks for reviewing!
--
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/853#issuecomment-534410176___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint
