Skip to content
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

Replace codespell with typos #7248

Closed
wants to merge 9 commits into from
Closed

Conversation

szepeviktor
Copy link
Contributor

@szepeviktor szepeviktor commented Jan 10, 2024

From #7241

@szepeviktor szepeviktor changed the title Install typos for build Replace codespell with typos Jan 10, 2024
@szepeviktor
Copy link
Contributor Author

@aarongable Am I doing it right?

Copy link
Contributor

@aarongable aarongable left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, this is definitely on the right track!

When I build this docker image and then run typos, I get the following error:

typos: /lib/x86_64-linux-gnu/libc.so.6: version `GLIBC_2.32' not found (required by typos)
typos: /lib/x86_64-linux-gnu/libc.so.6: version `GLIBC_2.33' not found (required by typos)
typos: /lib/x86_64-linux-gnu/libc.so.6: version `GLIBC_2.34' not found (required by typos)

Looks like the base image we're copying the typos binary into doesn't have the right version of libc. If you fix that, this change will LGTM

.typos.toml Outdated Show resolved Hide resolved
.typos.toml Show resolved Hide resolved
@szepeviktor
Copy link
Contributor Author

szepeviktor commented Jan 10, 2024

Looks like the base image we're copying the typos binary into doesn't have the right version of libc. If you fix that, this change will LGTM

Added static linking!
crate-ci/typos#903 (comment)

@pgporada
Copy link
Member

pgporada commented Jan 17, 2024

I have a potential solution for this. See my changes to the Dockerfile and a new file used by the Dockerfile called build-rust-deps.sh.

What's happening here is the chosen rust container for the docker multipart build, rust:bookworm has a newer GLIBC than the container that we actually run commands from, letsencrypt/boulder-tools, which is based on Ubuntu Focal. This means we get the errors that @aarongable mentioned above.

$ docker run -it rust:bookworm bash
# dpkg -l | grep libc6
ii  libc6:amd64                        2.36-9+deb12u3                 amd64        GNU C Library: Shared libraries
ii  libc6-dev:amd64                    2.36-9+deb12u3                 amd64        GNU C Library: Development Libraries and Header Files

$ docker run -it letsencrypt/boulder-tools:go1.21.5_2024-01-17 bash
# dpkg -l | grep libc6
ii  libc6:amd64                2.31-0ubuntu9.14                  amd64        GNU C Library: Shared libraries
ii  libc6:amd64                2.31-0ubuntu9.14                  amd64        GNU C Library: Shared libraries

A newer GLIBC can run binaries requiring an older GLIBC (to an extent), but not the other way around which is this case. Even with static linking I think there would be a problem. So what can we do? Can we instruct the rust compiler to target a specific version of GLIBC? No, we cannot. Alright, so let's use an Ubuntu Focal official rust container. Sorry, not currently possible. All hope is lost right? I mean, yeah probably, but not for this issue! We can use an older Debian Bullseye container that contains a suitable GLIBC!

$ docker run -it rust:bullseye bash
# dpkg -l | grep libc6
ii  libc6:amd64                        2.31-13+deb11u7                amd64        GNU C Library: Shared libraries
ii  libc6-dev:amd64                    2.31-13+deb11u7                amd64        GNU C Library: Development Libraries and Header Files

This solution works on both x86_64 and Apple Silicon M1/M2/M3 Macbooks.

# uname -a
Linux 264571b4fe9f 6.6.6-76060606-generic #202312111032~1702306143~22.04~d28ffec SMP PREEMPT_DYNAMIC Mon D x86_64 x86_64 x86_64 GNU/Linux

# typos -h
Source Code Spelling Correction

Usage: typos [OPTIONS] [PATH]...
# uname -a
Linux b3f20d997b18 6.5.11-linuxkit #1 SMP PREEMPT Wed Dec  6 17:08:31 UTC 2023 aarch64 aarch64 aarch64 GNU/Linux

# typos -h
Source Code Spelling Correction

Usage: typos [OPTIONS] [PATH]...

@beautifulentropy
Copy link
Member

Thanks @pgporada, I've confirmed, as you state above, that this works on my Mac equipped with Apple Silicon.

@szepeviktor
Copy link
Contributor Author

szepeviktor commented Jan 17, 2024

rust:bullseye

Implemented 🍏

@szepeviktor szepeviktor marked this pull request as ready for review January 17, 2024 20:56
@szepeviktor szepeviktor requested a review from a team as a code owner January 17, 2024 20:56
@pgporada
Copy link
Member

I suggest taking the rest of the changes my branch has for the Dockerfile and the new build-rust-deps.sh script.

@szepeviktor
Copy link
Contributor Author

@pgporada Could you open a new PR from your branch?

I get Pull request creation failed. Validation failed: must be a collaborator

@pgporada
Copy link
Member

pgporada commented Jan 17, 2024

Sure. I would have just copied and pasted the files.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants