-
Notifications
You must be signed in to change notification settings - Fork 62
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Skip only if all macros in ExclusiveArch and ExcludeArch evaluated #3513
base: main
Are you sure you want to change the base?
Conversation
…aluated Fix fedora-copr#3244 It can happen that our builder doesn't understand some architecture macro, e.g. `%{zig_arches}`. In such a case when it is used in `ExclusiveArch`, we shouldn't skip all chroots like we do now. A better approach is to be safe and avoid any skipping. Builds will fail in some chroots that should have been skipped but that can be workarounded by the user by manually submitting the build only for a subset of chroots.
"This chroot was skipped because of ExclusiveArch" | ||
finish(chroot, StatusEnum("skipped")) | ||
# Skip only if all macros in ExclusiveArch evaluated | ||
if not [x for x in exclusivearch if x.startswith("%")]: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Macros may look like %foo a b %c
, so probably we should just check for '%' in x
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And, ... could we give user a hint that something weird is happening? in the sources' builder-live.log/backend.log?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Macros may look like %foo a b %c, so probably we should just check for '%' in x
True, but if I understand correctly, this shouldn't matter. For example, in our beaker tests we use this specfile
https://src.fedoraproject.org/rpms/biosdevname/blob/rawhide/f/biosdevname.spec
It has ExclusiveArch: %{ix86} x86_64
... If it was reversed and it was ExclusiveArch: x86_64 %{ix86}
, that would be the case you are talking about?
It shouldn't be a problem because in copr-rpmbuild
, we produce a list out of them:
Output: ['exclusivearch-test-1-1.src.rpm']
Running SRPMResults tool
Package info: {
"name": "exclusivearch-test",
"epoch": null,
"version": "1",
"release": "1",
"exclusivearch": [
"noarch",
"x86_64",
"aarch64"
],
"excludearch": []
}
SRPMResults finished
So here, in this frontend code, our exclusivearch
variable is ["noarch", "x86_64", "aarch64"]
. If there was an unevaluated macro, it would be ["noarch", "x86_64", "aarch64", "%{zig_arches}"]
. So IMHO startswith
should work as expected.
That being said, I have no issue with '%' in x
and we can use it if you prefer.
And, ... coul give user a hint that something weird is happening? in the sources' builder-live.log/backend.log?
That would be great but ... probably not? This is a frontend code, so no user-logs. And we cannot display an UI banner here either. The only indication is that at the end of the SRPM builder-live.log, there will be unevaluated macro in the exclusivearch
list.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You are correct. Awesome analysis. Can we have a test for this case?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a frontend code
What would happen if we produced an empty list on copr-rpmuibuild then? Properly warning user about unexpanded macro..
Fix #3244
It can happen that our builder doesn't understand some architecture macro, e.g.
%{zig_arches}
. In such a case when it is used inExclusiveArch
, we shouldn't skip all chroots like we do now. A better approach is to be safe and avoid any skipping. Builds will fail in some chroots that should have been skipped but that can be workarounded by the user by manually submitting the build only for a subset of chroots.