From 4687e6dbb6f913ac886616dcbcbf7b216ec4e62f Mon Sep 17 00:00:00 2001 From: Eliah Kagan Date: Tue, 4 Mar 2025 04:35:17 +0000 Subject: [PATCH 01/38] Start toward testing s390x via cross --- Cross.toml | 2 ++ etc/docker/Dockerfile.test-cross-s390x | 11 +++++++++++ 2 files changed, 13 insertions(+) create mode 100644 Cross.toml create mode 100644 etc/docker/Dockerfile.test-cross-s390x diff --git a/Cross.toml b/Cross.toml new file mode 100644 index 00000000000..b7c1890604b --- /dev/null +++ b/Cross.toml @@ -0,0 +1,2 @@ +[target.s390x-unknown-linux-gnu] +image = "cross-rs-gitoxide/s390x-unknown-linux-gnu" diff --git a/etc/docker/Dockerfile.test-cross-s390x b/etc/docker/Dockerfile.test-cross-s390x new file mode 100644 index 00000000000..d248d5695b1 --- /dev/null +++ b/etc/docker/Dockerfile.test-cross-s390x @@ -0,0 +1,11 @@ +FROM ghcr.io/cross-rs/s390x-unknown-linux-gnu:latest + +RUN apt-get update && \ + apt-get install apt-transport-https && \ + echo 'deb https://ppa.launchpadcontent.net/git-core/ppa/ubuntu xenial main' > \ + /etc/apt/sources.list.d/git-core-ubuntu-ppa-xenial.list && \ + apt-key adv --keyserver keyserver.ubuntu.com \ + --recv-keys F911AB184317630C59970973E363C90F8F1B6217 && \ + apt-get update && \ + apt-get install --no-install-recommends -y git && \ + rm -rf /var/lib/apt/lists/* From a7db5888749899df8b2ebf2ce8636290ddd57565 Mon Sep 17 00:00:00 2001 From: Eliah Kagan Date: Tue, 4 Mar 2025 05:23:22 +0000 Subject: [PATCH 02/38] Add `system`-scope config var in container So installation_config tests can pass. This also adjusts the RUN script in the dockerfile to show each command before it runs and to make sure it is not overwriting files unintentionally, and adds a convenience script outside for building the image. --- etc/docker/Dockerfile.test-cross-s390x | 6 ++++-- etc/docker/build-test-cross-s390x-image.sh | 5 +++++ 2 files changed, 9 insertions(+), 2 deletions(-) create mode 100755 etc/docker/build-test-cross-s390x-image.sh diff --git a/etc/docker/Dockerfile.test-cross-s390x b/etc/docker/Dockerfile.test-cross-s390x index d248d5695b1..e6f7c138d83 100644 --- a/etc/docker/Dockerfile.test-cross-s390x +++ b/etc/docker/Dockerfile.test-cross-s390x @@ -1,6 +1,7 @@ FROM ghcr.io/cross-rs/s390x-unknown-linux-gnu:latest -RUN apt-get update && \ +RUN set -xC && \ + apt-get update && \ apt-get install apt-transport-https && \ echo 'deb https://ppa.launchpadcontent.net/git-core/ppa/ubuntu xenial main' > \ /etc/apt/sources.list.d/git-core-ubuntu-ppa-xenial.list && \ @@ -8,4 +9,5 @@ RUN apt-get update && \ --recv-keys F911AB184317630C59970973E363C90F8F1B6217 && \ apt-get update && \ apt-get install --no-install-recommends -y git && \ - rm -rf /var/lib/apt/lists/* + rm -rf /var/lib/apt/lists/* && \ + git config --system gitoxide.imaginary.arbitraryVariable arbitraryValue diff --git a/etc/docker/build-test-cross-s390x-image.sh b/etc/docker/build-test-cross-s390x-image.sh new file mode 100755 index 00000000000..41ef43dbcba --- /dev/null +++ b/etc/docker/build-test-cross-s390x-image.sh @@ -0,0 +1,5 @@ +#!/bin/sh +set -e +mkdir empty-context +docker build -f etc/docker/Dockerfile.test-cross-s390x \ + -t cross-rs-gitoxide/s390x-unknown-linux-gnu empty-context From 3a296aefa67f8983beeba3b717e50f2904b35ca2 Mon Sep 17 00:00:00 2001 From: Eliah Kagan Date: Tue, 4 Mar 2025 20:03:17 +0000 Subject: [PATCH 03/38] Add `jq` to container; simplify container build command Installing `jq` in the container allows the `ask::askpass_only` and `ask::username_password` to pass (they use `jq`). --- etc/docker/Dockerfile.test-cross-s390x | 2 +- etc/docker/build-test-cross-s390x-image.sh | 5 +---- 2 files changed, 2 insertions(+), 5 deletions(-) diff --git a/etc/docker/Dockerfile.test-cross-s390x b/etc/docker/Dockerfile.test-cross-s390x index e6f7c138d83..775700a62e2 100644 --- a/etc/docker/Dockerfile.test-cross-s390x +++ b/etc/docker/Dockerfile.test-cross-s390x @@ -8,6 +8,6 @@ RUN set -xC && \ apt-key adv --keyserver keyserver.ubuntu.com \ --recv-keys F911AB184317630C59970973E363C90F8F1B6217 && \ apt-get update && \ - apt-get install --no-install-recommends -y git && \ + apt-get install --no-install-recommends -y git jq && \ rm -rf /var/lib/apt/lists/* && \ git config --system gitoxide.imaginary.arbitraryVariable arbitraryValue diff --git a/etc/docker/build-test-cross-s390x-image.sh b/etc/docker/build-test-cross-s390x-image.sh index 41ef43dbcba..ce47c05bb2a 100755 --- a/etc/docker/build-test-cross-s390x-image.sh +++ b/etc/docker/build-test-cross-s390x-image.sh @@ -1,5 +1,2 @@ #!/bin/sh -set -e -mkdir empty-context -docker build -f etc/docker/Dockerfile.test-cross-s390x \ - -t cross-rs-gitoxide/s390x-unknown-linux-gnu empty-context +docker build -t cross-rs-gitoxide/s390x-unknown-linux-gnu - Date: Tue, 4 Mar 2025 22:17:16 +0000 Subject: [PATCH 04/38] Put experiment details (except dockerfile) in a script --- etc/docker/build-test-cross-s390x-image.sh | 2 -- etc/run-cross-experiment.sh | 15 +++++++++++++++ 2 files changed, 15 insertions(+), 2 deletions(-) delete mode 100755 etc/docker/build-test-cross-s390x-image.sh create mode 100755 etc/run-cross-experiment.sh diff --git a/etc/docker/build-test-cross-s390x-image.sh b/etc/docker/build-test-cross-s390x-image.sh deleted file mode 100755 index ce47c05bb2a..00000000000 --- a/etc/docker/build-test-cross-s390x-image.sh +++ /dev/null @@ -1,2 +0,0 @@ -#!/bin/sh -docker build -t cross-rs-gitoxide/s390x-unknown-linux-gnu - Date: Wed, 5 Mar 2025 09:38:38 +0000 Subject: [PATCH 05/38] Use image:tag naming style for image; let more env through This changes the style of how we name the custom Docker image for `cross` testing, so it is not looked up on Docker Hub (it is not there, and the text before the `/` in the old name was not intended to have a meaning related to resolving the image in any registry). This also allows through the environment varibles that currently have special meaning to gitoxide or its test suite -- including `GIX_TEST_IGNORE_ARCHIVES` to test the fixture scripts in the container, if it is set for the `cross` process -- or that the test suite may reasonably use to detect CI in general or GitHub Actions in particular. (Most GitHub Actions environment variables continue not to be passed through, because some of them would hold paths that couldn't necessarily be resolved properly in the container, while the presence of others would wrongly create the appearance that all GitHub Actions environment variables would be usable.) --- Cross.toml | 16 +++++++++++++++- etc/run-cross-experiment.sh | 2 +- 2 files changed, 16 insertions(+), 2 deletions(-) diff --git a/Cross.toml b/Cross.toml index b7c1890604b..37877834e2e 100644 --- a/Cross.toml +++ b/Cross.toml @@ -1,2 +1,16 @@ +[build.env] +passthrough = [ + "CI", + "GITHUB_ACTIONS", + "GIX_CREDENTIALS_HELPER_STDERR", + "GIX_EXTERNAL_COMMAND_STDERR", + "GIX_OBJECT_CACHE_MEMORY", + "GIX_PACK_CACHE_MEMORY", + "GIX_TEST_CREATE_ARCHIVES_EVEN_ON_CI", + "GIX_TEST_EXPECT_REDUCED_TRUST", + "GIX_TEST_IGNORE_ARCHIVES", + "GIX_VERSION", +] + [target.s390x-unknown-linux-gnu] -image = "cross-rs-gitoxide/s390x-unknown-linux-gnu" +image = "cross-rs-gitoxide:s390x-unknown-linux-gnu" diff --git a/etc/run-cross-experiment.sh b/etc/run-cross-experiment.sh index eb55b4646d3..323d9316007 100755 --- a/etc/run-cross-experiment.sh +++ b/etc/run-cross-experiment.sh @@ -2,7 +2,7 @@ set -ex # Build the customized `cross` container image. -docker build -t cross-rs-gitoxide/s390x-unknown-linux-gnu \ +docker build -t cross-rs-gitoxide:s390x-unknown-linux-gnu \ - Date: Sat, 8 Mar 2025 11:08:19 +0000 Subject: [PATCH 06/38] Start toward testing armv7-linux-androideabi --- Cross.toml | 3 +++ etc/docker/Dockerfile.test-cross | 15 +++++++++++++++ etc/docker/Dockerfile.test-cross-s390x | 13 ------------- etc/run-cross-experiment.sh | 15 +++++++++++---- 4 files changed, 29 insertions(+), 17 deletions(-) create mode 100644 etc/docker/Dockerfile.test-cross delete mode 100644 etc/docker/Dockerfile.test-cross-s390x diff --git a/Cross.toml b/Cross.toml index 37877834e2e..b891146c831 100644 --- a/Cross.toml +++ b/Cross.toml @@ -12,5 +12,8 @@ passthrough = [ "GIX_VERSION", ] +[target.armv7-linux-androideabi] +image = "cross-rs-gitoxide:armv7-linux-androideabi" + [target.s390x-unknown-linux-gnu] image = "cross-rs-gitoxide:s390x-unknown-linux-gnu" diff --git a/etc/docker/Dockerfile.test-cross b/etc/docker/Dockerfile.test-cross new file mode 100644 index 00000000000..704c4d593be --- /dev/null +++ b/etc/docker/Dockerfile.test-cross @@ -0,0 +1,15 @@ +ARG TARGET + +FROM ghcr.io/cross-rs/${TARGET}:latest + +RUN apt-get update && \ + apt-get install --no-install-recommends -y apt-transport-https gnupg && \ + release="$(sed -n 's/^VERSION_CODENAME=//p' /etc/os-release)" && \ + echo "deb https://ppa.launchpadcontent.net/git-core/ppa/ubuntu $release main" \ + >/etc/apt/sources.list.d/git-core-ubuntu-ppa.list && \ + apt-key adv --keyserver keyserver.ubuntu.com \ + --recv-keys F911AB184317630C59970973E363C90F8F1B6217 && \ + apt-get update && \ + apt-get install --no-install-recommends -y git jq && \ + rm -rf /var/lib/apt/lists/* && \ + git config --system gitoxide.imaginary.arbitraryVariable arbitraryValue diff --git a/etc/docker/Dockerfile.test-cross-s390x b/etc/docker/Dockerfile.test-cross-s390x deleted file mode 100644 index 775700a62e2..00000000000 --- a/etc/docker/Dockerfile.test-cross-s390x +++ /dev/null @@ -1,13 +0,0 @@ -FROM ghcr.io/cross-rs/s390x-unknown-linux-gnu:latest - -RUN set -xC && \ - apt-get update && \ - apt-get install apt-transport-https && \ - echo 'deb https://ppa.launchpadcontent.net/git-core/ppa/ubuntu xenial main' > \ - /etc/apt/sources.list.d/git-core-ubuntu-ppa-xenial.list && \ - apt-key adv --keyserver keyserver.ubuntu.com \ - --recv-keys F911AB184317630C59970973E363C90F8F1B6217 && \ - apt-get update && \ - apt-get install --no-install-recommends -y git jq && \ - rm -rf /var/lib/apt/lists/* && \ - git config --system gitoxide.imaginary.arbitraryVariable arbitraryValue diff --git a/etc/run-cross-experiment.sh b/etc/run-cross-experiment.sh index 323d9316007..863d49d71ce 100755 --- a/etc/run-cross-experiment.sh +++ b/etc/run-cross-experiment.sh @@ -1,15 +1,22 @@ #!/bin/sh -set -ex + +# Usage: +# +# etc/run-cross-experiment.sh armv7-linux-androideabi +# etc/run-cross-experiment.sh s390x-unknown-linux-gnu + +set -eux +target="$1" # Build the customized `cross` container image. -docker build -t cross-rs-gitoxide:s390x-unknown-linux-gnu \ - - Date: Sun, 9 Mar 2025 03:58:23 +0000 Subject: [PATCH 07/38] feat: Show unexpected stderr in `umask` panic message Because `gix_testtools::umask()` is only suitable for use in tests, where signaling an error with a panic is typically acceptable, it panics rather than returning an `Error` to indicate errors. To avoid leading to the use of a potentially inaccurate umask value, it treats as an error any departure from the typical output of the `umask` command: in addition to treating a nonzero exit status as an error, it also treats anything it cannot strictly parse on stdout as an error, as well as anything at all written to stderr as an error. The latter is to avoid a situation where a warning is printed that is could be significant to some umask use cases. Warnings from `umask` are rare, as well as from the shell that is used as an intermediary for running the command (since no external `umask` command need exist and, often, does not) when it is used just to run `umask`. When they do occur, they are sometimes from the dynamic linker, such as a warning about a shared library listed in the `LD_PRELOAD` environment variable that cannot be used by the shell program. To understand and distinguish such errors, it is useful to show the text that was sent to stderr, since tests are sometimes run in environments that are nontrivial to reproduce otherwise. For example, running tests with `cross` produces an environment that is not in all respects the same as what one gets with `docker exec -it `, even if `` is the same still-running container being used to run the tests. This modifies `gix_testtools::umask()` so that when it panics due anything being written to stderr, it shows what was written. --- tests/tools/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/tools/src/lib.rs b/tests/tools/src/lib.rs index cbe2b42b948..a9c2938a56f 100644 --- a/tests/tools/src/lib.rs +++ b/tests/tools/src/lib.rs @@ -992,7 +992,7 @@ pub fn umask() -> u32 { .output() .expect("can execute `sh -c umask`"); assert!(output.status.success(), "`sh -c umask` failed"); - assert!(output.stderr.is_empty(), "`sh -c umask` unexpected message"); + assert_eq!(output.stderr.as_bstr(), "", "`sh -c umask` unexpected message"); let text = output.stdout.to_str().expect("valid Unicode").trim(); u32::from_str_radix(text, 8).expect("parses as octal number") } From 8d596b4179f37fbecf09291f29c15103851b63f2 Mon Sep 17 00:00:00 2001 From: Eliah Kagan Date: Sun, 9 Mar 2025 04:08:48 +0000 Subject: [PATCH 08/38] Add `env` testtools subcommand to show the environment (This is only temporarily in `gix-testtools` and shall either be moved to `internal-tools` or removed altogether.) It is useful to be able to observe environment variables that are set when running code with tools such as `cargo` or `cross`. This is therefore not intended for general-purpose use. A system command like `printenv` or `env` or a shell facility should be preferred where applicable, since it is considered a feature rather than a bug that commands like `cargo run -p gix-testtools env` include changes to the environment from `cargo` itself. Since one use for checking environment variables is to investigate the effects of environments that contain variable names or values that are not valid Unicode, this avoids requiring that environment variables all be Unicode. Any name or value that is not Unicode is shown in its Rust debug representation. This is always quoted, and to decrease ambiguity any name or (more likely) value that contains literal double quotes is likewise shown in its debug representation so that it is always clear if a quotation mark is just for display. Each name and value is otherwise shown literally. In either case the name or its representation is separated by a `=` from the value or its representation. --- tests/tools/src/main.rs | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/tests/tools/src/main.rs b/tests/tools/src/main.rs index 66a485dd360..420fd7778b1 100644 --- a/tests/tools/src/main.rs +++ b/tests/tools/src/main.rs @@ -9,6 +9,19 @@ fn bash_program() -> io::Result<()> { Ok(()) } +fn env() -> io::Result<()> { + fn repr(text: &std::ffi::OsStr) -> String { + text.to_str() + .filter(|s| !s.contains('"')) + .map(ToOwned::to_owned) + .unwrap_or_else(|| format!("{text:?}")) + } + for (name, value) in std::env::vars_os() { + println!("{}={}", repr(&name), repr(&value)); + } + Ok(()) +} + fn mess_in_the_middle(path: PathBuf) -> io::Result<()> { let mut file = fs::OpenOptions::new().read(false).write(true).open(path)?; file.seek(io::SeekFrom::Start(file.metadata()?.len() / 2))?; @@ -27,6 +40,7 @@ fn main() -> Result<(), Box> { let scmd = args.next().expect("sub command"); match &*scmd { "bash-program" | "bp" => bash_program()?, + "env" => env()?, "mess-in-the-middle" => mess_in_the_middle(PathBuf::from(args.next().expect("path to file to mess with")))?, #[cfg(unix)] "umask" => umask()?, From ab2b26c8e0fb3b9c2bae956f07c17e960c98f0a0 Mon Sep 17 00:00:00 2001 From: Eliah Kagan Date: Sun, 9 Mar 2025 06:25:50 +0000 Subject: [PATCH 09/38] Recognize `NO_PRELOAD_CXX` to not preload Android C++ library This further customizes the `cross` Dockerfile so it patches the `/android-runner` script in the image so, when the script runs, it checks if something like `NO_PRELOAD_CXX=1` is set (specifically, if `NO_PRELOAD_CXX` exists with a value other than the empty string or `0`) and, if so, refrains from setting `LD_PRELOAD` to the path of an Android C++ standard library implementation. (This also adds `NO_PRELOAD_CXX` to the list of variables `cross` allows to pass from host to container environment, so it can be set easily.) The problem this solves is warnings, which appear to be errors based on their wording but which do not inherently cause failure, that a library cannot be loaded into executables that are for a different architecture. `ld.so` issues these when running x86-64 executables in the `cross` container for `armv7-linux-androideabi`. Currently that includes shells (`sh`, `bash`) and `git`. ERROR: ld.so: object '/android-ndk/sysroot/usr/lib/arm-linux-androideabi/libc++_shared.so' from LD_PRELOAD cannot be preloaded (wrong ELF class: ELFCLASS32): ignored. While the warnings do not inherently cause failure, removing them is not just to decrease noise in the output of test runs. Because the messages are sent to stderr, they are picked up in code that expects stderr to be of a particular form, including test cases that include assertions about what is written to standard error. In addition to not representing actual bugs, the resulting test failures are confusing because they consist of a message that a library cannot be preloaded for a platform-related reason ("wrong ELF class"), followed by the failure of a test that exercises code that runs an external command (or possibly, in some cases, tests that capture stderr in a fixture script to set up a test). That creates the appearance that the external command could not be run, when actually the problem is that the external command ran, often with no problems at all, but its captured stderr contained one or (if it ran its own subprocesses) more extra lines describing themselves as errors. In e94526e and 094023a (both in #1687), I had claimed in comments that the reason to test `gix-hashtable` rather than `gix` in the `armv7-linux-androideabi` job (since removed) was that, in `armv7-linux-androideabi`, some `gix` tests needed to be able to run `git`, and needed ARMv7 `git`. This is technically true, but misleading (and while the logs have expired so I cannot check what I saw at that time, I suspect I was myself misled). They have relied on ARMv7 `git` on such a platform in the sense that the x86-64 `git` is completely usable but causes "wrong ELF class" warnings, which appear to be errors, to be written to stderr, breaking tests or test fixtures that examine stderr (as well as any test that attempts to read a `once_cell::sync::Lazy` that a previous test run in the same process tried to initialize by running code that panicked due to such a failure -- when control exits a `Lazy` initializer due to unwinding *from a panic*, the cell enters a "poisoned" state and subsequent uses of it fail). If an ARMv7 `git` were to be used, then the shared library -- even if `git` didn't use the library -- would presumably be preloaded, which would avoid the warning. But the problem was not inherent to the architecture or build of binares like `git`, instead arising from the incompatible `LD_PRELOAD` value. (Also, as far as I know, we have so far never used an ARMv7 build of `git` in any `cross` test... though it is used in the current `test-32bit` arm32v7 job.) Avoiding setting `LD_PRELOAD`, as done here, fixes that. However, this is not an ideal solution, because the library may be needed if an external command is built for the Android target architecture and is written in C++. It might be ideal to suppress the warnings while still attempting to load the library, but it's not clear if a reasonably simple way to do so is available: - `LD_PRELOAD` paths support `$LIB`, but I am not sure it can be used here, because the library name, and not just the directory prefix, differs. (Also, when not attempting to link to the C++ stdlib for Android, nothing would *ordinarily* be added to `LD_PRELOAD` at all.) - glibc issue https://sourceware.org/bugzilla/show_bug.cgi?id=20172 proposes a new environment variable `LD_PRELOAD_IGNORE_ERRORS`, but no patch with that feature has been merged into any version of glibc at this time (so certainly not the fairly old glibc in the `cross` image). The other reason this is not an ideal solution is that, at least if testing with `cross` is to become fully useful, some external commands in the `cross` container *should* be built for the target architecture, because in practice those are the builds (or in some cases, ports) that gitoxide would have to be compatible with. That includes `git` and, if feasible, `sh`, and maybe some others such as `gpg` and (if sufficiently tested to matter) `ssh`. So while the existing workaround makes it so ARMv7 `git` is not necessary to keep the tests from failing when they should pass, it remains potentially necessary to keep the tests from passing when they should fail. (This also applies to other architectures tested with `cross`, particularly s390x, where failure occurring in native runs, mentioned in #1687 and discused in #1698 comments, have not yet been reproducible when testing with `cross`. The reason is *possibly* some difference between the behavior of x86-64 and s390x `git`.) However, even if that can be achieved, it will likely also be an incomplete solution on its own, because as long as we may ever capture stderr from *some* external command where the warning is issued, we have to suppress or tolerate the warning in some way. --- Cross.toml | 1 + etc/docker/Dockerfile.test-cross | 1 + etc/run-cross-experiment.sh | 2 +- 3 files changed, 3 insertions(+), 1 deletion(-) diff --git a/Cross.toml b/Cross.toml index b891146c831..88913f12fcc 100644 --- a/Cross.toml +++ b/Cross.toml @@ -10,6 +10,7 @@ passthrough = [ "GIX_TEST_EXPECT_REDUCED_TRUST", "GIX_TEST_IGNORE_ARCHIVES", "GIX_VERSION", + "NO_PRELOAD_CXX", ] [target.armv7-linux-androideabi] diff --git a/etc/docker/Dockerfile.test-cross b/etc/docker/Dockerfile.test-cross index 704c4d593be..edbc978a7e8 100644 --- a/etc/docker/Dockerfile.test-cross +++ b/etc/docker/Dockerfile.test-cross @@ -12,4 +12,5 @@ RUN apt-get update && \ apt-get update && \ apt-get install --no-install-recommends -y git jq && \ rm -rf /var/lib/apt/lists/* && \ + sed -i.orig 's/^export LD_PRELOAD=/test "${NO_PRELOAD_CXX:-0}" != 0 || &/' /android-runner && \ git config --system gitoxide.imaginary.arbitraryVariable arbitraryValue diff --git a/etc/run-cross-experiment.sh b/etc/run-cross-experiment.sh index 863d49d71ce..e716d7e99a2 100755 --- a/etc/run-cross-experiment.sh +++ b/etc/run-cross-experiment.sh @@ -17,6 +17,6 @@ cargo clean gix clean -xd -m '*generated*' -e # Run the test suite. -cross test --workspace --no-fail-fast --target "$target" \ +NO_PRELOAD_CXX=1 cross test --workspace --no-fail-fast --target "$target" \ --no-default-features --features max-pure \ -- --skip realpath::fuzzed_timeout From b26c2a2205553fcb6df10706ce3404db62125cd3 Mon Sep 17 00:00:00 2001 From: Eliah Kagan Date: Sun, 9 Mar 2025 08:08:26 +0000 Subject: [PATCH 10/38] Only patch `/android-runner` if it exists So the `cross` Dockerfile can continue to be used for both Android and non-Android targets. --- etc/docker/Dockerfile.test-cross | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/etc/docker/Dockerfile.test-cross b/etc/docker/Dockerfile.test-cross index edbc978a7e8..cad60f0bc29 100644 --- a/etc/docker/Dockerfile.test-cross +++ b/etc/docker/Dockerfile.test-cross @@ -12,5 +12,7 @@ RUN apt-get update && \ apt-get update && \ apt-get install --no-install-recommends -y git jq && \ rm -rf /var/lib/apt/lists/* && \ - sed -i.orig 's/^export LD_PRELOAD=/test "${NO_PRELOAD_CXX:-0}" != 0 || &/' /android-runner && \ + to_patch=/android-runner && \ + patcher='s/^export LD_PRELOAD=/test "${NO_PRELOAD_CXX:-0}" != 0 || &/' && \ + if test -f "$to_patch"; then sed -i.orig "$patcher" -- "$to_patch"; fi && \ git config --system gitoxide.imaginary.arbitraryVariable arbitraryValue From 35627b5146c6cbf72a41b1ef4cd6b3d64010c0bc Mon Sep 17 00:00:00 2001 From: Eliah Kagan Date: Sun, 9 Mar 2025 08:15:36 +0000 Subject: [PATCH 11/38] Change `interpolate_user` tests to cover Android `interpolate_user()` gives `UserInterpolationUnsupported` on both Windows and Android, which is intentional. But the tests treated Windows as the only platform under which this was the case, thereby failing on Android tests. This may have been obscured by prior difficulties running tests on Android builds of `gix-config-value`. --- gix-config-value/tests/value/path.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/gix-config-value/tests/value/path.rs b/gix-config-value/tests/value/path.rs index 94259d6355b..999c07b06f9 100644 --- a/gix-config-value/tests/value/path.rs +++ b/gix-config-value/tests/value/path.rs @@ -89,16 +89,16 @@ mod interpolate { Ok(()) } - #[cfg(windows)] + #[cfg(any(target_os = "windows", target_os = "android"))] #[test] - fn tilde_with_given_user_is_unsupported_on_windows() { + fn tilde_with_given_user_is_unsupported_on_windows_and_android() { assert!(matches!( interpolate_without_context("~baz/foo/bar"), Err(gix_config_value::path::interpolate::Error::UserInterpolationUnsupported) )); } - #[cfg(not(windows))] + #[cfg(not(any(target_os = "windows", target_os = "android")))] #[test] fn tilde_with_given_user() -> crate::Result { let home = std::env::current_dir()?; From c19bf1de76a3d6afc7740bdbc35181276bfcb58d Mon Sep 17 00:00:00 2001 From: Eliah Kagan Date: Sun, 9 Mar 2025 08:40:30 +0000 Subject: [PATCH 12/38] Test `gix_testtools::umask()` on Android targets Rather than only on non-Android Linux-based systems. Like most Linux-based systems, Android has `/proc`, so the test can get the umask expected value using its `/proc`-based approach that is independent of the approach used in the code under test. --- tests/tools/tests/umask.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/tests/tools/tests/umask.rs b/tests/tools/tests/umask.rs index 0a4cc04250b..5463b57ad49 100644 --- a/tests/tools/tests/umask.rs +++ b/tests/tools/tests/umask.rs @@ -1,6 +1,9 @@ #[test] #[cfg(unix)] -#[cfg_attr(not(target_os = "linux"), ignore = "The test itself uses /proc")] +#[cfg_attr( + not(any(target_os = "linux", target_os = "android")), + ignore = "The test itself uses /proc" +)] fn umask() { use std::fs::File; use std::io::{BufRead, BufReader}; From d3a9f5ff5771d4d91ddb8a8cdddbc51a03c4667f Mon Sep 17 00:00:00 2001 From: Eliah Kagan Date: Sun, 9 Mar 2025 11:19:55 +0000 Subject: [PATCH 13/38] Move `cross` testing commands into `justfile` --- etc/run-cross-experiment.sh | 22 ---------------------- justfile | 17 +++++++++++++++++ 2 files changed, 17 insertions(+), 22 deletions(-) delete mode 100755 etc/run-cross-experiment.sh diff --git a/etc/run-cross-experiment.sh b/etc/run-cross-experiment.sh deleted file mode 100755 index e716d7e99a2..00000000000 --- a/etc/run-cross-experiment.sh +++ /dev/null @@ -1,22 +0,0 @@ -#!/bin/sh - -# Usage: -# -# etc/run-cross-experiment.sh armv7-linux-androideabi -# etc/run-cross-experiment.sh s390x-unknown-linux-gnu - -set -eux -target="$1" - -# Build the customized `cross` container image. -docker build --build-arg "TARGET=$target" -t "cross-rs-gitoxide:$target" \ - - Date: Sun, 9 Mar 2025 11:23:17 +0000 Subject: [PATCH 14/38] Ensure `cross` test images have `patch` For `armv7-linux-androideabi`, in GIX_TEST_IGNORE_ARCHIVES=1 cross test ... as tested by running GIX_TEST_IGNORE_ARCHIVES=1 just cross-test-android the `id::ancestors::pre_epoch` test failed with: /project/gix/tests/fixtures/make_pre_epoch_repo.sh: line 12: patch: command not found This fixes it by making sure `patch` is installed in the container. --- etc/docker/Dockerfile.test-cross | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/etc/docker/Dockerfile.test-cross b/etc/docker/Dockerfile.test-cross index cad60f0bc29..83ae0c3e3f6 100644 --- a/etc/docker/Dockerfile.test-cross +++ b/etc/docker/Dockerfile.test-cross @@ -10,7 +10,7 @@ RUN apt-get update && \ apt-key adv --keyserver keyserver.ubuntu.com \ --recv-keys F911AB184317630C59970973E363C90F8F1B6217 && \ apt-get update && \ - apt-get install --no-install-recommends -y git jq && \ + apt-get install --no-install-recommends -y git jq patch && \ rm -rf /var/lib/apt/lists/* && \ to_patch=/android-runner && \ patcher='s/^export LD_PRELOAD=/test "${NO_PRELOAD_CXX:-0}" != 0 || &/' && \ From 239eee49ba4d14d03886c84c55a8636fe81df1dc Mon Sep 17 00:00:00 2001 From: Eliah Kagan Date: Sun, 9 Mar 2025 12:15:09 +0000 Subject: [PATCH 15/38] Keep the `cross` test config from affecting other `cross` usage Since we also use `cross` to build some of the releases. --- Cross.toml => etc/docker/test-cross.toml | 4 ++++ justfile | 3 ++- 2 files changed, 6 insertions(+), 1 deletion(-) rename Cross.toml => etc/docker/test-cross.toml (68%) diff --git a/Cross.toml b/etc/docker/test-cross.toml similarity index 68% rename from Cross.toml rename to etc/docker/test-cross.toml index 88913f12fcc..ce921b42b21 100644 --- a/Cross.toml +++ b/etc/docker/test-cross.toml @@ -1,3 +1,7 @@ +# `cross` configuration for running tests. Treated like `Cross.toml` if enabled +# with `CROSS_CONFIG=etc/docker/test-cross.toml`. This avoids affecting other +# `cross` usage, e.g. in `release.yml`. See `cross-test` recipes in `justfile`. + [build.env] passthrough = [ "CI", diff --git a/justfile b/justfile index 0f2f19f2a1c..58f78b525f5 100755 --- a/justfile +++ b/justfile @@ -231,7 +231,8 @@ cross-image target: # Test another platform with `cross` cross-test target: (cross-image target) - NO_PRELOAD_CXX=1 cross test --workspace --no-fail-fast --target "{{ target }}" \ + CROSS_CONFIG=etc/docker/test-cross.toml NO_PRELOAD_CXX=1 \ + cross test --workspace --no-fail-fast --target "{{ target }}" \ --no-default-features --features max-pure \ -- --skip realpath::fuzzed_timeout From 2fb7db84dd6679b8c4c410afc0121be37497f6eb Mon Sep 17 00:00:00 2001 From: Eliah Kagan Date: Sun, 9 Mar 2025 22:22:40 +0000 Subject: [PATCH 16/38] Remove potentially misleading "(limited)" for Android This removes "(limited)" from the description of the `justfile` recipe `cross-test-android`. It *is* very limited, but that label is misleading, since currently `cross-test-s390x` (as well as other `cross` testing targets that could be run through the parameterized `cross-test` recipe) are limited in all the same ways. In particular, we are not currently testing them with any external components built for the target architecture. For example, `git` built for the host has been upgraded in the container and the environment has been configured so `gix-path`, `gix-testtools`, any test cases throughout the workspace can use it without problems (either directly or through a shell). In the future, some `cross` targets may gain a richer target environment to interact with when testing, by installing target builds of `git`, by installing target builds of libraries that facilitate more features than `max-pure`, or in other ways. At that point, convenience recipes for specific targets in the `justfile` that are more limited than other targets could be marked "(limited)". --- justfile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/justfile b/justfile index 58f78b525f5..ce5ba3666d9 100755 --- a/justfile +++ b/justfile @@ -239,7 +239,7 @@ cross-test target: (cross-image target) # Test s390x with `cross` cross-test-s390x: (cross-test 's390x-unknown-linux-gnu') -# Test Android with `cross` (limited) +# Test Android with `cross` cross-test-android: (cross-test 'armv7-linux-androideabi') # Run `cargo diet` on all crates to see that they are still in bounds From 809fb2f3fa2ceaef7fbcad5222ec6b892a086a9b Mon Sep 17 00:00:00 2001 From: Eliah Kagan Date: Mon, 10 Mar 2025 03:07:51 +0000 Subject: [PATCH 17/38] Move `env` subcommand to `internal-tools` From `gix-testtools`, where it was originally placed. This also adapts and improves the original description from the commit message that had introduced the subcommand, to make clear when it makes sense to use this (given that operating systems and shells already supply facilities for listing environment variables). --- tests/it/src/args.rs | 17 +++++++++++++++++ tests/it/src/commands/env.rs | 15 +++++++++++++++ tests/it/src/commands/mod.rs | 3 +++ tests/it/src/main.rs | 1 + tests/tools/src/main.rs | 14 -------------- 5 files changed, 36 insertions(+), 14 deletions(-) create mode 100644 tests/it/src/commands/env.rs diff --git a/tests/it/src/args.rs b/tests/it/src/args.rs index e0ccc820ae6..7d9f9a5ef53 100644 --- a/tests/it/src/args.rs +++ b/tests/it/src/args.rs @@ -73,6 +73,23 @@ pub enum Subcommands { /// current repository. Its main use is checking that fixture scripts are have correct modes. #[clap(visible_alias = "cm")] CheckMode {}, + /// Print environment variables as `NAME=value` lines. + /// + /// It is useful to be able to observe environment variables that are set when running code + /// with tools such as `cargo` or `cross`. Commands like `cargo run -p internal-tools -- env` + /// include environment changes from `cargo` itself. With `cross`, changes are more extensive, + /// due to the effect of `build.env.passthrough`, container customization, and existing special + /// cases in wrapper scripts shipped in default `cross` containers (such as to `LD_PRELOAD`). + /// + /// Since one use for checking environment variables is to investigate the effects of + /// environments that contain variable names or values that are not valid Unicode, this avoids + /// requiring that environment variables all be Unicode. Any name or value that is not Unicode + /// is shown in its Rust debug representation. This is always quoted, and to decrease ambiguity + /// any name or (more likely) value that contains literal double quotes is likewise shown in + /// its debug representation so that it is always clear if a quotation mark is just for + /// display. Each name and value is otherwise shown literally. + #[clap(visible_alias = "e")] + Env {}, } #[derive(Clone)] diff --git a/tests/it/src/commands/env.rs b/tests/it/src/commands/env.rs new file mode 100644 index 00000000000..0f95ed90221 --- /dev/null +++ b/tests/it/src/commands/env.rs @@ -0,0 +1,15 @@ +pub(super) mod function { + pub fn env() -> anyhow::Result<()> { + for (name, value) in std::env::vars_os() { + println!("{}={}", repr(&name), repr(&value)); + } + Ok(()) + } + + fn repr(text: &std::ffi::OsStr) -> String { + text.to_str() + .filter(|s| !s.contains('"')) + .map(ToOwned::to_owned) + .unwrap_or_else(|| format!("{text:?}")) + } +} diff --git a/tests/it/src/commands/mod.rs b/tests/it/src/commands/mod.rs index adc7a6decb2..ae00a26a8d7 100644 --- a/tests/it/src/commands/mod.rs +++ b/tests/it/src/commands/mod.rs @@ -6,3 +6,6 @@ pub use git_to_sh::function::git_to_sh; pub mod check_mode; pub use check_mode::function::check_mode; + +pub mod env; +pub use env::function::env; diff --git a/tests/it/src/main.rs b/tests/it/src/main.rs index 847978481df..f18dd3450be 100644 --- a/tests/it/src/main.rs +++ b/tests/it/src/main.rs @@ -32,6 +32,7 @@ fn main() -> anyhow::Result<()> { patterns, } => commands::copy_royal(dry_run, &worktree_root, destination_dir, patterns), Subcommands::CheckMode {} => commands::check_mode(), + Subcommands::Env {} => commands::env(), } } diff --git a/tests/tools/src/main.rs b/tests/tools/src/main.rs index 420fd7778b1..66a485dd360 100644 --- a/tests/tools/src/main.rs +++ b/tests/tools/src/main.rs @@ -9,19 +9,6 @@ fn bash_program() -> io::Result<()> { Ok(()) } -fn env() -> io::Result<()> { - fn repr(text: &std::ffi::OsStr) -> String { - text.to_str() - .filter(|s| !s.contains('"')) - .map(ToOwned::to_owned) - .unwrap_or_else(|| format!("{text:?}")) - } - for (name, value) in std::env::vars_os() { - println!("{}={}", repr(&name), repr(&value)); - } - Ok(()) -} - fn mess_in_the_middle(path: PathBuf) -> io::Result<()> { let mut file = fs::OpenOptions::new().read(false).write(true).open(path)?; file.seek(io::SeekFrom::Start(file.metadata()?.len() / 2))?; @@ -40,7 +27,6 @@ fn main() -> Result<(), Box> { let scmd = args.next().expect("sub command"); match &*scmd { "bash-program" | "bp" => bash_program()?, - "env" => env()?, "mess-in-the-middle" => mess_in_the_middle(PathBuf::from(args.next().expect("path to file to mess with")))?, #[cfg(unix)] "umask" => umask()?, From c9b0abe352de9aadffaddeb14202a7206a1fb966 Mon Sep 17 00:00:00 2001 From: Eliah Kagan Date: Mon, 10 Mar 2025 04:06:42 +0000 Subject: [PATCH 18/38] Have `it env` use quoted/debug form when newlines are present In addition to any invalid Unicode and double quotes, as before. + Revise its documentation. --- tests/it/src/args.rs | 9 ++++----- tests/it/src/commands/env.rs | 2 +- 2 files changed, 5 insertions(+), 6 deletions(-) diff --git a/tests/it/src/args.rs b/tests/it/src/args.rs index 7d9f9a5ef53..537842ed328 100644 --- a/tests/it/src/args.rs +++ b/tests/it/src/args.rs @@ -78,16 +78,15 @@ pub enum Subcommands { /// It is useful to be able to observe environment variables that are set when running code /// with tools such as `cargo` or `cross`. Commands like `cargo run -p internal-tools -- env` /// include environment changes from `cargo` itself. With `cross`, changes are more extensive, - /// due to the effect of `build.env.passthrough`, container customization, and existing special + /// due to effects of `build.env.passthrough`, container customization, and preexisting special /// cases in wrapper scripts shipped in default `cross` containers (such as to `LD_PRELOAD`). /// /// Since one use for checking environment variables is to investigate the effects of /// environments that contain variable names or values that are not valid Unicode, this avoids /// requiring that environment variables all be Unicode. Any name or value that is not Unicode - /// is shown in its Rust debug representation. This is always quoted, and to decrease ambiguity - /// any name or (more likely) value that contains literal double quotes is likewise shown in - /// its debug representation so that it is always clear if a quotation mark is just for - /// display. Each name and value is otherwise shown literally. + /// is shown in its Rust debug representation. This is always quoted. To decrease ambiguity, + /// any name or value containing a literal double quote or newline is also shown in its debug + /// representation. Names and values without such content are shown literally and not quoted. #[clap(visible_alias = "e")] Env {}, } diff --git a/tests/it/src/commands/env.rs b/tests/it/src/commands/env.rs index 0f95ed90221..ef610c341de 100644 --- a/tests/it/src/commands/env.rs +++ b/tests/it/src/commands/env.rs @@ -8,7 +8,7 @@ pub(super) mod function { fn repr(text: &std::ffi::OsStr) -> String { text.to_str() - .filter(|s| !s.contains('"')) + .filter(|s| !s.chars().any(|c| c == '"' || c == '\n')) .map(ToOwned::to_owned) .unwrap_or_else(|| format!("{text:?}")) } From a6afbfbf315c9424bfc35ac8e1d3913f5510191c Mon Sep 17 00:00:00 2001 From: Eliah Kagan Date: Mon, 17 Mar 2025 23:26:10 +0000 Subject: [PATCH 19/38] Let backtrace-related env vars into the `cross` container --- etc/docker/test-cross.toml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/etc/docker/test-cross.toml b/etc/docker/test-cross.toml index ce921b42b21..31c5e671b5b 100644 --- a/etc/docker/test-cross.toml +++ b/etc/docker/test-cross.toml @@ -15,6 +15,8 @@ passthrough = [ "GIX_TEST_IGNORE_ARCHIVES", "GIX_VERSION", "NO_PRELOAD_CXX", + "RUST_BACKTRACE", + "RUST_LIB_BACKTRACE", ] [target.armv7-linux-androideabi] From eeb52791c26beb4a90fcfa6b4d18cda24e6670f8 Mon Sep 17 00:00:00 2001 From: Eliah Kagan Date: Mon, 17 Mar 2025 23:51:51 +0000 Subject: [PATCH 20/38] Start on `test-cross` CI job definition This *should* be able to run as written, but it can be improved. --- .github/workflows/ci.yml | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index f80297f737c..8eed4bf90d9 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -327,6 +327,31 @@ jobs: - name: Test (nextest) run: cargo nextest run --target $env:TARGET --workspace --no-fail-fast size + # TODO: If this is kept, then remove `fail-fast: false`; run only a subset of the tests, unless + # building and running the whole test suite is fast; install the cross target early via + # `dtolnay/rust-toolchain`, listing it as an additional target; and add a `Swatinem/rust-cache` + # step to cache dependencies. + test-cross: + runs-on: ubuntu-latest + + strategy: + matrix: + moniker: [ android, s390x ] + fail-fast: false + + steps: + - uses: actions/checkout@v4 + - uses: dtolnay/rust-toolchain@stable + - uses: extractions/setup-just@v3 + - name: Install cross + uses: taiki-e/install-action@v2 + with: + tool: cross + - name: Test (unit) + env: + RUST_BACKTRACE: '1' + run: just cross-test-${{ matrix.moniker }} + lint: runs-on: ubuntu-latest @@ -484,6 +509,7 @@ jobs: env: # List all jobs that are intended NOT to block PR auto-merge here. EXPECTED_NONBLOCKING_JOBS: |- + test-cross cargo-deny-advisories wasm tests-pass From cfeb06dc2653f04a308d651c5abcafe74ab062a7 Mon Sep 17 00:00:00 2001 From: Eliah Kagan Date: Mon, 17 Mar 2025 23:58:57 +0000 Subject: [PATCH 21/38] Divide up the `test-cross` CI steps better Rather than having the last step build the image through the `justfile` recipe dependency, and rather than having it install the `cross` target implicitly due to `cross` finding that it is not yet installed, this performs those actions in preceding steps. --- .github/workflows/ci.yml | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 8eed4bf90d9..fab665ab119 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -327,30 +327,34 @@ jobs: - name: Test (nextest) run: cargo nextest run --target $env:TARGET --workspace --no-fail-fast size - # TODO: If this is kept, then remove `fail-fast: false`; run only a subset of the tests, unless - # building and running the whole test suite is fast; install the cross target early via - # `dtolnay/rust-toolchain`, listing it as an additional target; and add a `Swatinem/rust-cache` - # step to cache dependencies. + # FIXME: *If* this is kept, then remove `fail-fast: false`, run only a subset of the tests unless + # building and running the whole test suite is fast, and add a `Swatinem/rust-cache` step. test-cross: runs-on: ubuntu-latest strategy: matrix: - moniker: [ android, s390x ] + target: [ armv7-linux-androideabi, s390x-unknown-linux-gnu ] fail-fast: false steps: - uses: actions/checkout@v4 - - uses: dtolnay/rust-toolchain@stable + - name: Install Rust + uses: dtolnay/rust-toolchain@master + with: + toolchain: stable + targets: ${{ matrix.target }} - uses: extractions/setup-just@v3 - name: Install cross uses: taiki-e/install-action@v2 with: tool: cross + - name: Build cross image + run: just cross-image ${{ matrix.target }} - name: Test (unit) env: RUST_BACKTRACE: '1' - run: just cross-test-${{ matrix.moniker }} + run: just cross-test ${{ matrix.target }} lint: runs-on: ubuntu-latest From 7d66f9e2e945cb8effb42dd2de94f41a3df07c15 Mon Sep 17 00:00:00 2001 From: Eliah Kagan Date: Tue, 18 Mar 2025 00:40:07 +0000 Subject: [PATCH 22/38] Thanks clippy --- tests/it/src/commands/env.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/tests/it/src/commands/env.rs b/tests/it/src/commands/env.rs index ef610c341de..ed0cf5a9909 100644 --- a/tests/it/src/commands/env.rs +++ b/tests/it/src/commands/env.rs @@ -9,7 +9,6 @@ pub(super) mod function { fn repr(text: &std::ffi::OsStr) -> String { text.to_str() .filter(|s| !s.chars().any(|c| c == '"' || c == '\n')) - .map(ToOwned::to_owned) - .unwrap_or_else(|| format!("{text:?}")) + .map_or_else(|| format!("{text:?}"), ToOwned::to_owned) } } From c40ead98728e645e74bfb7a9b75b1f1ad81a55a2 Mon Sep 17 00:00:00 2001 From: Eliah Kagan Date: Tue, 18 Mar 2025 13:13:35 -0400 Subject: [PATCH 23/38] Configure binfmt_misc for QEMU on CI As in: https://docs.gitlab.com/omnibus/development/s390x/ --- .github/workflows/ci.yml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index fab665ab119..a873c13e7e1 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -349,6 +349,8 @@ jobs: uses: taiki-e/install-action@v2 with: tool: cross + - name: Configure binfmt_misc for QEMU + run: docker run --rm --privileged multiarch/qemu-user-static --reset -p yes - name: Build cross image run: just cross-image ${{ matrix.target }} - name: Test (unit) From db8697b3888b367ccb3eff4d75a89ec9b52ad6ad Mon Sep 17 00:00:00 2001 From: Eliah Kagan Date: Sun, 9 Mar 2025 22:20:34 +0000 Subject: [PATCH 24/38] Start on trying to get an s390x `git` in the s390x cross container Currently this fails with: Some packages could not be installed. This may mean that you have requested an impossible situation or if you are using the unstable distribution that some required packages have not yet been created or been moved out of Incoming. The following information may help to resolve the situation: The following packages have unmet dependencies: git:s390x : Depends: perl:s390x but it is not going to be installed Depends: liberror-perl:s390x but it is not installable Or, in some variations, the (same) error is expressed as: E: Package 'liberror-perl:s390x' has no installation candidate --- etc/docker/Dockerfile.test-cross | 19 ++++++++++++++----- 1 file changed, 14 insertions(+), 5 deletions(-) diff --git a/etc/docker/Dockerfile.test-cross b/etc/docker/Dockerfile.test-cross index 83ae0c3e3f6..d81c5def254 100644 --- a/etc/docker/Dockerfile.test-cross +++ b/etc/docker/Dockerfile.test-cross @@ -2,7 +2,15 @@ ARG TARGET FROM ghcr.io/cross-rs/${TARGET}:latest -RUN apt-get update && \ +ARG TARGET + +RUN set -uC && \ + apt_suffix= && \ + if target_arch="$(dpkg-architecture --host-type "$TARGET" --query DEB_HOST_ARCH)"; then \ + dpkg --add-architecture "$target_arch" && \ + apt_suffix=":$target_arch"; \ + fi && \ + apt-get update && \ apt-get install --no-install-recommends -y apt-transport-https gnupg && \ release="$(sed -n 's/^VERSION_CODENAME=//p' /etc/os-release)" && \ echo "deb https://ppa.launchpadcontent.net/git-core/ppa/ubuntu $release main" \ @@ -10,9 +18,10 @@ RUN apt-get update && \ apt-key adv --keyserver keyserver.ubuntu.com \ --recv-keys F911AB184317630C59970973E363C90F8F1B6217 && \ apt-get update && \ - apt-get install --no-install-recommends -y git jq patch && \ + apt-get install --no-install-recommends -y "git$apt_suffix" jq patch && \ rm -rf /var/lib/apt/lists/* && \ - to_patch=/android-runner && \ - patcher='s/^export LD_PRELOAD=/test "${NO_PRELOAD_CXX:-0}" != 0 || &/' && \ - if test -f "$to_patch"; then sed -i.orig "$patcher" -- "$to_patch"; fi && \ + if test -f /android-runner; then \ + sed -i.orig 's/^export LD_PRELOAD=/test "${NO_PRELOAD_CXX:-0}" != 0 || &/' \ + /android-runner; \ + fi && \ git config --system gitoxide.imaginary.arbitraryVariable arbitraryValue From 154b1a65573ff3d6329b8fa20224c2d29f143465 Mon Sep 17 00:00:00 2001 From: Eliah Kagan Date: Mon, 10 Mar 2025 09:27:49 +0000 Subject: [PATCH 25/38] Try to enable ports APT sources unconditionally This does not work as written, at least on s390x, because the ports list is already written there from the base image. The existing file is actually better than what this writes, though what this writes is simpler and so might potentially be easier to debug or reason about. Unfortunately, if we clobber the existing file with this one, the error installing `git:s390x` is unchanged. Uninstalling the old `amd64` `git` package first (not shown here) and packages installed as its dependencies doesn't help either. --- etc/docker/Dockerfile.test-cross | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/etc/docker/Dockerfile.test-cross b/etc/docker/Dockerfile.test-cross index d81c5def254..9b5383bc413 100644 --- a/etc/docker/Dockerfile.test-cross +++ b/etc/docker/Dockerfile.test-cross @@ -5,14 +5,17 @@ FROM ghcr.io/cross-rs/${TARGET}:latest ARG TARGET RUN set -uC && \ + apt-get update && \ + apt-get install --no-install-recommends -y apt-transport-https gnupg && \ apt_suffix= && \ if target_arch="$(dpkg-architecture --host-type "$TARGET" --query DEB_HOST_ARCH)"; then \ dpkg --add-architecture "$target_arch" && \ apt_suffix=":$target_arch"; \ fi && \ - apt-get update && \ - apt-get install --no-install-recommends -y apt-transport-https gnupg && \ release="$(sed -n 's/^VERSION_CODENAME=//p' /etc/os-release)" && \ + for pocket in "$release" "$release-security" "$release-updates" "$release-backports"; do \ + echo "deb http://ports.ubuntu.com/ubuntu-ports/ $pocket main universe"; \ + done >/etc/apt/sources.list.d/ports.list && \ echo "deb https://ppa.launchpadcontent.net/git-core/ppa/ubuntu $release main" \ >/etc/apt/sources.list.d/git-core-ubuntu-ppa.list && \ apt-key adv --keyserver keyserver.ubuntu.com \ From ec63b31ba2dcc3a3e3432a261f260a138294d482 Mon Sep 17 00:00:00 2001 From: Eliah Kagan Date: Mon, 10 Mar 2025 10:24:33 +0000 Subject: [PATCH 26/38] Show that the problem still happens with `add-apt-repository` Thus, the lightweight manual approach used before isn't the cause. --- etc/docker/Dockerfile.test-cross | 15 +++++---------- 1 file changed, 5 insertions(+), 10 deletions(-) diff --git a/etc/docker/Dockerfile.test-cross b/etc/docker/Dockerfile.test-cross index 9b5383bc413..7e65f58cd37 100644 --- a/etc/docker/Dockerfile.test-cross +++ b/etc/docker/Dockerfile.test-cross @@ -5,23 +5,18 @@ FROM ghcr.io/cross-rs/${TARGET}:latest ARG TARGET RUN set -uC && \ - apt-get update && \ - apt-get install --no-install-recommends -y apt-transport-https gnupg && \ apt_suffix= && \ if target_arch="$(dpkg-architecture --host-type "$TARGET" --query DEB_HOST_ARCH)"; then \ dpkg --add-architecture "$target_arch" && \ apt_suffix=":$target_arch"; \ fi && \ - release="$(sed -n 's/^VERSION_CODENAME=//p' /etc/os-release)" && \ - for pocket in "$release" "$release-security" "$release-updates" "$release-backports"; do \ - echo "deb http://ports.ubuntu.com/ubuntu-ports/ $pocket main universe"; \ - done >/etc/apt/sources.list.d/ports.list && \ - echo "deb https://ppa.launchpadcontent.net/git-core/ppa/ubuntu $release main" \ - >/etc/apt/sources.list.d/git-core-ubuntu-ppa.list && \ - apt-key adv --keyserver keyserver.ubuntu.com \ - --recv-keys F911AB184317630C59970973E363C90F8F1B6217 && \ + apt-get update && \ + apt-get install --no-install-recommends -y software-properties-common && \ + add-apt-repository -y ppa:git-core/ppa && \ apt-get update && \ apt-get install --no-install-recommends -y "git$apt_suffix" jq patch && \ + apt-get purge --autoremove software-properties-common && \ + apt-get clean && \ rm -rf /var/lib/apt/lists/* && \ if test -f /android-runner; then \ sed -i.orig 's/^export LD_PRELOAD=/test "${NO_PRELOAD_CXX:-0}" != 0 || &/' \ From a3fed7707ede3469cd89eadff508c5cd2d86deff Mon Sep 17 00:00:00 2001 From: Eliah Kagan Date: Mon, 10 Mar 2025 20:29:05 +0000 Subject: [PATCH 27/38] Convert the problem to failure to install perl:s390x The following packages have unmet dependencies: perl : Conflicts: perl:s390x but 5.22.1-9ubuntu0.9 is to be installed perl:s390x : Depends: perl-base:s390x (= 5.22.1-9ubuntu0.9) but it is not going to be installed Depends: libperl5.22:s390x (= 5.22.1-9ubuntu0.9) but it is not going to be installed Conflicts: perl but 5.22.1-9ubuntu0.9 is to be installed --- etc/docker/Dockerfile.test-cross | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/etc/docker/Dockerfile.test-cross b/etc/docker/Dockerfile.test-cross index 7e65f58cd37..190e0ababba 100644 --- a/etc/docker/Dockerfile.test-cross +++ b/etc/docker/Dockerfile.test-cross @@ -14,7 +14,11 @@ RUN set -uC && \ apt-get install --no-install-recommends -y software-properties-common && \ add-apt-repository -y ppa:git-core/ppa && \ apt-get update && \ - apt-get install --no-install-recommends -y "git$apt_suffix" jq patch && \ + apt-get install --no-install-recommends -y "libc6$apt_suffix" "libcurl3-gnutls$apt_suffix" \ + "libexpat1$apt_suffix" "libpcre2-8-0$apt_suffix" "zlib1g$apt_suffix" "perl$apt_suffix" \ + liberror-perl git-man patch jq && \ + apt-get download "git$apt_suffix" && \ + dpkg --ignore-depends="liberror-perl$apt_suffix" -i ./git[-_]*.deb && \ apt-get purge --autoremove software-properties-common && \ apt-get clean && \ rm -rf /var/lib/apt/lists/* && \ From f0773eecd4798eda2eb3e7f1348afd67cc09e8c7 Mon Sep 17 00:00:00 2001 From: Eliah Kagan Date: Mon, 10 Mar 2025 20:44:35 +0000 Subject: [PATCH 28/38] Install host-arch `perl` deps, force target arch `git` to use them This does cause `git:s390x` to install in the s390x cross container but the changes are not finished -- container creation still fails due to a subsequent attempt to fix missing packages. Assuming `git` can use `perl` from a different architecture, which seems likely since it is invoking it as an interpreter (rather than linking to it), this should be fine, but to limit container bloat some other changes are needed to make it so we don't need the `autoremove` operation to get rid of files only used in setting up the PPA. --- etc/docker/Dockerfile.test-cross | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/etc/docker/Dockerfile.test-cross b/etc/docker/Dockerfile.test-cross index 190e0ababba..e0323ca1efe 100644 --- a/etc/docker/Dockerfile.test-cross +++ b/etc/docker/Dockerfile.test-cross @@ -15,10 +15,12 @@ RUN set -uC && \ add-apt-repository -y ppa:git-core/ppa && \ apt-get update && \ apt-get install --no-install-recommends -y "libc6$apt_suffix" "libcurl3-gnutls$apt_suffix" \ - "libexpat1$apt_suffix" "libpcre2-8-0$apt_suffix" "zlib1g$apt_suffix" "perl$apt_suffix" \ - liberror-perl git-man patch jq && \ + "libexpat1$apt_suffix" "libpcre2-8-0$apt_suffix" "zlib1g$apt_suffix" perl liberror-perl \ + git-man patch jq && \ apt-get download "git$apt_suffix" && \ - dpkg --ignore-depends="liberror-perl$apt_suffix" -i ./git[-_]*.deb && \ + dpkg --ignore-depends="perl$apt_suffix,liberror-perl$apt_suffix" -i git[-_]*.deb && \ + echo INSTALLED/UPGRADED GIT && \ + rm git[-_]*.deb && \ apt-get purge --autoremove software-properties-common && \ apt-get clean && \ rm -rf /var/lib/apt/lists/* && \ From 9be7e99e79c4f2e9813a932aabdd67fa137d2b3a Mon Sep 17 00:00:00 2001 From: Eliah Kagan Date: Mon, 10 Mar 2025 21:32:23 +0000 Subject: [PATCH 29/38] Manually set up PPA + other fixups This avoids installing `software-properties-common` in the container, instead manually setting up the PPA again. Accordingly, it is not necessary to uninstall any packages to avoid high bloat in the image, so that is removed. Other cleanup is still done. This also removes the old `git` and packages that were marked as automatically installed that were only needed as its dependencies, and shows details about the `git` that is installed. For s390x, this works to cause the test suite to use a s390x build of `git`, at a reasonably new version as provided by the PPA. That should happen for some other targets, but not all. This therefore shows the `git` version as well as examining the executable to report its architecture while the container is being built. Note that this does not yet appear sufficient to produce the s390x test failures mentioned in #1687 and discused in #1698 comments. (That might relate to how this is still just testing `max-pure`.) --- etc/docker/Dockerfile.test-cross | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/etc/docker/Dockerfile.test-cross b/etc/docker/Dockerfile.test-cross index e0323ca1efe..704a18529c4 100644 --- a/etc/docker/Dockerfile.test-cross +++ b/etc/docker/Dockerfile.test-cross @@ -11,17 +11,23 @@ RUN set -uC && \ apt_suffix=":$target_arch"; \ fi && \ apt-get update && \ - apt-get install --no-install-recommends -y software-properties-common && \ - add-apt-repository -y ppa:git-core/ppa && \ + apt-get install --no-install-recommends -y apt-transport-https gnupg && \ + release="$(sed -n 's/^VERSION_CODENAME=//p' /etc/os-release)" && \ + echo "deb https://ppa.launchpadcontent.net/git-core/ppa/ubuntu $release main" \ + >/etc/apt/sources.list.d/git-core-ubuntu-ppa.list && \ + apt-key adv --keyserver keyserver.ubuntu.com \ + --recv-keys F911AB184317630C59970973E363C90F8F1B6217 && \ apt-get update && \ + apt-get purge --autoremove -y git && \ apt-get install --no-install-recommends -y "libc6$apt_suffix" "libcurl3-gnutls$apt_suffix" \ "libexpat1$apt_suffix" "libpcre2-8-0$apt_suffix" "zlib1g$apt_suffix" perl liberror-perl \ - git-man patch jq && \ + git-man file jq patch && \ apt-get download "git$apt_suffix" && \ dpkg --ignore-depends="perl$apt_suffix,liberror-perl$apt_suffix" -i git[-_]*.deb && \ - echo INSTALLED/UPGRADED GIT && \ + git version --build-options && \ + git="$(command -v git)" && \ + file -- "$git" && \ rm git[-_]*.deb && \ - apt-get purge --autoremove software-properties-common && \ apt-get clean && \ rm -rf /var/lib/apt/lists/* && \ if test -f /android-runner; then \ From 6e7122f4d8ce28e269ecb89452fce9a20b73df69 Mon Sep 17 00:00:00 2001 From: Eliah Kagan Date: Tue, 11 Mar 2025 01:21:30 +0000 Subject: [PATCH 30/38] Make sure we actually have `dpkg-architecture` It is provided by `dpkg-dev`, which is installed already in some but not all `cross` images. This commit also: - Allows more configuration to occur by installing `apt-utils`. Without this, `gnupg` warns about skipping some debconf configuration. - Sets up the environment in which commands like `apt-get` and `dpkg` run so configuring packages never prompts. Passing `-y` is not sufficient to achieve this. (The problem currently does not occur, but it would happen in at least the Android image if a new dependency brings in `tzdata` indirectly, as will occur if attempting to install dependencies to test more than `max-pure`.) --- etc/docker/Dockerfile.test-cross | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/etc/docker/Dockerfile.test-cross b/etc/docker/Dockerfile.test-cross index 704a18529c4..60a29d10cf1 100644 --- a/etc/docker/Dockerfile.test-cross +++ b/etc/docker/Dockerfile.test-cross @@ -5,13 +5,16 @@ FROM ghcr.io/cross-rs/${TARGET}:latest ARG TARGET RUN set -uC && \ + export DEBIAN_FRONTEND=noninteractive TZ=UTC && \ + apt-get update && \ + apt-get install --no-install-recommends -y apt-utils && \ + apt-get install --no-install-recommends -y apt-transport-https dpkg-dev gnupg && \ + type dpkg-architecture && \ apt_suffix= && \ if target_arch="$(dpkg-architecture --host-type "$TARGET" --query DEB_HOST_ARCH)"; then \ dpkg --add-architecture "$target_arch" && \ apt_suffix=":$target_arch"; \ fi && \ - apt-get update && \ - apt-get install --no-install-recommends -y apt-transport-https gnupg && \ release="$(sed -n 's/^VERSION_CODENAME=//p' /etc/os-release)" && \ echo "deb https://ppa.launchpadcontent.net/git-core/ppa/ubuntu $release main" \ >/etc/apt/sources.list.d/git-core-ubuntu-ppa.list && \ From 1df389496b0479732e81336aaad64c6fd687a50e Mon Sep 17 00:00:00 2001 From: Eliah Kagan Date: Tue, 11 Mar 2025 01:42:54 +0000 Subject: [PATCH 31/38] Let `apt-get download` drop privileges for the download + Adjust whitespace style by letting short closely related commands appear on the same line. Broadly speaking, this style is often not clearer, but in this case, especially with the new commands here, it seems to improve clarity. (Though maybe not as much as breaking the `RUN` contents out into a script file, where they could instead be easily grouped with whitespace and comments.) --- etc/docker/Dockerfile.test-cross | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/etc/docker/Dockerfile.test-cross b/etc/docker/Dockerfile.test-cross index 60a29d10cf1..6dd9ba1189f 100644 --- a/etc/docker/Dockerfile.test-cross +++ b/etc/docker/Dockerfile.test-cross @@ -25,14 +25,11 @@ RUN set -uC && \ apt-get install --no-install-recommends -y "libc6$apt_suffix" "libcurl3-gnutls$apt_suffix" \ "libexpat1$apt_suffix" "libpcre2-8-0$apt_suffix" "zlib1g$apt_suffix" perl liberror-perl \ git-man file jq patch && \ - apt-get download "git$apt_suffix" && \ - dpkg --ignore-depends="perl$apt_suffix,liberror-perl$apt_suffix" -i git[-_]*.deb && \ - git version --build-options && \ - git="$(command -v git)" && \ - file -- "$git" && \ - rm git[-_]*.deb && \ - apt-get clean && \ - rm -rf /var/lib/apt/lists/* && \ + mkdir /tmp/dl && chown _apt /tmp/dl && \ + (cd /tmp/dl && apt-get download "git$apt_suffix") && \ + dpkg --ignore-depends="perl$apt_suffix,liberror-perl$apt_suffix" -i /tmp/dl/git[-_]*.deb && \ + git version --build-options && git="$(command -v git)" && file -- "$git" && \ + apt-get clean && rm -rf /tmp/dl /var/lib/apt/lists/* && \ if test -f /android-runner; then \ sed -i.orig 's/^export LD_PRELOAD=/test "${NO_PRELOAD_CXX:-0}" != 0 || &/' \ /android-runner; \ From 6e3e0686df2a0dec414bc89a60015ade091475db Mon Sep 17 00:00:00 2001 From: Eliah Kagan Date: Tue, 11 Mar 2025 03:37:47 +0000 Subject: [PATCH 32/38] Test max on s390x (and potentially elsewhere supported) This modifies the `cross` dockerfile and associated `justfile` recipes to install dependencies needed for testing beyond max-pure. This works when installing packages for the cross target is already working, which is the same scenario where the upgraded `git` (from the git-core PPA) is installed for the `cross` target. But unlike `git`, when testing max-pure it is not necessary to install these libraries. Yet the current approach does install them... but native builds of them. This is inefficient in the Android test where there may be no immediately forthcoming way to install appropriate `cross`-target builds of the libraries. This extra work should probably be eliminated. But the best way to do that will depend on whether the two cases of cross targets (those we can install packages for and those we cannot) remain only two cases, and whether a single `RUN` script continues to be used, versus splitting it (or creating multiple `cross` dockerfiles). This also gives the `cross-test` recipe a `*cargo-test-args` parameter, and moves `--no-default-features --features max-pure` out of `cross-test` and into the arguments `cross-test-android` passes to `cross-test` (causing `*cargo-test-args` to receive it). The reason for doing this rather than having a parameter that is put in place of `max-pure`, for which `cross-test-s390x` could pass `max`, is that it is not obvious that `--no-default-features` is free of undesired effects when `--workspace` is used: it seems like it may suppress some features of some tested `gix-*` crate projects that do not individually recognize `max`. (This differs from the situation with the journey tests, which test the `ein` and `gix` executables from by the `gitoxide` crate, which recognizes `max-pure`.) The intuitive effect of "testing max" is currently to run the entire test suite with all default features enabled, so this strives to do that. This still does not appear sufficient to produce the s390x test failures mentioned in #1687 and discused in #1698 comments, even though this is now running s390x tests with the same full default features as were used when those failures were observed. --- etc/docker/Dockerfile.test-cross | 3 ++- justfile | 12 ++++++------ 2 files changed, 8 insertions(+), 7 deletions(-) diff --git a/etc/docker/Dockerfile.test-cross b/etc/docker/Dockerfile.test-cross index 6dd9ba1189f..8fd1b3fc102 100644 --- a/etc/docker/Dockerfile.test-cross +++ b/etc/docker/Dockerfile.test-cross @@ -24,7 +24,8 @@ RUN set -uC && \ apt-get purge --autoremove -y git && \ apt-get install --no-install-recommends -y "libc6$apt_suffix" "libcurl3-gnutls$apt_suffix" \ "libexpat1$apt_suffix" "libpcre2-8-0$apt_suffix" "zlib1g$apt_suffix" perl liberror-perl \ - git-man file jq patch && \ + git-man ca-certificates cmake "curl$apt_suffix" file jq "libc-dev$apt_suffix" \ + "libssl-dev$apt_suffix" patch pkgconf && \ mkdir /tmp/dl && chown _apt /tmp/dl && \ (cd /tmp/dl && apt-get download "git$apt_suffix") && \ dpkg --ignore-depends="perl$apt_suffix,liberror-perl$apt_suffix" -i /tmp/dl/git[-_]*.deb && \ diff --git a/justfile b/justfile index ce5ba3666d9..86cf6cf9a26 100755 --- a/justfile +++ b/justfile @@ -230,17 +230,17 @@ cross-image target: - Date: Tue, 11 Mar 2025 09:51:54 +0000 Subject: [PATCH 33/38] Start toward moving the dense `RUN` to a commented script --- etc/docker/customize-test-cross-image.sh | 112 +++++++++++++++++++++++ 1 file changed, 112 insertions(+) create mode 100755 etc/docker/customize-test-cross-image.sh diff --git a/etc/docker/customize-test-cross-image.sh b/etc/docker/customize-test-cross-image.sh new file mode 100755 index 00000000000..52a0ded0c30 --- /dev/null +++ b/etc/docker/customize-test-cross-image.sh @@ -0,0 +1,112 @@ +#!/bin/bash +set -euxC + +target="$1" + +# Arrange for the indirect `tzdata` dependency to be installed and configured +# without prompting for the time zone. (Passing `-y` is not enough.) +export DEBIAN_FRONTEND=noninteractive TZ=UTC + +# Install tools for setting up APT repositores. Install `apt-utils` before the +# others, so the installation of `gnupg` can use it for debconf. +apt-get update +apt-get install --no-install-recommends -y apt-utils +apt-get install --no-install-recommends -y apt-transport-https dpkg-dev gnupg +type dpkg-architecture # Make sure we really have this. + +# Decide what architecture to use for `git`, shared libraries for gitoxide when +# attempting to build `max`, and shared libraries used by `git` itself. +apt_suffix= +if target_arch="$(dpkg-architecture --host-type "$target" --query DEB_HOST_ARCH)"; then + dpkg --add-architecture "$target_arch" + apt_suffix=":$target_arch" +fi + +# Add the git-core PPA manually. (Faster than installing `add-apt-repository`.) +release="$(sed -n 's/^VERSION_CODENAME=//p' /etc/os-release)" +echo "deb https://ppa.launchpadcontent.net/git-core/ppa/ubuntu $release main" \ + >/etc/apt/sources.list.d/git-core-ubuntu-ppa.list +apt-key adv --keyserver keyserver.ubuntu.com \ + --recv-keys F911AB184317630C59970973E363C90F8F1B6217 +apt-get update + +# Remove the old `git` and associated packages. +apt-get purge --autoremove -y git + +# Git dependencies. These are for the desired architecture, except `git-man` is +# the same package for all architectures, and we can't always install `perl` or +# `liberror-perl` for the desired architecture (at least in s390x). +# TODO(maint): Resolve these dynamically to support future `cross` base images. +git_deps=( + git-man + "libc6$apt_suffix" + "libcurl3-gnutls$apt_suffix" + "libexpat1$apt_suffix" + liberror-perl + "libpcre2-8-0$apt_suffix" + "zlib1g$apt_suffix" + perl +) + +# Other dependencies for running the gitoxide test suite and fixture scripts, +# and for building and testing gitoxide for feature sets beyond `max-pure`. +gix_test_deps=( + ca-certificates + cmake + "curl$apt_suffix" + jq + "libc-dev$apt_suffix" + "libssl-dev$apt_suffix" + patch + pkgconf +) + +# Install everything we need except `git` (and what we already have). We can't +# necessarily install `git` this way, because it insists on `perl` and +# `liberror-perl` dependencies of the same architecture as it. These may not be +# possible to install in a mixed environment, where most packages are a +# different architecture, and where `perl` is a dependency of other important +# packages. So we will install everything else first (then manually add `git`). +apt-get install --no-install-recommends -y \ + "${git_deps[@]}" "${gix_test_deps[@]}" file + +# Add `git` by manually downloading it and installing it with `dpkg`, forcing +# installation to proceed even if its `perl` and `liberror-perl` dependencies, +# as declared by `git`, are absent. (We have already installed them, but in a +# possibly different architecture. `git` can still use them, because its use is +# to run scripts, rather than to link to a shared library they provide.) It is +# preferred to let `apt-get download` drop privileges to the `_apt` user during +# download, so we download it inside `/tmp`. But we create a subdirectory so it +# is safe to make assumptions about what files globs can expand to, even if +# `/tmp` is mounted to an outside share temp dir on a multi-user system. +mkdir /tmp/dl # Don't use `-p`; if it exists already, we cannot trust it. +chown _apt /tmp/dl # Use owner, as the container may not have an `_apt` group. +(cd /tmp/dl && apt-get download "git$apt_suffix") +dpkg --ignore-depends="perl$apt_suffix,liberror-perl$apt_suffix" \ + -i /tmp/dl/git[-_]*.deb + +# Show information about the newly installed `git` (and ensure it can run). +git version --build-options +git="$(command -v git)" +file -- "$git" + +# Clean up files related to package management that we won't need anymore. +apt-get clean +rm -rf /tmp/dl /var/lib/apt/lists/* + +# If this is an Android-related image or otherwise has a runner script `cross` +# uses for Android, then patch the script to add the ability to suppress its +# customization of `LD_PRELOAD`. This runner script sets `LD_PRELOAD` to the +# path of `libc++_shared.so` in the vendored Android NDK. But this causes a +# problem for us because, when a non-Android (i.e. a host-architecture) program +# is run, `ld.so` shows a message about the "wrong ELF class". Such programs +# can still run, but when we make an assertion about, parse, or otherwise rely +# on their output to standard error, we get test failures. (That especially +# affects fixtures.) This change lets us pass `NO_PRELOAD_CXX=1` to avoid that. +if test -f /android-runner; then + sed -i.orig 's/^export LD_PRELOAD=/test "${NO_PRELOAD_CXX:-0}" != 0 || &/' + /android-runner +fi + +# Ensure a nonempty Git `system` scope (for the `installation_config` tests). +git config --system gitoxide.imaginary.arbitraryVariable arbitraryValue From c3ea44bf3a768d3c11583daf4e24f8bbfa074198 Mon Sep 17 00:00:00 2001 From: Eliah Kagan Date: Wed, 12 Mar 2025 08:33:04 +0000 Subject: [PATCH 34/38] Finish moving the dense `RUN` to a commented script - Clarify and expand comments. - Adjust style for greater consistency within the script. - Have the script tolerate leading whitespace when it patches the Android runner script, since it looks like future releases of `cross` may have it undented under an `if` check for the Android version (not all versions have a C++ library in the NDK). - Move the script into a subdirectory that will be used as context. - Replace the old `RUN` step with a command to run the script. - Modify the build command in the `justfile` to use the context. --- etc/docker/Dockerfile.test-cross | 37 ++------------- .../customize.sh} | 47 ++++++++++++------- justfile | 5 +- 3 files changed, 36 insertions(+), 53 deletions(-) rename etc/docker/{customize-test-cross-image.sh => test-cross-context/customize.sh} (69%) diff --git a/etc/docker/Dockerfile.test-cross b/etc/docker/Dockerfile.test-cross index 8fd1b3fc102..7d420c754d0 100644 --- a/etc/docker/Dockerfile.test-cross +++ b/etc/docker/Dockerfile.test-cross @@ -1,38 +1,7 @@ ARG TARGET - FROM ghcr.io/cross-rs/${TARGET}:latest ARG TARGET - -RUN set -uC && \ - export DEBIAN_FRONTEND=noninteractive TZ=UTC && \ - apt-get update && \ - apt-get install --no-install-recommends -y apt-utils && \ - apt-get install --no-install-recommends -y apt-transport-https dpkg-dev gnupg && \ - type dpkg-architecture && \ - apt_suffix= && \ - if target_arch="$(dpkg-architecture --host-type "$TARGET" --query DEB_HOST_ARCH)"; then \ - dpkg --add-architecture "$target_arch" && \ - apt_suffix=":$target_arch"; \ - fi && \ - release="$(sed -n 's/^VERSION_CODENAME=//p' /etc/os-release)" && \ - echo "deb https://ppa.launchpadcontent.net/git-core/ppa/ubuntu $release main" \ - >/etc/apt/sources.list.d/git-core-ubuntu-ppa.list && \ - apt-key adv --keyserver keyserver.ubuntu.com \ - --recv-keys F911AB184317630C59970973E363C90F8F1B6217 && \ - apt-get update && \ - apt-get purge --autoremove -y git && \ - apt-get install --no-install-recommends -y "libc6$apt_suffix" "libcurl3-gnutls$apt_suffix" \ - "libexpat1$apt_suffix" "libpcre2-8-0$apt_suffix" "zlib1g$apt_suffix" perl liberror-perl \ - git-man ca-certificates cmake "curl$apt_suffix" file jq "libc-dev$apt_suffix" \ - "libssl-dev$apt_suffix" patch pkgconf && \ - mkdir /tmp/dl && chown _apt /tmp/dl && \ - (cd /tmp/dl && apt-get download "git$apt_suffix") && \ - dpkg --ignore-depends="perl$apt_suffix,liberror-perl$apt_suffix" -i /tmp/dl/git[-_]*.deb && \ - git version --build-options && git="$(command -v git)" && file -- "$git" && \ - apt-get clean && rm -rf /tmp/dl /var/lib/apt/lists/* && \ - if test -f /android-runner; then \ - sed -i.orig 's/^export LD_PRELOAD=/test "${NO_PRELOAD_CXX:-0}" != 0 || &/' \ - /android-runner; \ - fi && \ - git config --system gitoxide.imaginary.arbitraryVariable arbitraryValue +COPY customize.sh /usr/local/bin/ +RUN chmod +x /usr/local/bin/customize.sh && \ + /usr/local/bin/customize.sh "$TARGET" diff --git a/etc/docker/customize-test-cross-image.sh b/etc/docker/test-cross-context/customize.sh similarity index 69% rename from etc/docker/customize-test-cross-image.sh rename to etc/docker/test-cross-context/customize.sh index 52a0ded0c30..31396e3ee2e 100755 --- a/etc/docker/customize-test-cross-image.sh +++ b/etc/docker/test-cross-context/customize.sh @@ -2,6 +2,7 @@ set -euxC target="$1" +test -n "$target" # Arrange for the indirect `tzdata` dependency to be installed and configured # without prompting for the time zone. (Passing `-y` is not enough.) @@ -14,16 +15,30 @@ apt-get install --no-install-recommends -y apt-utils apt-get install --no-install-recommends -y apt-transport-https dpkg-dev gnupg type dpkg-architecture # Make sure we really have this. -# Decide what architecture to use for `git`, shared libraries for gitoxide when -# attempting to build `max`, and shared libraries used by `git` itself. +# Decide what architecture to use for `git`, shared libraries `git` links to, +# and shared libraries gitoxide links to when building `max`. Instead of this +# custom logic, we could use `$CROSS_DEB_ARCH`, which `cross` tries to provide +# (https://github.com/cross-rs/cross/blob/v0.2.5/src/lib.rs#L268), and which is +# available for roughly the same architectures where this logic gets a nonempty +# value. But using `$CROSS_DEB_ARCH` may make it harder to build and test the +# image manually. In particular, if it is not passed, we would conclude that we +# should install the versions of those packages with the host's architecture. apt_suffix= -if target_arch="$(dpkg-architecture --host-type "$target" --query DEB_HOST_ARCH)"; then +if target_arch="$(dpkg-architecture --host-type "$target" --query DEB_HOST_ARCH)" +then dpkg --add-architecture "$target_arch" apt_suffix=":$target_arch" + printf 'INFO: Using target architecture for `git` and libs in container.\n' + printf 'INFO: This architecture is %s.\n' "$target_arch" +else + apt_suffix='' + printf 'WARNING: Using HOST architecture for `git` and libs in container.\n' fi -# Add the git-core PPA manually. (Faster than installing `add-apt-repository`.) +# Get release codename. Like `lsb_release -sc`. (`lsb_release` may be absent.) release="$(sed -n 's/^VERSION_CODENAME=//p' /etc/os-release)" + +# Add the git-core PPA manually. (Faster than installing `add-apt-repository`.) echo "deb https://ppa.launchpadcontent.net/git-core/ppa/ubuntu $release main" \ >/etc/apt/sources.list.d/git-core-ubuntu-ppa.list apt-key adv --keyserver keyserver.ubuntu.com \ @@ -94,19 +109,17 @@ file -- "$git" apt-get clean rm -rf /tmp/dl /var/lib/apt/lists/* -# If this is an Android-related image or otherwise has a runner script `cross` -# uses for Android, then patch the script to add the ability to suppress its -# customization of `LD_PRELOAD`. This runner script sets `LD_PRELOAD` to the -# path of `libc++_shared.so` in the vendored Android NDK. But this causes a -# problem for us because, when a non-Android (i.e. a host-architecture) program -# is run, `ld.so` shows a message about the "wrong ELF class". Such programs -# can still run, but when we make an assertion about, parse, or otherwise rely -# on their output to standard error, we get test failures. (That especially -# affects fixtures.) This change lets us pass `NO_PRELOAD_CXX=1` to avoid that. -if test -f /android-runner; then - sed -i.orig 's/^export LD_PRELOAD=/test "${NO_PRELOAD_CXX:-0}" != 0 || &/' - /android-runner -fi +# If this image has a runner script `cross` uses for Android, patch the script +# to add the ability to suppress its customization of `LD_PRELOAD`. The runner +# script sets `LD_PRELOAD` to the path of `libc++_shared.so` in the Android NDK +# (https://github.com/cross-rs/cross/blob/v0.2.5/docker/android-runner#L34). +# But this causes a problem for us. When a host-architecture program is run, +# `ld.so` shows a message about the "wrong ELF class". Such programs can still +# run, but when we rely on their specific output to stderr, fixtures and tests +# fail. The change we make here lets us set `NO_PRELOAD_CXX=1` to avoid that. +runner=/android-runner +patch='s/^[[:blank:]]*export LD_PRELOAD=/test "${NO_PRELOAD_CXX:-0}" != 0 || &/' +if test -f "$runner"; then sed -i.orig "$patch" -- "$runner"; fi # Ensure a nonempty Git `system` scope (for the `installation_config` tests). git config --system gitoxide.imaginary.arbitraryVariable arbitraryValue diff --git a/justfile b/justfile index 86cf6cf9a26..7fcc4da5b86 100755 --- a/justfile +++ b/justfile @@ -226,8 +226,9 @@ journey-tests-async: dbg # Build a customized `cross` container image for testing cross-image target: - docker build --build-arg "TARGET={{ target }}" -t "cross-rs-gitoxide:{{ target }}" \ - - Date: Wed, 19 Mar 2025 15:21:51 +0000 Subject: [PATCH 35/38] Only install `git` strangely when there is reason to do so We install `git` in the `cross` container, intending that it come from the git-core PPA and thus be of a recent version. If we can find a Debian-style architecture name that is correct for the `cross` target triple, then we install it for that architecture. In principle we might find such a name without the PPA having a build for that architecture, but that is in practice unlikely to occur and, if it does, then either the `git` we do manage to install is new enough, or we will get test failures due to absent features the test suite needs (such as `GIT_CONFIG_{COUNT,KEY,VALUE}` support). When we don't find such a name, we use the host architecture. When we install `git` for the target architecture, its `perl` and `liberror-perl` dependencies, which as declared must be of that architecture as well, may not be installable. We run into this problem in the container for `s390x-unknown-linux-gnu`. To solve it, we install the dependencies except for those, as well as ensuring those are installed for the host architecture, and then install `git` while forcing it install even if those two dependencies are considered unmet. This works because `git` doesn't use `perl` as a library, but rather as an interpreter for some scripts; and `libperl-error` is a library, but it is a Perl module used from Perl scripts, not a shared library object any Git-related binaries have to load. (Currently, direct dependencies of `git` are hard-coded, which, as commented, may be worth changing to decrease the likelihood of breakage in future versions.) In order to test that the procedure really has a chance of working on a variety of architectures (in case `cross` is to be used to test others at some point), this complex procedure for installing `git` is used whether it is being installed for the target or the host architecture. But this doesn't provide much assurance; that the procedure works when it is effectively just installing exactly the same packages as would be installed anyway does not imply that it works with other cross-target installations where it would be installing different packages. More importantly, using this even when it is known not to be needed obscures the cases where it is not needed, and may create the impression that test failures are due to the strange way `git` was installed, even when they are not. Therefore, this modifies the `cross` container customization script so we only take the strange semi-manual dependency installation approach when there is a chance it might actually be needed, i.e., when we are installing `git` for the cross target rather than the host target. --- etc/docker/test-cross-context/customize.sh | 54 +++++++++++++--------- 1 file changed, 31 insertions(+), 23 deletions(-) diff --git a/etc/docker/test-cross-context/customize.sh b/etc/docker/test-cross-context/customize.sh index 31396e3ee2e..a7dd5a4f79b 100755 --- a/etc/docker/test-cross-context/customize.sh +++ b/etc/docker/test-cross-context/customize.sh @@ -76,29 +76,37 @@ gix_test_deps=( pkgconf ) -# Install everything we need except `git` (and what we already have). We can't -# necessarily install `git` this way, because it insists on `perl` and -# `liberror-perl` dependencies of the same architecture as it. These may not be -# possible to install in a mixed environment, where most packages are a -# different architecture, and where `perl` is a dependency of other important -# packages. So we will install everything else first (then manually add `git`). -apt-get install --no-install-recommends -y \ - "${git_deps[@]}" "${gix_test_deps[@]}" file +if test -n "$apt_suffix"; then + # Install everything we need except `git` (and what we already have). We + # can't necessarily install `git` this way, because it insists on `perl` + # and `liberror-perl` dependencies of the same architecture as it. These + # may not be possible to install in a mixed environment, where most + # packages are a different architecture, and where `perl` is a dependency + # of other important packages. So we will install everything else first + # (then manually add `git`). + apt-get install --no-install-recommends -y \ + "${git_deps[@]}" "${gix_test_deps[@]}" file -# Add `git` by manually downloading it and installing it with `dpkg`, forcing -# installation to proceed even if its `perl` and `liberror-perl` dependencies, -# as declared by `git`, are absent. (We have already installed them, but in a -# possibly different architecture. `git` can still use them, because its use is -# to run scripts, rather than to link to a shared library they provide.) It is -# preferred to let `apt-get download` drop privileges to the `_apt` user during -# download, so we download it inside `/tmp`. But we create a subdirectory so it -# is safe to make assumptions about what files globs can expand to, even if -# `/tmp` is mounted to an outside share temp dir on a multi-user system. -mkdir /tmp/dl # Don't use `-p`; if it exists already, we cannot trust it. -chown _apt /tmp/dl # Use owner, as the container may not have an `_apt` group. -(cd /tmp/dl && apt-get download "git$apt_suffix") -dpkg --ignore-depends="perl$apt_suffix,liberror-perl$apt_suffix" \ - -i /tmp/dl/git[-_]*.deb + # Add `git` by manually downloading it and installing it with `dpkg`, + # forcing installation to proceed even if its `perl` and `liberror-perl` + # dependencies, as declared by `git`, are absent. (We have already + # installed them, but in a possibly different architecture. `git` can still + # use them, because its use is to run scripts, rather than to link to a + # shared library they provide.) It is preferred to let `apt-get download` + # drop privileges to the `_apt` user during download, so we download it + # inside `/tmp`. But we create a subdirectory so it is safe to make + # assumptions about what files globs can expand to, even if `/tmp` is + # mounted to an outside share temp dir on a multi-user system. + mkdir /tmp/dl # Don't use `-p`; if it exists already, we cannot trust it. + chown _apt /tmp/dl # Use owner, as the container may have no `_apt` group. + (cd /tmp/dl && apt-get download "git$apt_suffix") + dpkg --ignore-depends="perl$apt_suffix,liberror-perl$apt_suffix" \ + -i /tmp/dl/git[-_]*.deb + rm -r /tmp/dl +else + # Install everything we need, including `git`. + apt-get install --no-install-recommends -y git "${gix_test_deps[@]}" file +fi # Show information about the newly installed `git` (and ensure it can run). git version --build-options @@ -107,7 +115,7 @@ file -- "$git" # Clean up files related to package management that we won't need anymore. apt-get clean -rm -rf /tmp/dl /var/lib/apt/lists/* +rm -rf /var/lib/apt/lists/* # If this image has a runner script `cross` uses for Android, patch the script # to add the ability to suppress its customization of `LD_PRELOAD`. The runner From e88e6355957aae50576b1a8054edc45af4564719 Mon Sep 17 00:00:00 2001 From: Eliah Kagan Date: Wed, 19 Mar 2025 20:00:45 +0000 Subject: [PATCH 36/38] Make it easier to pass arguments to test executable So option arguments like `--test-threads=1` that must come after `--` can be passed through `just` to `cross`, while still being able to customize options like `--features` that go before it. --- justfile | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/justfile b/justfile index 7fcc4da5b86..138deaadd67 100755 --- a/justfile +++ b/justfile @@ -231,17 +231,17 @@ cross-image target: -f etc/docker/Dockerfile.test-cross etc/docker/test-cross-context # Test another platform with `cross` -cross-test target *cargo-test-args: (cross-image target) +cross-test target options test-options: (cross-image target) CROSS_CONFIG=etc/docker/test-cross.toml NO_PRELOAD_CXX=1 \ cross test --workspace --no-fail-fast --target {{ target }} \ - {{ cargo-test-args }} -- --skip realpath::fuzzed_timeout + {{ options }} -- --skip realpath::fuzzed_timeout {{ test-options }} # Test s390x with `cross` -cross-test-s390x: (cross-test 's390x-unknown-linux-gnu') +cross-test-s390x: (cross-test 's390x-unknown-linux-gnu' '' '') # Test Android with `cross` (max-pure) -cross-test-android: (cross-test 'armv7-linux-androideabi' \ - '--no-default-features' '--features' 'max-pure') +cross-test-android: (cross-test 'armv7-linux-androideabi' + '--no-default-features --features max-pure' '') # Run `cargo diet` on all crates to see that they are still in bounds check-size: From 803d0544dff9ce7a570f16f8146c2d5e522b4c21 Mon Sep 17 00:00:00 2001 From: Eliah Kagan Date: Wed, 19 Mar 2025 20:11:43 +0000 Subject: [PATCH 37/38] Adjust `cross` CI jobs for changed `just` recipe syntax Also, since `--test-threads 1` didn't reveal anything different locally, this uses the nicknamed convenience recipes. --- .github/workflows/ci.yml | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index a873c13e7e1..01f40f5187a 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -334,7 +334,12 @@ jobs: strategy: matrix: - target: [ armv7-linux-androideabi, s390x-unknown-linux-gnu ] + nickname: [ android, s390x ] + include: + - nickname: android + target: armv7-linux-androideabi + - nickname: s390x + target: s390x-unknown-linux-gnu fail-fast: false steps: @@ -356,7 +361,7 @@ jobs: - name: Test (unit) env: RUST_BACKTRACE: '1' - run: just cross-test ${{ matrix.target }} + run: just cross-test-${{ matrix.nickname }} lint: runs-on: ubuntu-latest From 73dcf57f9cf49bdc9d1745d39b5dd2d93609f3f5 Mon Sep 17 00:00:00 2001 From: Eliah Kagan Date: Mon, 7 Apr 2025 04:14:15 +0000 Subject: [PATCH 38/38] Remove `test-cross` CI job definition One or both of these matrix jobs, or related ones, be brought back some day, probably to run only specific tests that have failed in the past and started passing. The files for local testing with `cross` (and associated `justfile` recipes) are kept. --- .github/workflows/ci.yml | 37 ------------------------------------- 1 file changed, 37 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 01f40f5187a..f80297f737c 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -327,42 +327,6 @@ jobs: - name: Test (nextest) run: cargo nextest run --target $env:TARGET --workspace --no-fail-fast size - # FIXME: *If* this is kept, then remove `fail-fast: false`, run only a subset of the tests unless - # building and running the whole test suite is fast, and add a `Swatinem/rust-cache` step. - test-cross: - runs-on: ubuntu-latest - - strategy: - matrix: - nickname: [ android, s390x ] - include: - - nickname: android - target: armv7-linux-androideabi - - nickname: s390x - target: s390x-unknown-linux-gnu - fail-fast: false - - steps: - - uses: actions/checkout@v4 - - name: Install Rust - uses: dtolnay/rust-toolchain@master - with: - toolchain: stable - targets: ${{ matrix.target }} - - uses: extractions/setup-just@v3 - - name: Install cross - uses: taiki-e/install-action@v2 - with: - tool: cross - - name: Configure binfmt_misc for QEMU - run: docker run --rm --privileged multiarch/qemu-user-static --reset -p yes - - name: Build cross image - run: just cross-image ${{ matrix.target }} - - name: Test (unit) - env: - RUST_BACKTRACE: '1' - run: just cross-test-${{ matrix.nickname }} - lint: runs-on: ubuntu-latest @@ -520,7 +484,6 @@ jobs: env: # List all jobs that are intended NOT to block PR auto-merge here. EXPECTED_NONBLOCKING_JOBS: |- - test-cross cargo-deny-advisories wasm tests-pass