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

Linux arm build #1465

Merged
merged 101 commits into from
Sep 9, 2024
Merged

Linux arm build #1465

merged 101 commits into from
Sep 9, 2024

Conversation

csasarak
Copy link
Contributor

@csasarak csasarak commented Sep 4, 2024

Overview

This PR is to add an ARM Linux binary. It's kind of complicated because of a few restrictions that I had to work within:

  1. To test and build a static fossa cli it needs to be built in an an alpine Linux image because GHC isn't runtime relocatable and we need to build against musl libc for a truly static build.
  2. GHA supports ARM Linux runners, but apparently you can't run actions steps that are written in JS inside alpine images and significant parts of our pipeline (like the checkout, caching, tool installation steps) are.
  3. I could not get Rust cross-compiling outside of the Alpine linux image initially. I may revisit this in a future PR.

The basic approach I had to take here was to run the JS actions inside the default Ubuntu Linux ARM image and then run individual build steps inside the haskell-static-alpine image. One unfortunate side-effect of this is that some steps aren't broken out individually in GHA - instead they run all together as part of a build.sh script. I couldn't really see how to avoid this. I made the script output each command it runs so it shouldn't be too bad to follow though.

The build-all.yml workflow is kind of messy right now. I would really like refactor it but I think it's out of scope for this PR.

Acceptance criteria

  • Releases now include a Linux ARM binary.
  • install-latest.sh will install the ARM binary for Linux if necessary.
  • install-latest.sh exits with an error if you try to install a version of the CLI for which an ARM binary doesn't exist on Linux.

Testing plan

I mainly viewed the actions output and tested by downloading binaries from this draft release.

I tested by using subcommands that would use dependent binaries as well such as:

fossa license-scan direct <dir>

and

fossa container analyze test/App/Fossa/Container/testdata/jar_test_container.tar

You can run an emulated Linux arm system in docker with a command like:

docker run --mount type=bind,source=<test-project-dir>,target=/root --platform=linux/arm64 -it ubuntu:latest

Note that if you are running this on x86_64 that emulating an arm machine will slow things down. If you want to test on a more native machine I have an EC2 instance I can get you access to.

Additionally, I tested install-latest.sh by giving it versions that didn't have valid Linux ARM binaries and viewing the error messages. Testing the actual download is harder because it would require me to actually make a release, but I verified the generated URLs would be what I'd expect. The worst-case for this is that customers using the CLI in CI are slightly delayed in getting to deploy on Linux ARM systems. It should not break existing workflows.

Risks

Mainly I want to make sure I didn't get my wires crossed with filenames and such (e.g. putting an amd64 binary in the arm64 archive, etc.). In my testing I didn't see cases where I had but it'd be good to have another set of eyes.

Metrics

None

References

Checklist

  • I added tests for this PR's change (or explained in the PR description why tests don't make sense).
  • If this PR introduced a user-visible change, I added documentation into docs/.
  • If this PR added docs, I added links as appropriate to the user manual's ToC in docs/README.ms and gave consideration to how discoverable or not my documentation is.
  • If this change is externally visible, I updated Changelog.md. If this PR did not mark a release, I added my changes into an # Unreleased section at the top.
  • If I made changes to .fossa.yml or fossa-deps.{json.yml}, I updated docs/references/files/*.schema.json AND I have updated example files used by fossa init command. You may also need to update these if you have added/removed new dependency type (e.g. pip) or analysis target type (e.g. poetry).
  • If I made changes to a subcommand's options, I updated docs/references/subcommands/<subcommand>.md.

@csasarak csasarak marked this pull request as ready for review September 4, 2024 20:45
@csasarak csasarak requested a review from a team as a code owner September 4, 2024 20:45
Copy link
Contributor

@spatten spatten left a comment

Choose a reason for hiding this comment

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

This looks good to me! I made a few comments, but nothing at all blocking approval

.github/workflows/build-all.yml Show resolved Hide resolved
.github/workflows/build-all.yml Outdated Show resolved Hide resolved
# This line adds a comment to our version source file to prompt cabal/GHC to rebuild Version.hs.
echo "{- $GITHUB_RUN_ID -}" >> src/App/Version.hs
cabal update
cabal build --project-file="$PROJECT_FILE" all
Copy link
Contributor

Choose a reason for hiding this comment

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

This is getting rid of the second attempt at a build that we used to do, like this:

$RUN_CMD || $RUN_CMD

Just double-checking that this is not something we want to keep

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It is. Honestly I don't think I've ever seen the issue in the comment before - I think it's a holdover from a while ago. We can always add it back in in the future if needed. What do you think?

Copy link
Contributor

Choose a reason for hiding this comment

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

That sounds good to me. No need to leave it in if we don't need it, and it's easy to add back

.github/workflows/build-all.yml Show resolved Hide resolved
@@ -12,7 +12,7 @@ tikv-jemallocator = { version = "0.5.4", optional = true }
clap = { version = "4.3.21", features = ["derive", "env", "cargo"] }
stable-eyre = "0.2.2"
srclib = { version = "*", git = "https://github.com/fossas/foundation-libs" }
snippets = { version = "0.1.3", git = "https://github.com/fossas/foundation-libs", features = ["lang-all"] }
snippets = { version = "0.1.3", tag = "v0.1.3", git = "https://github.com/fossas/lib-snippets", features = ["lang-all"] }
Copy link
Contributor

Choose a reason for hiding this comment

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

Was this just something we missed updating previously? It doesn't seem related to this PR

Copy link
Contributor Author

@csasarak csasarak Sep 6, 2024

Choose a reason for hiding this comment

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

It isn't related directly. When I was trying to get the rust deps to build I was having trouble and for one of the issues I had I tried updating this library because I thought I might have to make some changes to it. I just kept this bit since it didn't seem to be hurting anything. If you feel strongly about it I can delete.

test/App/Fossa/PathDependencySpec.hs Outdated Show resolved Hide resolved
@csasarak csasarak merged commit 10e2abc into master Sep 9, 2024
19 checks passed
@csasarak csasarak deleted the linux-arm-build branch September 9, 2024 15:23
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.

2 participants