-
Notifications
You must be signed in to change notification settings - Fork 697
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
Add a cabal target command #9744
base: master
Are you sure you want to change the base?
Conversation
64443a6
to
6d9267d
Compare
Related PR: #7500 |
Thanks for the link @fendor. I can see you've put a lot of effort into a larger scoped change. |
This looks plausible to me. We definitely need a test and changelog entry -- though I understand this only makes sense to do if the feature is generally agreed upon, which I can't guarantee by myself. I'll bring it up in the dev meeting on Thursday. |
Because this uses Note I'm suprised to see
|
In investigating further, trying to build only the dependencies, I hit an error I've never seen before;
|
Maybe
Let me emphasise that those are only the targets within the project (as the help already points out) and that different commands can interpret their arguments more liberally (e.g. any package-id is a valid target for |
@philderbeast Ill give it a proper review ASAP |
6d9267d
to
262b008
Compare
I dislike the |
What about |
Can we have a
|
Yes @michaelpj a subcommand instead is a possibility. As implemented this is acting as |
I like |
I favour
|
-1 for attaching this to
Make is different because a) it doesn't have sub-commands and b) the only operation on targets is building them. Food for thought: https://doc.rust-lang.org/cargo/commands/cargo-metadata.html |
My main motivation for this change, no matter which command or subcommand or option we put it under, is to help interactive use of cabal itself and not for producing a machine readable format for IDE tooling like the |
I understand and support this motivation. I think we should avoid too many top-level commands, as I fear trouble down the line with UX and discoverability. While |
2887bc1
to
14f3985
Compare
We have targets when there is no actual project file, when there is just a |
So a top-level |
What else would we do with targets rather than list them that is not already covered by other commands taking targets? Perhaps we should pluralize it as |
14f3985
to
adeb5cf
Compare
Let's keep discussing this. Does anybody remember the conclusions of the discussion of the command names (related to |
adeb5cf
to
ed75265
Compare
ed75265
to
09946f1
Compare
@andreasabel are you suggesting we keep $ cabal list targets --help
Target disclosure and synonym of command 'cabal target'.
- Usage: cabal target [TARGETS]
+ Usage: cabal list targets [TARGETS]
Reveal the targets of build plan. If no [TARGETS] are given 'all' will be used
for selecting a build plan.
A [TARGETS] item can be one of these target forms;
- a package target (e.g. [pkg:]package)
- a component target (e.g. [package:][ctype:]component)
- all packages (e.g. all)
- components of a particular type (e.g. package:ctypes or all:ctypes)
- a module target: (e.g. [package:][ctype:]module)
- a filepath target: (e.g. [package:][ctype:]filepath)
- a script target: (e.g. path/to/script)
The ctypes can be one of: libs or libraries, exes or executables, tests,
benches or benchmarks, and flibs or foreign-libraries.
Flags for target:
-h, --help Show this help text
Examples:
- cabal target all
+ cabal list targets all
Targets of the package in the current directory or all packages in the project
- cabal target pkgname
+ cabal list targets pkgname
Targets of the package named pkgname in the project
- cabal target ./pkgfoo
+ cabal list targets ./pkgfoo
Targets of the package in the ./pkgfoo directory
- cabal target cname
+ cabal list targets cname
Targets of the component named cname in the project |
I'm not sure about this. Even with haskell/containers@c651094 having been built, I'm seeing targets from dependencies shown in the output;
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you want to obtain list the targets you probably do not need all most of targetAction
.
withContextAndSelectors
is what powers cabal run myscript.hs
unless you want to support cabal target myscript.hs
you won't need it. If you do not use it, cabal target
will only work in a project context. This is what I would expect.
runProjectPreBuildPhase
rebuilds the project plan and then uses the function you pass to trim it down to only the required targets. Again, if you just want to list the available targets, you don't need to trim anything. Also it marks the up-to-date targets as 'installed', which means you won't get them from InstallPlan.executionOrder
.
Assuming I understand what you want to do, I think you need to:
- use
establishProjectBaseContext
to read the project configuration - use
rebuildInstallPlan
to get the full plan - use
readTargetSelectors
to parse the target selector strings on the command line into a list ofTargetSelector
- use
resolveTargets
to resolve those target selectors against the plan
This gives you a TargetsMap
associating each unit in the plan with a list of components and the matched selectors.
type TargetsMap = Map UnitId [(ComponentTarget, NonEmpty TargetSelector)]
I guess this is want you might want to display. You might need to lookup the unit ids in the plan again though, which is a bit annoying.
I hope I didn't misunderstood what you are trying to do.
:: Verbosity | ||
-> ProjectBuildContext | ||
-> IO () | ||
printPlanTargetForms |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This function shows every single single component in elaboratedPlanToExecute
. This does not correspond to the list of available targets.
E.g.
✦ ~/code/cabal branchless/09946f1d1fa1217f4480d9ac919e2b264b5a3ecb*
λ $(cabal list-bin cabal) target
...
- uuid-types:lib
- vector-algorithms:lib
- vector-binary-instances:lib
- vector-stream:lib
- vector-th-unbox:lib
- vector:lib
- wherefrom-compat:lib
- witherable:lib
- zinza:lib
- zlib:lib
✦ ~/code/cabal branchless/09946f1d1fa1217f4480d9ac919e2b264b5a3ecb* 10s
λ $(cabal list-bin cabal) build zlib:lib
Warning: this is a debug build of cabal-install with assertions enabled.
Error: [Cabal-7130]
Internal error in target matching: could not make an unambiguous fully qualified target selector for 'zlib:lib'.
We made the target 'zlib:lib' (unknown-component) that was expected to be unambiguous but matches the following targets:
'zlib:lib', matching:
- zlib:lib (unknown-component)
- :pkg:zlib:lib:zlib:file:lib (unknown-file)
Note: Cabal expects to be able to make a single fully qualified name for a target or provide a more specific error. Our failure to do so is a bug in cabal. Tracking issue: https://github.com/haskell/cabal/issues/8684
Hint: this may be caused by trying to build a package that exists in the project directory but is missing from the 'packages' stanza in your cabal project file.
let targetStrings = if null ts then ["all"] else ts | ||
withContextAndSelectors RejectNoTargets Nothing flags targetStrings globalFlags BuildCommand $ \targetCtx ctx targetSelectors -> do |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here you require (RejectNoTargets
) a list of target selectors.Why?
I would think a command to list targets does not need targets to be specified.
If you want to just list the targets you probably do not need all most of this function, but only the elaborated plan.
withContextAndSelectors
is what powers cabal run myscript.hs
unless you want to support cabal target myscript.hs
you won't need it. If you do not use it, cabal target
will only work in a project context. This is what I would expect.
runProjectPreBuildPhase
rebuilds the project plan and then uses the function you pass to trim it down to only the required targets. Again, if you just want to list the available targets, you don't need to trim anything. Also it marks the up-to-date targets as 'installed', which means you won't get them from InstallPlan.executionOrder
.
09946f1
to
3d6a9d0
Compare
3d6a9d0
to
06e0c97
Compare
06e0c97
to
be09cdc
Compare
Adds a
cabal target
command for showing targets that can be supplied to other commands like thecabal build
command.The initial motivation would have been #8953 but this is not a fix for that issue but it could be if the results are filtered. For example
lib
dependencies forall:tests
are not filtered yet.Related issues are #8683, #9732, #1382 and #4070.
I took the
cabal build
command and cut it short where it made a call toprintPlan
. Thecabal target
command callsprintPlanTargetForms
instead that is a modifiation ofprintPlan
. Is there another way to get the targets (an easier way or a better way)?Here's the help;
Note
I've changed the above
cabal --help
to the following.The command specific help, a lot of which is taken straight from the user guide on target forms;
Note
I've changed the above
cabal target --help
to the following.The command in action on the cabal project;
Notice that I'm not stripping out local dependencies that must also be built by the plan;