fedora-copr / copr

RPM build system - upstream for https://copr.fedorainfracloud.org/
113 stars 62 forks source link

Excluding i686 architectures doesn't work #3061

Closed FrostyX closed 1 week ago

FrostyX commented 11 months ago

This package:

Package: firefox
Version: 120.0.1-1
Source Type: Build from DistGit
Committish: 91f5d4fe1b3fbf627eff5630934106653668bb4a
Clone_Url: https://src.fedoraproject.org/rpms/firefox.git

Is not being skipped for fedora-rawhide-i386 chroot and instead it tries to build, and fails with

error: Architecture is excluded: i686

Reported by @fberat.

praiskup commented 10 months ago

Shouldn't the package use ExcludeAarch: %ix86 instead?

praiskup commented 8 months ago

@fberat do you think %ix86 is usable?

praiskup commented 8 months ago

Or @FrostyX ?

FrostyX commented 8 months ago

I think having ExcludeAarch: %ix86 in the package, should fix the issue.

fberat commented 8 months ago

@fberat do you think %ix86 is usable?

Assuming firefox is the only package with this problem yes, but wouldn't there any way to cover that in copr ?

praiskup commented 8 months ago

Copr doesn't know the target claims itself as i686, best bet is i386 from its name. At the same time, when the decision "are we going to build it for this chroot?" is done, we do not have the chroot in hand (we would have to initialize all the potential chroots candidates).

I don't see an automatic solution here. We would have to implement some chroot => arch mapping. This isn't easy either, because what "chroot claims" may not really be consistent (a) in time, things tend to change over time and (b) in our builder set - one builder, on hardware A, will claim ppc64le, and the other, on hardware B, will claim powerpc64le, yet both are used for fedora-rawhide-ppc64le.

That said, we could implement some fedora-rawhide-i386 => ['i386', 'i686', 'i586', ...] mapping, and use that to decide. I am not sure it is worth it because at the end of the day, %ix86 exists exactly for this purpose.

penguinpee commented 6 months ago

I think having ExcludeAarch: %ix86 in the package, should fix the issue.

It doesn't. At least not for me. Here's what I posted earlier in #3244:

I believe the reverse also holds. For a package that has ExcludeArch: %{ix86} in a Copr with x86_64 and i386 chroots, Copr attempts the build and fails, weheras previously the result would be skipped.

In the build log I see error: Architecture is excluded: i686 and the last entry is:

ERROR: Command failed: 
 # ['bash', '--login', '-c', '/usr/bin/rpmbuild -br  --target i686 --nodeps /builddir/build/originals/python-pandas.spec']

Copr build error: Build failed

I've seen skipped results before for excluded arches. Albeit, that was when a package excluded s390x and ppc64le. But I think the behavior for excluding i686 should be the same.

penguinpee commented 6 months ago

I actually need this to work properly with a result of skipped when the arch is on the exclude list. I need to test a bunch of packages with regards to dropping i686. Skipped means no further investigation is needed, whereas failed would require investigation.

Let me know if I can help or provide any more information on the matter.

praiskup commented 6 months ago

Can you please post the affected build ID?

penguinpee commented 6 months ago

Here is an example where it fails: https://copr.fedorainfracloud.org/coprs/gui1ty/pandas_no_i686/build/7420555/

And another one where it worked correctly, albeit for different excluded arches using ExclusiveArch: %{qt5_qtwebengine_arches}: https://copr.fedorainfracloud.org/coprs/gui1ty/Spyder6/build/7379609/

praiskup commented 6 months ago

Thank you for the update @penguinpee , this seems like another issue. Seems like you define ExcludeAarch in sub-package.

penguinpee commented 6 months ago

Ah, good catch! I'll move it to the main package and try again. It wasn't meant to be in a sub package.

penguinpee commented 6 months ago

Yes, that was it. It shows as skipped now. Sorry about the distraction.

nikromen commented 1 week ago

This is possible to fix inside the package, it seems like users don't need this so we'll probably never fix this due to team capacity issues.