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

411 simple integration test using old simple rust programs #413

Merged
merged 32 commits into from
Sep 8, 2024

Conversation

jberthold
Copy link
Member

@jberthold jberthold commented Aug 28, 2024

This PR adds a set of simple rust programs (from prior art) for a simple parser test:

Remarks:

  • The tests are currently only proving that the parser succeeds. We can add better checks later when the format is stable enough to not cause noise in golden tests.
  • In order to run smir_pretty, information about the rust target architecture is needed. To circumvent this, the related script (within smir_pretty repo) is patched (but only the copy in the docker file system). This is a workaround and could be addressed properly by
    • crafting a github action to set up K and removing the dockerised K,
    • or preparing smir_pretty to run within nix and using a nix shell.
    • or (not recommended) placing the custom rust build and smir_pretty within the docker image or container.

@jberthold jberthold marked this pull request as ready for review August 30, 2024 03:01
Comment on lines +122 to +130
- name: 'HACK: patch rustc_arch.sh script (within docker)'
run: |
arch=$(rustc -vV | sed -n -e 's/host: \(.*\)$/\1/p')
docker exec --user user mir-smir-ci-${GITHUB_SHA} \
bash -c "printf '#!/bin/sh\necho \"$arch\"\n' > deps/smir_pretty/rustc_arch.sh"
docker exec --user user mir-smir-ci-${GITHUB_SHA} \
cat deps/smir_pretty/rustc_arch.sh
docker exec --user user mir-smir-ci-${GITHUB_SHA} \
deps/smir_pretty/rustc_arch.sh
Copy link
Collaborator

Choose a reason for hiding this comment

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

@jberthold This is the hack to get our version of rustc running for smir pretty on CI?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes. The problem is that we need to run inside the docker image (to have kompile) but there is no rustup in the image. That script uses rustup to find rustc, then calls rustc just to find out the target architecture string (which is part of the path to the libraries to link to). ➰ ➿
The hack is to find that architecture string outside the docker image and then patch the script inside the docker image's file system (which is not shared with the host for some other reason... 🤷 ) to print that string

Copy link
Collaborator

Choose a reason for hiding this comment

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

Okay my understanding is that this will not need to rebuild the stage 2 compiler from our semantics from consecutive runs of the tests, and even if the K source is changed as the semantics will kompile but the stage 2 compiler is already built.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, I even think the entire set of compiler artefacts could be cached, and tried to set up a cache for it, but the compiler build setup is multi-stage and therefore more complex to avoid rebuilds. For now this was just supposed to be the first step - our setup with the compiler dependency is definitely "suboptimal" anyway (and should/will change soon).

@dkcumming
Copy link
Collaborator

I will let you choose when to merge @jberthold in case there was more commits to add

@jberthold jberthold merged commit 4ca065c into master Sep 8, 2024
5 checks passed
@jberthold jberthold deleted the 411-salvage-old-rust-tests branch September 8, 2024 23:43
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