-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
env2mfile: Add unit test coverage for dpkg -> Meson, and fix some obvious errors #13722
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
smcv
commented
Sep 27, 2024
smcv
force-pushed
the
env2mfile-deb-kernel
branch
from
October 2, 2024 13:48
f03705b
to
ceab9db
Compare
smcv
changed the title
env2mfile: Don't hard-code Debian as always being Linux
env2mfile: Add unit test coverage for dpkg -> Meson, and fix some obvious errors
Oct 2, 2024
smcv
force-pushed
the
env2mfile-deb-kernel
branch
from
October 2, 2024 13:53
ceab9db
to
cdbe731
Compare
smcv
force-pushed
the
env2mfile-deb-kernel
branch
from
October 2, 2024 13:55
cdbe731
to
2601cff
Compare
smcv
commented
Oct 2, 2024
smcv
force-pushed
the
env2mfile-deb-kernel
branch
2 times, most recently
from
October 2, 2024 14:10
f9f6b2b
to
fc87950
Compare
This was referenced Oct 2, 2024
dcbaker
requested changes
Oct 2, 2024
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.
Thanks for the very thorough testing of this, I've got a few comments below.
This will make it easier to unit-test functions that work with a MachineInfo, by constructing the expected object in a single call. Signed-off-by: Simon McVittie <[email protected]>
Separating the part that runs dpkg-architecture from the part that interprets its results will make it easier to unit-test the latter. Signed-off-by: Simon McVittie <[email protected]>
smcv
force-pushed
the
env2mfile-deb-kernel
branch
from
October 2, 2024 17:58
fc87950
to
7dd17f3
Compare
This test parses several possible outputs of dpkg-architecture and asserts that they produce the expected MachineInfo. To avoid depending on suitable cross-tools being installed, use unittest.mock to override locate_path with a version that pretends that all of the tools we're interested in are in /usr/bin. Similarly, use mock environment variables to exercise what happens when we have those set. The test data used here exercises most variations: * big- and little-endianness * GNU CPU (x86_64) differing from dpkg CPU (amd64) * Linux, kFreeBSD and Hurd * special-cased architectures: x86, arm, mips64el, ppc64el expected_compilers() intentionally doesn't assume that every compiler is gcc (even though they all are, right now), because mesonbuild#13721 proposes adding valac which does not take a gcc suffix. Signed-off-by: Simon McVittie <[email protected]>
…ases This makes the frequent pattern of things like "CPU families are the same as GNU CPUs, with a few known exceptions" less verbose. Signed-off-by: Simon McVittie <[email protected]>
As per <https://mesonbuild.com/Reference-tables.html>, and matching what happens when running Meson for a native build on Debian GNU/Hurd. Signed-off-by: Simon McVittie <[email protected]>
All official Debian release architectures use the Linux kernel, but unofficial ports like hurd-i386 and kfreebsd-amd64 use the Hurd and FreeBSD kernel, respectively. Map Linux to 'linux' and kFreeBSD ports to 'freebsd' as per the reference tables in Meson's documentation. For now, use the Debian system name such as 'hurd' for anything else (see mesonbuild#13740 for the question of whether Hurd should identify its kernel differently). Signed-off-by: Simon McVittie <[email protected]>
`DEB_HOST_ARCH` encodes both the CPU family and the OS, so using it to get the CPU type gives the wrong answer for non-Linux ports. However, `DEB_HOST_GNU_CPU` gives less detailed information about the CPU: it's `arm` for all 32-bit ARM CPUs, and doesn't distinguish between the differing baselines of `armel` (ARMv5 softfloat) and `armhf` (ARMv7 hardfloat). When cross-compiling for x86_64 Linux, this changes the `cpu()` from `amd64` to `x86_64`, which is consistent with the answer we get during native builds on that architecture. When cross-compiling for `ppc64el`, this changes the `cpu()` from `ppc64el` to `ppc64`, which is a reasonable change but is still not consistent with what we see during native builds (which is `ppc64le`): see mesonbuild#13741 for that. Resolves: mesonbuild#13742 Signed-off-by: Simon McVittie <[email protected]>
smcv
force-pushed
the
env2mfile-deb-kernel
branch
from
October 2, 2024 18:00
7dd17f3
to
3614f58
Compare
dcbaker
approved these changes
Oct 2, 2024
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.
Thanks, this looks good to me.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
env2mfile: Convert MachineInfo into a dataclass
This will make it easier to unit-test functions that work with a
MachineInfo, by constructing the expected object in a single call.
env2mfile: Split detect_cross_debianlike()
Separating the part that runs dpkg-architecture from the part that
interprets its results will make it easier to unit-test the latter.
unittests: Test env2mfile's dpkg_architecture_to_machine_info
This test parses several possible outputs of dpkg-architecture and
asserts that they produce the expected MachineInfo.
To avoid depending on suitable cross-tools being installed, use
unittest.mock to override locate_path with a version that pretends that
all of the tools we're interested in are in /usr/bin.
Similarly, use mock environment variables to exercise what happens
when we have those set.
The test data used here exercises most variations:
expected_compilers() intentionally doesn't assume that every compiler
is gcc (even though they all are, right now), because env2mfile: Generalize handling of architecture-prefixed tools on Debian derivatives #13721 proposes
adding valac which does not take a gcc suffix.
env2mfile: Add a function for mostly-identity mappings with special cases
This makes the frequent pattern of things like "CPU families are the
same as GNU CPUs, with a few known exceptions" less verbose.
env2mfile: Map Debian GNU/Hurd to system() -> gnu
As per https://mesonbuild.com/Reference-tables.html, and matching what
happens when running Meson for a native build on Debian GNU/Hurd.
env2mfile: Don't hard-code Debian as always being Linux
All official Debian release architectures use the Linux kernel, but
unofficial ports like hurd-i386 and kfreebsd-amd64 use the Hurd and
FreeBSD kernel, respectively.
Map Linux to 'linux' and kFreeBSD ports to 'freebsd' as per the
reference tables in Meson's documentation. For now, use the Debian
system name such as 'hurd' for anything else (see Cannot detect kernel during native build on Debian GNU/Hurd #13740 for the
question of whether Hurd should identify its kernel differently).
env2mfile: Base cpu on DEB_HOST_GNU_CPU unless DEB_HOST_ARCH is special
DEB_HOST_ARCH
encodes both the CPU family and the OS, so using it toget the CPU type gives the wrong answer for non-Linux ports.
However,
DEB_HOST_GNU_CPU
gives less detailed information about theCPU: it's
arm
for all 32-bit ARM CPUs, and doesn't distinguish betweenthe differing baselines of
armel
(ARMv5 softfloat) andarmhf
(ARMv7 hardfloat).
When cross-compiling for x86_64 Linux, this changes the
cpu()
fromamd64
tox86_64
, which is consistent with the answer we get duringnative builds on that architecture.
When cross-compiling for
ppc64el
, this changes thecpu()
fromppc64el
toppc64
, which is a reasonable change but is still notconsistent with what we see during native builds (which is
ppc64le
):see Inconsistent host_machine.cpu() for Debian ppc64el cross vs. native #13741 for that.
Resolves: Inconsistent host_machine.cpu() for Debian amd64 cross vs. native #13742
This is not a complete solution for #13740 and #13741, but would be a good starting point for fixing those afterwards.