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

Slimmed down the nix distribution of booster #342

Merged
merged 9 commits into from
Oct 19, 2023
Merged

Conversation

goodlyrottenapple
Copy link
Contributor

@goodlyrottenapple goodlyrottenapple commented Oct 18, 2023

@goodlyrottenapple goodlyrottenapple changed the title Slimmed down the nix distribution of booster [DO NOT MERGE] Slimmed down the nix distribution of booster Oct 18, 2023
@goodlyrottenapple goodlyrottenapple changed the title [DO NOT MERGE] Slimmed down the nix distribution of booster Slimmed down the nix distribution of booster Oct 19, 2023
@goodlyrottenapple goodlyrottenapple marked this pull request as ready for review October 19, 2023 08:32
Comment on lines -8 to -16
QuickCheck -old-random +templatehaskell,
any.StateVar ==1.2.2,
any.adjunctions ==4.4.2,
any.aeson ==2.0.3.0,
aeson -cffi +ordered-keymap,
any.aeson-pretty ==0.8.9,
aeson-pretty -lib-only,
any.ansi-terminal ==0.11.4,
ansi-terminal -example +win32-2-13-1,
Copy link
Member

Choose a reason for hiding this comment

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

The script scripts/freeze-cabal-to-stack-resolver.sh generates these flags so maybe we should not remove them?
Or is the nix-ified version not generating them?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

My local version of cabal removed those and i dont really have a way to run the exact version as on CI. Since we don't use the freeze file for anything anyway (as far as i know) and solely depend on stack.yaml in CI, it doesn't matter much IMO and in the next haskell backend ci update job it will likely put those back anyway...

Copy link
Member

Choose a reason for hiding this comment

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

OK fair enough, if we run into problems with the script we'll notice it soon enough.
I thought we were using the freeze file in the nix build (which uses cabal)...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We were before, but I have switched us back to using stack.yaml/stackage as source of truth for nix.

@@ -28,5 +28,5 @@ done
echo "Running a request which gets stuck, with logTiming enabled"
${client} \
execute $dir/state-c.execute ${client_args} -O log-timing=true | \
$JQ 'del(.result.logs[].time)' | \
jq 'del(.result.logs[].time)' | \
Copy link
Contributor Author

Choose a reason for hiding this comment

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

since the script can now be invoked locally, I've removed $JQ and assume jq is on local path

Copy link
Member

@jberthold jberthold left a comment

Choose a reason for hiding this comment

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

Code LGTM, 🤞 hope CI agrees

@goodlyrottenapple goodlyrottenapple merged commit 26b5d2d into main Oct 19, 2023
5 checks passed
@goodlyrottenapple goodlyrottenapple deleted the sam/slim-nix branch October 19, 2023 09:37
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