TargetSelector: Include a hint in the error message for internal error #8628

lf- · 2022-12-06T19:29:16Z

This is a mediocre solution, and really should generate a different error entirely at an earlier stage, but I'm not familiar enough with how to achieve that and I don't think I have time at the moment.

Fixes #8484.

[nix-shell:~/co/cabal/t8484]$ cabal build lib:generic-deriving
Error: cabal: Internal error in target matching. It should always be possible
to find a syntax that's sufficiently qualified to give an unambiguous match.
However when matching 'lib:generic-deriving' we found lib:generic-deriving
(unknown-component) which does not have an unambiguous syntax. The possible
syntax and the targets they match are as follows:
'lib:generic-deriving' which matches lib:generic-deriving (unknown-component),
:pkg:lib:lib:lib:file:generic-deriving (unknown-file)


[nix-shell:~/co/cabal/t8484]$ /Users/jade/co/cabal/dist-newstyle/build/aarch64-osx/ghc-9.2.4/cabal-install-3.9.0.0/x/cabal/build/cabal/cabal build lib:generic-deriving
Warning: this is a debug build of cabal-install with assertions enabled.
Error: cabal: Internal error in target matching. It should always be possible
to find a syntax that's sufficiently qualified to give an unambiguous match.
However when matching 'lib:generic-deriving' we found lib:generic-deriving
(unknown-component) which does not have an unambiguous syntax. The possible
syntax and the targets they match are as follows:
'lib:generic-deriving' which matches lib:generic-deriving (unknown-component),
:pkg:lib:lib:lib:file:generic-deriving (unknown-file)


Hint: this may be caused by trying to build a package that exists in the
project directory but is missing from cabal.project.

Please include the following checklist in your PR:

Patches conform to the coding conventions.
[??] Any changes that could be relevant to users have been recorded in the changelog.
The documentation has been updated, if necessary.

Please also shortly describe how you tested your change. Bonus points for added tests!

lf- · 2022-12-06T19:43:10Z

looks like the ghc 9.4.2 CI is broken unrelated to this

Mikolaj · 2022-12-07T10:59:16Z

Indeed, CI was broken. Let me rebase to include the fix.

Mikolaj · 2022-12-07T10:59:22Z

@mergify rebase

mergify · 2022-12-07T11:00:29Z

rebase

✅ Branch has been successfully rebased

Mikolaj · 2022-12-07T12:52:32Z

Success!

jneira · 2023-01-03T20:59:32Z

Hi, thanks for the pr. I agree this is not optimal and it does not scale if we have to list all possible causes of the error. Having only one of the possible causes will make users check it in first place for the rest of cases.

dmwit · 2023-01-15T17:45:40Z

For what it's worth, I just ran into this error, and the hint given here would have resolved my problem.

Mikolaj · 2023-01-16T13:48:03Z

Hah, so we have divergent opinions.

@jneira: any cheap constructive suggestion from you? E.g., adding a disclaimer to the hint that it's only one of possible causes?

Any other opinions, ideas, anybody?

jneira · 2023-01-16T14:23:30Z

yeah such disclaimer would be fine, good idea

ulysses4ever · 2023-01-16T15:12:00Z

I like the current state. What I'm afraid of is that the output is already very mouthful. I'm not sure many people will get to the hint. Even less so — to the disclaimer. I'd invite people to think whether we could shrink the preexisted text.

Anyway, I think this should be merged in some form; e.g. just adding a disclaimer to the current wording in the patch would work for me.

Mikolaj · 2023-01-16T15:40:30Z

So perhaps let's start with a short version of the hint and elaborate afterwards?

@lf-: your take? we'd better merge ASAP for cabal 3.10.

lf- · 2023-01-16T17:43:03Z

So perhaps let's start with a short version of the hint and elaborate afterwards?

@lf-: your take? we'd better merge ASAP for cabal 3.10.

Thoughts on the following attempt? I can get it into code later today.

Internal error in target matching: could not make a fully qualified target selector.
We made the target 'lib:generic-deriving' that was expected to be unambiguous but matches the following targets:

lib:generic-deriving (unknown-component)

:pkg:lib:lib:lib:file:generic-deriving (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: #8684

Hint: one possible cause is that a package 'generic-deriving' exists in
the project directory but isn't included in the packages stanza of your cabal project file

lf- · 2023-01-16T17:54:34Z

several edits later, i think my rewrite of the message is at least more legible and concise. any thoughts? did i remove any info that needed to be there?

Mikolaj · 2023-01-16T18:10:56Z

@dmwit: you are the positive force here: would that one be helpful to you still?

ulysses4ever · 2023-01-16T19:02:16Z

Looks like a good improvement to me, thank you!

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.

Well, our failure to do the latter (provide a more specific error) is a bug, but the former (resolve the target) may be not. So, this phrase is a little confusing to me.

lf- · 2023-01-16T19:35:46Z

Looks like a good improvement to me, thank you!

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.

Well, our failure to do the latter (provide a more specific error) is a bug, but the former (resolve the target) may be not. So, this phrase is a little confusing to me.

My understanding of this error is that this error is saying "we screwed up and couldn't come up with an unambiguous qualified name for the target", which is always a bug. There's two cases:

We should have issued a different error earlier, and the leftovers of that mistake have caused the qualified-name logic to explode, which is a bug (I think that this is the case when we issue this error for something missing from the project file)
We did actually screw up coming up with a fully qualified name, and the qualified name code should have done a better job, which is also a bug, since it should always be able to come up with a qualified name

ulysses4ever · 2023-01-16T21:19:43Z

My understanding of this error is that this error is saying "we screwed up and couldn't come up with an unambiguous qualified name for the target"

Maybe I'm wrong but I think that this failure can come from the fact that the user underspecified the target. Imagine there's an executable and a test with the same name blah. Cabal will have to fail if you ask to cabal build blah, right?

lf- · 2023-01-16T21:26:11Z

Nope, this is specifically a "we screwed up" error. That error is as follows, today:

$ cabal build joshing
Error: cabal: Ambiguous target 'joshing'. It could be:
exe:joshing (component)
lib:joshing (component)

lf- · 2023-01-17T06:34:45Z

Alright, it looks like so:

[nix-shell:~/co/cabal/repro]$ /home/jade/co/cabal/dist-newstyle/build/x86_64-linux/ghc-9.2.4/cabal-install-3.9.0.0/x/cabal/build/cabal/cabal build lib:generic-deriving
Warning: this is a debug build of cabal-install with assertions enabled.
Error: cabal: Internal error in target matching: could not make an unambiguous
fully qualified target selector for 'lib:generic-deriving'.
We made the target 'lib:generic-deriving' (unknown-component) that was
expected to be unambiguous but matches the following targets:
'lib:generic-deriving', matching:
- lib:generic-deriving (unknown-component)
- :pkg:lib:lib:lib:file:generic-deriving (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.

dmwit · 2023-01-17T15:31:15Z

The updated error+hint looks quite approachable to me. The target name :pkg:lib:lib:lib:file:generic-deriving still kind of blows my mind -- apparently the language of targets is much richer than I knew -- but that's kinda beside the point!

Mikolaj

I'd say, let's ship it.

Mikolaj · 2023-01-17T17:56:57Z

@lf-: please kindly set the merge_me label once you are ready.

lf- · 2023-01-17T18:11:14Z

@lf-: please kindly set the merge_me label once you are ready.

I cannot do this because I don't have the relevant github role, but go ahead and merge :)

Mikolaj · 2023-01-17T18:20:28Z

Oh, you have not accepted the invite? Whatever you prefer...

lf- · 2023-01-17T18:24:04Z

ah my mistake, didn't see the email. done

Mikolaj · 2023-01-17T18:47:42Z

Splendid. Thank you. In 2 days, it should auto-merge, unless the PR changes.

This is a mediocre solution, and really should generate a different error entirely at an earlier stage, but I'm not familiar enough with how to achieve that.

mergify · 2023-01-19T18:49:32Z

⚠️ This pull request got rebased on behalf of a random user of the organization.
This behavior will change on the 1st February 2023, Mergify will pick the author of the pull request instead.

To get the future behavior now, you can configure bot_account options (e.g.: bot_account: { author } or update_bot_account: { author }.

Or you can create a dedicated github account for squash and rebase operations, and use it in different bot_account options.

kokobd force-pushed the jadel/t8484 branch from 1ca2eb4 to 920a1a8 Compare December 7, 2022 11:00

ulysses4ever approved these changes Jan 16, 2023

View reviewed changes

lf- force-pushed the jadel/t8484 branch from 920a1a8 to f071c9d Compare January 17, 2023 06:34

Mikolaj approved these changes Jan 17, 2023

View reviewed changes

lf- added the merge me Tell Mergify Bot to merge label Jan 17, 2023

mergify bot added the merge delay passed Applied (usually by Mergify) when PR approved and received no updates for 2 days label Jan 19, 2023

TargetSelector: Rewrite the internal error message

58bc9c7

This is a mediocre solution, and really should generate a different error entirely at an earlier stage, but I'm not familiar enough with how to achieve that.

cocreature force-pushed the jadel/t8484 branch from f071c9d to 58bc9c7 Compare January 19, 2023 18:49

mergify bot merged commit a78d77b into haskell:master Jan 19, 2023

TargetSelector: Include a hint in the error message for internal error #8628

TargetSelector: Include a hint in the error message for internal error #8628

Uh oh!

Conversation

lf- commented Dec 6, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

lf- commented Dec 6, 2022

Uh oh!

Mikolaj commented Dec 7, 2022

Uh oh!

Mikolaj commented Dec 7, 2022

Uh oh!

mergify bot commented Dec 7, 2022

✅ Branch has been successfully rebased

Uh oh!

Mikolaj commented Dec 7, 2022

Uh oh!

jneira commented Jan 3, 2023

Uh oh!

dmwit commented Jan 15, 2023

Uh oh!

Mikolaj commented Jan 16, 2023

Uh oh!

jneira commented Jan 16, 2023

Uh oh!

ulysses4ever commented Jan 16, 2023

Uh oh!

Mikolaj commented Jan 16, 2023

Uh oh!

lf- commented Jan 16, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

lf- commented Jan 16, 2023

Uh oh!

Mikolaj commented Jan 16, 2023

Uh oh!

ulysses4ever commented Jan 16, 2023

Uh oh!

lf- commented Jan 16, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ulysses4ever commented Jan 16, 2023

Uh oh!

lf- commented Jan 16, 2023

Uh oh!

lf- commented Jan 17, 2023

Uh oh!

dmwit commented Jan 17, 2023

Uh oh!

Mikolaj left a comment

Choose a reason for hiding this comment

Uh oh!

Mikolaj commented Jan 17, 2023

Uh oh!

lf- commented Jan 17, 2023

Uh oh!

Mikolaj commented Jan 17, 2023

Uh oh!

lf- commented Jan 17, 2023

Uh oh!

Mikolaj commented Jan 17, 2023

Uh oh!

mergify bot commented Jan 19, 2023

Uh oh!

Uh oh!

lf- commented Dec 6, 2022 •

edited

Loading

lf- commented Jan 16, 2023 •

edited

Loading

lf- commented Jan 16, 2023 •

edited

Loading