Re: [Rpm-maint] [rpm-software-management/rpm] Add %bcond macro for defining build conditionals (#1520)
I got back to this; all issues except the nested `%{expand:..}` should be
addressed.
--
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/1520#issuecomment-912633112___
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 %bcond macro for defining build conditionals (#1520)
@encukou commented on this pull request.
> @@ -76,47 +76,25 @@
%defined() %{expand:%%{?%{1}:1}%%{!?%{1}:0}}
%undefined() %{expand:%%{?%{1}:0}%%{!?%{1}:1}}
-# Shorthand for %{defined with_...}
+# Handle conditional builds.
+# (see 'conditionalbuilds' in the manual)
+#
+# Internally, the `--with foo` option defines the macro `_with_foo` and the
+# `--without foo` option defines the macro `_without_foo`.
+# Based on those and a default (used when neither is given), bcond macros
+# define the macro `with_foo`, which should later be checked:
+
+%bcond() %{expand:%[ (%2)
+? "%{expand:%%{!?_without_%{1}:%%global with_%{1} 1}}"
+: "%{expand:%%{?_with_%{1}:%%global with_%{1} 1}}"
+]}
+%bcond_with() %{expand:%%{?_with_%{1}:%%global with_%{1} 1}}
+%bcond_without() %{expand:%%{!?_without_%{1}:%%global with_%{1} 1}}
I haven't found a way to avoid it :(
--
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/1520#discussion_r701984570___
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 %bcond macro for defining build conditionals (#1520)
@hroncok I gave you access to my fork in case you want to take this in the next few weeks. I probably won't have free cycles soon. -- 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/1520#issuecomment-802267361___ 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 %bcond macro for defining build conditionals (#1520)
Sorry, this has gotten a bit forgotten in middle of other action. The idea is nice, certainly. Just some minor nitpicks, plus rebase and squash the "doc nit" commit. -- 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/1520#issuecomment-801149285___ 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 %bcond macro for defining build conditionals (#1520)
@pmatilai commented on this pull request. > @@ -24,10 +53,12 @@ If you want to change whether or not an option is enabled > by default, only change `%bcond_with` to `%bcond_without` or vice versa. In such a case, the remainder of the spec file can be left unchanged. + Seems like stray whitespace? -- 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/1520#pullrequestreview-614398748___ 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 %bcond macro for defining build conditionals (#1520)
@pmatilai commented on this pull request. > @@ -990,3 +990,25 @@ xxx error: bad ]) AT_CLEANUP + +AT_SETUP([bcond macro]) +AT_KEYWORDS([macros]) +AT_CHECK([ Move this test into the other one in rpmbuild.at - the macro tests are more about testing the macro language and its builtins. There can be multiple AT_CHECK sections inside AT_SETUP-AT_CLEANUP pair, this will fit nicely in the build-side test. -- 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/1520#pullrequestreview-614397369___ 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 %bcond macro for defining build conditionals (#1520)
@pmatilai commented on this pull request.
> @@ -76,47 +76,25 @@
%defined() %{expand:%%{?%{1}:1}%%{!?%{1}:0}}
%undefined() %{expand:%%{?%{1}:0}%%{!?%{1}:1}}
-# Shorthand for %{defined with_...}
+# Handle conditional builds.
+# (see 'conditionalbuilds' in the manual)
+#
+# Internally, the `--with foo` option defines the macro `_with_foo` and the
+# `--without foo` option defines the macro `_without_foo`.
+# Based on those and a default (used when neither is given), bcond macros
+# define the macro `with_foo`, which should later be checked:
+
+%bcond() %{expand:%[ (%2)
+? "%{expand:%%{!?_without_%{1}:%%global with_%{1} 1}}"
+: "%{expand:%%{?_with_%{1}:%%global with_%{1} 1}}"
+]}
+%bcond_with() %{expand:%%{?_with_%{1}:%%global with_%{1} 1}}
+%bcond_without() %{expand:%%{!?_without_%{1}:%%global with_%{1} 1}}
I think the new %bcond macro should be built on top of the pre-existing
%bcond_with/without macros, or the other way around. This is effectively
implementing the inner logic twice. The other thing is, seeing nested
%{expand:..} is a bit of a red flag: they quickly escalate into %%%-escape
madness in specs. Didn't try it, but to me it seems the outer %{expand:..}
should not be needed. The one that creates the %global definition is.
--
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/1520#pullrequestreview-614390730___
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 %bcond macro for defining build conditionals (#1520)
I'd *really* like to see this happen. How can I move it forward? -- 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/1520#issuecomment-799870515___ 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 %bcond macro for defining build conditionals (#1520)
@DemiMarie commented on this pull request.
> @@ -76,47 +76,25 @@
%defined() %{expand:%%{?%{1}:1}%%{!?%{1}:0}}
%undefined() %{expand:%%{?%{1}:0}%%{!?%{1}:1}}
-# Shorthand for %{defined with_...}
+# Handle conditional builds.
+# (see 'conditionalbuilds' in the manual)
+#
+# Internally, the `--with foo` option defines the macro `_with_foo` and the
+# `--without foo` option defines the macro `_with_foo`.
You’re welcome!
--
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/1520#discussion_r577950438___
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 %bcond macro for defining build conditionals (#1520)
@encukou commented on this pull request.
> @@ -76,47 +76,25 @@
%defined() %{expand:%%{?%{1}:1}%%{!?%{1}:0}}
%undefined() %{expand:%%{?%{1}:0}%%{!?%{1}:1}}
-# Shorthand for %{defined with_...}
+# Handle conditional builds.
+# (see 'conditionalbuilds' in the manual)
+#
+# Internally, the `--with foo` option defines the macro `_with_foo` and the
+# `--without foo` option defines the macro `_with_foo`.
Thank you!
--
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/1520#discussion_r577920394___
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 %bcond macro for defining build conditionals (#1520)
@encukou pushed 1 commit. eef816619d9bd225e9768aa81990fa6b36e387e8 Doc nit -- You are receiving this because you are subscribed to this thread. View it on GitHub: https://github.com/rpm-software-management/rpm/pull/1520/files/3510e485cd647bdf211097365531af8511d90ddf..eef816619d9bd225e9768aa81990fa6b36e387e8 ___ 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 %bcond macro for defining build conditionals (#1520)
@DemiMarie commented on this pull request.
Doc nit
> @@ -76,47 +76,25 @@
%defined() %{expand:%%{?%{1}:1}%%{!?%{1}:0}}
%undefined() %{expand:%%{?%{1}:0}%%{!?%{1}:1}}
-# Shorthand for %{defined with_...}
+# Handle conditional builds.
+# (see 'conditionalbuilds' in the manual)
+#
+# Internally, the `--with foo` option defines the macro `_with_foo` and the
+# `--without foo` option defines the macro `_with_foo`.
```suggestion
# `--without foo` option defines the macro `_without_foo`.
```
--
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/1520#pullrequestreview-592577079___
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 %bcond macro for defining build conditionals (#1520)
> So, I'd like to remove that section entirely. Feel free. I doubt anybody has paid this close attention to the docs in decades... -- 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/1520#issuecomment-769829470___ 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 %bcond macro for defining build conditionals (#1520)
All info from the comments in the macro file is in the docs, except that:
> When checking conditions: never use `without_foo`, `_with_foo` nor
> `_without_foo`, only `with_foo`.
contradics with the [Pass it to
%configure](https://github.com/rpm-software-management/rpm/blob/master/doc/manual/conditionalbuilds.md#pass-it-to-configure)
section, which uses `_with_*`.
Inded, this will not work correctly when used in combination with `%bcond` and
`%bcond_without`, because it doesn't honor the "default".
So, I'd like to remove that section entirely. The section above already
explains what to do – some variation of:
%{?with_gnutls:--with static} \
%{!?with_openssl:--without openssl}
... which is more verbose, but works with %bcond` & `%bcond_without`.
--
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/1520#issuecomment-769827340___
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 %bcond macro for defining build conditionals (#1520)
@encukou pushed 1 commit. e0d581b9b7c47b1817700cc35ea99a21963a97df Add newly added file to Makefile.am -- You are receiving this because you are subscribed to this thread. View it on GitHub: https://github.com/rpm-software-management/rpm/pull/1520/files/9218cbdeef7a9839d7a7b1033d5325c3e0444d34..e0d581b9b7c47b1817700cc35ea99a21963a97df ___ 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 %bcond macro for defining build conditionals (#1520)
@encukou pushed 1 commit. 9218cbdeef7a9839d7a7b1033d5325c3e0444d34 Fix typos in docs -- You are receiving this because you are subscribed to this thread. View it on GitHub: https://github.com/rpm-software-management/rpm/pull/1520/files/831de9ed06a9e33957074105693b04226d636b21..9218cbdeef7a9839d7a7b1033d5325c3e0444d34 ___ 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 %bcond macro for defining build conditionals (#1520)
@encukou commented on this pull request.
> ## Check whether an option is enabled or disabled
-To define `BuildRequires` depending on the command-line switch, you can use the
-`%{with foo}` macro:
+To make parts of the spec file conditional depending on the command-line
+switch, you can use the `%{with foo}` macro or its counterpart,
+`%{wthout foo}`:
```suggestion
`%{without foo}`:
```
--
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/1520#discussion_r566823894___
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 %bcond macro for defining build conditionals (#1520)
OK, I'll fold the comments into the Markdown docs. -- 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/1520#issuecomment-769807126___ 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 %bcond macro for defining build conditionals (#1520)
Oh BTW, https://github.com/rpm-software-management/rpm/blob/master/doc/manual/conditionalbuilds.md would be a better place for bulk of the documentation, the fact that there's verbose documentation with examples on pre-existing %bcond in macros file is only a historical artifact from not having a better place for it. -- 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/1520#issuecomment-769799661___ 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 %bcond macro for defining build conditionals (#1520)
@lubomir commented on this pull request.
> ## Check whether an option is enabled or disabled
-To define `BuildRequires` depending on the command-line switch, you can use the
-`%{with foo}` macro:
+To make parts of the spec file conditional depending on the command-line
+switch, you can use the `%{with foo}` macro or its counterpart,
+`%{wthout foo}`:
Typo: wthout → without
--
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/1520#pullrequestreview-579214502___
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 %bcond macro for defining build conditionals (#1520)
In general, I like this feature, and once it lands in RPM, I'll probably port this to debbuild. It makes a ton more sense than the existing stuff. -- 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/1520#issuecomment-769777511___ 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 %bcond macro for defining build conditionals (#1520)
@hroncok commented on this pull request. > -To use this feature in a spec file, add this to the beginning of the file: +To enable a build conditional in a spec file, use the `%bcond` macro at the +beginning of the file, specifying the name of the conditional and its default +value: + +``` +# To build with "gnutls" by default: +%bcond gnutls 1 +# To build without "gnutls" default: ```suggestion # To build without "gnutls" by default: ``` -- 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/1520#pullrequestreview-579210256___ 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 %bcond macro for defining build conditionals (#1520)
The added tests are failing probably because the added spec is missing from tests/Makefile.am. -- 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/1520#issuecomment-769774130___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
[Rpm-maint] [rpm-software-management/rpm] Add %bcond macro for defining build conditionals (#1520)
The names `%bcond_with` and `%bcond_without` are based the inner workings of
the macros. They describe the inverse of their default; anecdotal evidence
sugests that this is quite confusing in practice. I personally always have to
stop and think when using/reviewing them.
This PR adds the macro `%bcond`, which does the same thing as either
`%bcond_with` or `%bcond_without`, based on a numeric default value:
```
# To build with gnutls by default (equivalent to %bcond_without
gnutls):
%bcond gnutls 1
# To build without gnutls default (equivalent to %bcond_with
gnutls):
%bcond gnutls 0
```
It allows more complex defaults, simplifying a common if-elif pattern (see
https://github.com/rpm-software-management/rpm/issues/941):
```
# By default, build with openssl iff not building with gnutls (but allow
building with none or both)
%bcond openssl %{without gnutls}
# Enable some new feature that only works on Fedora 30+ with the gnutls crypto
%bcond new_feature %[ 0%fedora 30 %{with gnutls} ]
```
Documentation is changed to mention `%bcond` first – the only disadvantage I
can see vs. the existing macros is that old RPM versions wont have it.
---
This is my first contribution to RPM. Please tell me all the things Im
doing wrong!
(e.g. should this be discussed somewhere first?)
You can view, comment on, or merge this pull request online at:
https://github.com/rpm-software-management/rpm/pull/1520
-- Commit Summary --
* Add %bcond macro for defining build conditionals
-- File Changes --
M doc/manual/conditionalbuilds.md (39)
M macros.in (53)
A tests/data/SPECS/bcondtest.spec (33)
M tests/rpmbuild.at (62)
M tests/rpmmacro.at (21)
-- Patch Links --
https://github.com/rpm-software-management/rpm/pull/1520.patch
https://github.com/rpm-software-management/rpm/pull/1520.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/1520
___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint
