packaging/guidelines
6
2
Fork 11
Code Issues 111 Pull requests 27 Projects Releases Packages Wiki Activity Actions

Update advice on creating specfile macros #1454

Open
ferdnyc wants to merge 2 commits from rpm-macro-guidelines into main
pull from: rpm-macro-guidelines
merge into: packaging:main
Conversation 9 Commits 2 Files changed 1 +51 -18
ferdnyc commented 2025-04-12 02:38:25 +00:00
Member
Copy link

The guideline to use %global in preference to %define was
described by an RPM developer as "bad advice that spread far and
wide". This change completely reverses that advice, instructing
packagers to use %define by default and %global only when
needed.

This PR currently contains just the guidelines-text change. I'm hoping to also update many of the specfile templates and other bits of code that use %global (there are currently over 400 matches for %global in the packaging guidelines source), but that's in part pending an answer to my question over in #1449. I wanted to get this opened ASAP so everyone could decide how they feel about the wording, though.

Fixes #1449

The guideline to use `%global` in preference to `%define` was described by an RPM developer as "bad advice that spread far and wide". This change completely reverses that advice, instructing packagers to use `%define` by default and `%global` only when needed. This PR currently contains just the guidelines-text change. I'm hoping to also update many of the specfile templates and other bits of code that use `%global` (there are currently over 400 matches for `%global` in the packaging guidelines source), but that's in part pending an answer to [my question](https://pagure.io/packaging-committee/issue/1449#comment-965428) over in #1449. I wanted to get this opened ASAP so everyone could decide how they feel about the wording, though. Fixes #1449
ferdnyc commented 2025-04-12 02:44:13 +00:00
Author
Member
Copy link

i out a word, that should say "MAY be updated..."

i out a word, that should say "MAY <b>be</b> updated..."
ferdnyc commented 2025-04-12 02:45:49 +00:00
Author
Member
Copy link

1 new commit added

  • Add missing word
**1 new commit added** * ``Add missing word``
churchyard commented 2025-04-14 10:20:42 +00:00
Owner
Copy link

I am not a huge fan of introducing the term "variables" into this. They are macros, not variables.

I am not a huge fan of introducing the term "variables" into this. They are macros, not variables.
decathorpe commented 2025-04-14 10:51:39 +00:00
Owner
Copy link

Just making sure I understand this correctly,

  • %globals are expanded once at definition time,
  • %defines are expanded on every use?

Assuming this is correct, this advice might be too strong:

Packagers SHOULD use %define to create macros in spec files.
Legacy uses of %global MAY be updated to use %define.
Spec files MAY contain uses of %global
if immediate expansion is needed (a rare occurrence).

There will be instances where the time of evaluation will make a difference (for example, macros could be pulled in as BuildRequires, but not in Requires, leading them to be undefined at runtime), or macros that shell out and query the environment - and I don't think these are really that rare.

And assuming that I understand this correctly, it might be better to say something like

  • use %define when it doesn't matter when the macros is expanded (or how often), i.e. usually for simple text replacements
  • use %global when it does matter when / how often expansion happens , if the expansion involves some sort of "expensive computation", or if the expansion relies on things that are only available inside the build environment

TL;DR: I don't think we can just flip the "use %global instead of %define" rule around and don't expect things to break in funny ways.

Just making sure I understand this correctly, - `%global`s are expanded *once* at definition time, - `%define`s are expanded *on every use*? Assuming this is correct, this advice might be too strong: > Packagers **SHOULD** use `%define` to create macros in spec files. > Legacy uses of `%global` **MAY** be updated to use `%define`. > Spec files **MAY** contain uses of `%global` > if immediate expansion is needed (a rare occurrence). There will be instances where the time of evaluation will make a difference (for example, macros could be pulled in as BuildRequires, but not in Requires, leading them to be undefined at runtime), or macros that shell out and query the environment - and I don't think these are really *that* rare. And assuming that I understand this correctly, it might be better to say something like - use `%define` when it doesn't matter *when* the macros is expanded (or how often), i.e. usually for simple text replacements - use `%global` when it *does* matter when / how often expansion happens , if the expansion involves some sort of "expensive computation", or if the expansion relies on things that are only available inside the build environment TL;DR: I don't think we can just flip the "use %global instead of %define" rule around and don't expect things to break in funny ways.
ferdnyc commented 2025-04-21 01:48:55 +00:00
Author
Member
Copy link

@decathorpe

Just making sure I understand this correctly,

  • %globals are expanded once at definition time,
  • %defines are expanded on every use?

Broadly, yes, but I'm not sure how things like shell invocations play into that — just because %define macros are lazily expanded, doesn't necessarily mean that a %() subshell invocation will be re-executed at each use. (Doesn't mean that it won't, either, which is why I put that question to @pmatilai in #1449 -- but he hasn't responded yet, so I'll open an issue in the RPM upstream repo to clear that point up.)

The RPM macro docs do mention that %global is in part "useful to avoid re-expansion of expensive macros", but doesn't elaborate on what exactly that means at all.

But beyond that, situations like this:

for example, macros could be pulled in as BuildRequires, but not in Requires, leading them to be undefined at runtime),

...are where %define is most preferable to %global, because a %define macro will resolve even if some of its component macros are defined after it... with %global, the moment the statement is processed you've lost your chance to define/redefine any of the macros used in that statement.

With %define, macros are expanded at time of use, yes... but those uses still occur at build-time. No %define'd macros should survive to runtime unless they're explicitly escaped via %% invocations.

IOW, if I use a %define macro in a %preun script, the "time of use" is when that %preun code is processed into a script by rpmbuild as it's processing the specfile. What gets written into the RPM metadata is a static script with all of the macros expanded. There's no runtime dependency on the macros.

Ditto if I use a macro in a Version: or Provides: or etc. line in the specfile... the value that's stored is the expansion of that macro, regardless whether it's a %global macro or a %define macro. The only difference is that the %define macro is expanded when the Version: or Provides: line is processed, whereas the %global macro was expanded way back when it was created.

@decathorpe > Just making sure I understand this correctly, > > - `%global`s are expanded *once* at definition time, > - `%define`s are expanded *on every use*? Broadly, yes, but I'm not sure how things like shell invocations play into that — just because `%define` macros are lazily expanded, _doesn't_ necessarily mean that a `%()` subshell invocation will be re-executed at each use. (Doesn't mean that it **won't**, either, which is why I put that question to @pmatilai in #1449 -- but he hasn't responded yet, so I'll open an issue in the RPM upstream repo to clear that point up.) The [RPM macro docs](https://rpm-software-management.github.io/rpm/manual/macros.html#global-macros) do mention that `%global` is in part "useful to avoid re-expansion of expensive macros", but doesn't elaborate on what exactly that means at all. But beyond that, situations like this: > for example, macros could be pulled in as BuildRequires, but not in Requires, leading them to be undefined at runtime), ...are where `%define` is _most_ preferable to `%global`, because a `%define` macro will resolve even if some of its component macros are defined _after_ it... with `%global`, the moment the statement is processed you've lost your chance to define/redefine any of the macros used in that statement. With `%define`, macros are expanded at time of _use_, yes... but those uses still occur at build-time. No `%define`'d macros should survive to runtime unless they're explicitly escaped via `%%` invocations. IOW, if I use a `%define` macro in a `%preun` script, the "time of use" is when that `%preun` code is processed into a script by rpmbuild as it's processing the specfile. What gets written into the RPM metadata is a static script with all of the macros expanded. There's no runtime dependency on the macros. Ditto if I use a macro in a `Version:` or `Provides:` or etc. line in the specfile... the value that's stored is the expansion of that macro, regardless whether it's a `%global` macro or a `%define` macro. The only difference is that the `%define` macro is expanded when the `Version:` or `Provides:` line is processed, whereas the `%global` macro was expanded way back when it was created.
ferdnyc commented 2025-04-21 02:32:15 +00:00
Author
Member
Copy link

Question posted to the RPM repo's Q&A area: https://github.com/rpm-software-management/rpm/discussions/3727

Question posted to the RPM repo's Q&A area: https://github.com/rpm-software-management/rpm/discussions/3727
pmatilai commented 2025-04-22 07:57:15 +00:00
Member
Copy link

Apologies for being slow to respond to this stuff.

I'd recommend dropping the SHOULD/MAY language entirely because recommending one over the other doesn't make sense. It's like a recommendation between a spoon and a shovel: they do kinda similar things, but sometimes you need one and sometimes the other, and there's no universal recommendation you could make.

I think we should just help packagers make informed choices. Explaining the actual difference like here is good, practical examples of when to use each should help more.
For a practical rule of thumb, something like "Use %global for unchanging global values, otherwise use %define."

Apologies for being slow to respond to this stuff. I'd recommend dropping the SHOULD/MAY language entirely because recommending one over the other doesn't make sense. It's like a recommendation between a spoon and a shovel: they do kinda similar things, but sometimes you need one and sometimes the other, and there's no universal recommendation you could make. I think we should just help packagers make informed choices. Explaining the actual difference like here is good, practical examples of when to use each should help more. For a practical rule of thumb, something like "Use %global for unchanging global values, otherwise use %define."
ferdnyc commented 2025-04-23 12:07:57 +00:00
Author
Member
Copy link

@pmatilai

I'd recommend dropping the SHOULD/MAY language entirely because recommending one over the other doesn't make sense.

Well, they're guidelines, not RPM documentation — being prescriptive is sort of the entire point. I agree that including an explanation of the two is useful, but the guidelines are there specifically to provide rules for packagers to follow.

The rule can be (and will probably have to be) some form of, "packagers SHOULD use whatever makes the most sense for a given case" (heck, that could even be a MUST rule), but there's still gonna be a rule.

(And speaking of RPM documentation, I think this stuff could be covered in more detail in the actual RPM documentation — which, as I mentioned, doesn't really ever come out and say that shell invocations and other operations will be re-executed on every expansion of a %define macro — but that's a conversation for the PR against https://github.com/rpm-software-management/rpm/blob/master/docs/manual/macros.md that I expect I'll be talked into submitting 😉 )

For a practical rule of thumb, something like "Use %global for unchanging global values, otherwise use %define."

See, that's a pretty big departure from where this all started, because most of our macros ARE unchanging global values. Now, it's likely that's more out of necessity than anything else, since we scared people off of %define for so long that %global is all they know. As a result, they often tend to treat all macros as if they can only be unchanging global values.

(Which sort of explains, but doesn't, the unnecessary nonsense %global ... %{expand: ... } that started all this. The only explanation I can give for that is that we've so brainwashed our packagers into %global being the only way to define a macro, they no longer remember — or never knew — what it means relative to %define.)

So I definitely agree that helping packagers better understand the difference is useful.

I also agree that practical examples demonstrating situations that would favor either %global or %define (like the ones I used in my Q&A post, as a starting point) would also be useful... but in the back of my mind it feels wrong if we end up having better examples than the real docs, so that's probably (also) another topic for that macros.md PR. 😀

@pmatilai > I'd recommend dropping the SHOULD/MAY language entirely because recommending one over the other doesn't make sense. Well, they're guidelines, not RPM documentation — being prescriptive is sort of the entire point. I agree that including an explanation of the two is useful, but the guidelines are there specifically to provide rules for packagers to follow. The rule can be (and will probably have to be) some form of, "packagers SHOULD use whatever makes the most sense for a given case" (heck, that could even be a MUST rule), but there's still gonna be a rule. (And speaking of RPM documentation, I think this stuff **could** be covered in more detail in the _actual_ RPM documentation — which, as I mentioned, doesn't really ever come out and say that shell invocations and other operations will be re-executed on every expansion of a `%define` macro — but that's a conversation for the PR against https://github.com/rpm-software-management/rpm/blob/master/docs/manual/macros.md that I expect I'll be talked into submitting :wink: ) > For a practical rule of thumb, something like "Use %global for unchanging global values, otherwise use %define." See, that's a pretty big departure from where this all started, because _most_ of our macros ARE unchanging global values. Now, it's likely that's more out of necessity than anything else, since we scared people off of `%define` for so long that `%global` is all they know. As a result, they often tend to treat all macros as if they can only be unchanging global values. (Which sort of explains, but doesn't, the unnecessary nonsense `%global ... %{expand: ... }` that started all this. The only explanation I can give for that is that we've so brainwashed our packagers into `%global` being the _only_ way to define a macro, they no longer remember — or never knew — what it means relative to `%define`.) So I definitely agree that helping packagers better understand the difference is useful. I also agree that practical examples demonstrating situations that would favor either `%global` or `%define` (like the ones I used in my Q&A post, as a starting point) would also be useful... but in the back of my mind it feels wrong if we end up having **better** examples than the real docs, so that's probably (also) another topic for that `macros.md` PR. :grinning:
james commented 2025-04-24 16:41:48 +00:00
Owner
Copy link

We talked about this again, in the meeting this week, and one idea that came up was to try to move the real part of the documentation to upstream/rpm and then the Fedora docs could just say something like "use define/global appropriately as needed, see this link to the upstream rpm docs on how they work"

We talked about this again, in the meeting this week, and one idea that came up was to try to move the real part of the documentation to upstream/rpm and then the Fedora docs could just say something like "use define/global appropriately as needed, see this link to the upstream rpm docs on how they work"
This pull request is broken due to missing fork information.
View command line instructions

Checkout

From your project repository, check out a new branch and test the changes.
git fetch -u origin rpm-macro-guidelines:rpm-macro-guidelines
git switch rpm-macro-guidelines

Merge

Merge the changes and update on Forgejo.

Warning: The "Autodetect manual merge" setting is not enabled for this repository, you will have to mark this pull request as manually merged afterwards.

git switch main
git merge --no-ff rpm-macro-guidelines
git switch rpm-macro-guidelines
git rebase main
git switch main
git merge --ff-only rpm-macro-guidelines
git switch rpm-macro-guidelines
git rebase main
git switch main
git merge --no-ff rpm-macro-guidelines
git switch main
git merge --squash rpm-macro-guidelines
git switch main
git merge --ff-only rpm-macro-guidelines
git switch main
git merge rpm-macro-guidelines
git push origin main
Sign in to join this conversation.
Reviewers
No reviewers
Labels
Clear labels   
announce
Approved and written up change to be announced

  
bootstrap
Bootstrapping exception

  
Closed As
accepted
 Closed with the reason of accepted

  
Closed As
duplicate
 Closed with the reason of duplicate

  
Closed As
exceptionexpired
 Closed with the reason of exceptionexpired

  
Closed As
fixed
 Closed with the reason of fixed

  
Closed As
invalid
 Closed with the reason of invalid

  
Closed As
nothingtodo
 Closed with the reason of nothingtodo

  
Closed As
permanentexception
 Closed with the reason of permanentexception

  
Closed As
rejected
 Closed with the reason of rejected

  
Closed As
temporaryexception
 Closed with the reason of temporaryexception

  
Closed As
wontfix
 Closed with the reason of wontfix

  
cmake

   
committee
Waiting on work by a committee member

  
draftneeded
Someone needs to write a draft before this can proceed

  
hasdraft
A draft has been provided

  
meeting
Needs meeting discussion

  
meson

   
needinfo
Needs additional info from the submitter

  
Priority
In Committee
 Priority of In Committee

  
Priority
Needs Review
 Priority of Needs Review

  
Priority
Waiting For Reporter
 Priority of Waiting For Reporter

  
rust

   
writeup
Approved change to be written into the guidelines

No labels
announce
bootstrap
Closed As
accepted
Closed As
duplicate
Closed As
exceptionexpired
Closed As
fixed
Closed As
invalid
Closed As
nothingtodo
Closed As
permanentexception
Closed As
rejected
Closed As
temporaryexception
Closed As
wontfix
cmake
committee
draftneeded
hasdraft
meeting
meson
needinfo
Priority
In Committee
Priority
Needs Review
Priority
Waiting For Reporter
rust
writeup
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
5 participants
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
packaging/guidelines!1454
No description provided.