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

Generalise -- for v2-test, v2-bench, and v2-run #7454

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

ptkato
Copy link
Collaborator

@ptkato ptkato commented Jun 22, 2021

This PR aims to generalise the arguments that are passed to the executables called through v2-run for all the commands that could make use of such feature, as of now v2-test, v2-bench, and v2-run itself. The motivation for this "change" is that people often try to use -- and it doesn't work;

When the arguments reach the command function, they're all mangled together in a single [String]. While that usually works fine for v2-run, due to how it is structured (the command itself takes only one argument, that is the executable name), it won't work for the other commands, this PR will level -- for all the afore mentioned commands.

This PR fixes #6198. It will also close, possibly, #5416.


Please include the following checklist in your PR:

@ptkato
Copy link
Collaborator Author

ptkato commented Jun 22, 2021

Note that this PR is a draft, and the code as of now is only a POC of what I proposed in #6198 (comment).

Comment on lines +188 to +191
if not (null args1)
&& not (null args0)
&& elem (take 1 args0)
(map return ["run", "v2-run", "new-run", "test", "v2-test", "new-test", "bench", "v2-bench", "new-bench"])
Copy link
Collaborator Author

@ptkato ptkato Jun 22, 2021

Choose a reason for hiding this comment

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

I don't like this if, it's too ad hoc-y, however it seems that the actual build phase calls on this part of the code again (I'm actually not sure, see here). So if I don't handpick the command, it starts complaining i.e. unrecognized 'act-as-setup' option '--ghc', and fails to build.

@ulysses4ever ulysses4ever mentioned this pull request Aug 17, 2022
4 tasks
@mergify mergify bot added the merge delay passed Applied (usually by Mergify) when PR approved and received no updates for 2 days label Sep 1, 2022
@ulysses4ever ulysses4ever removed the merge delay passed Applied (usually by Mergify) when PR approved and received no updates for 2 days label Sep 3, 2022
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.

cabal v2-test does not respect --
2 participants