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

No error is raised when building an empty package not defined in the dune-project file #6099

Open
kit-ty-kate opened this issue Aug 24, 2022 · 7 comments
Labels
bug requires-team-discussion This topic requires a team discussion

Comments

@kit-ty-kate
Copy link
Member

kit-ty-kate commented Aug 24, 2022

It looks like somewhere between the merge of #4867 and the release of dune 3.0.0, the feature lost all its tests in #5120 and suffers from a regression:

$ cat dune-project
(lang dune 3.0)
$ touch test.opam
$ dune build # With dune >= 3.0.3 no error
$ ~/dune/dune.exe build # With 5c6eb65b149a9254dc2e9bd618df9e198a321705 we get the error
Error: The package test does not have any user defined stanzas attached to it.
If this is intentional, add (allow_empty) to the package definition in the
dune-project file
-> required by _build/default/test.install
-> required by alias all
-> required by alias default
$ echo "(package (name test))" >> dune-project
$ dune build # now dune >= 3.0.3 shows a warning
Error: The package test does not have any user defined stanzas attached to it.
If this is intentional, add (allow_empty) to the package definition in the
dune-project file
-> required by _build/default/test.install
-> required by alias all
-> required by alias default
@emillon
Copy link
Collaborator

emillon commented Sep 16, 2022

According to the description in #5120, that's intentional. This warning only happens when the package is defined in dune-project. So I don't think it's a bug or a regression (has there been a released version which triggers the warning on opam files?). We can discuss about extending the behavior, of course.

@kit-ty-kate
Copy link
Member Author

Ah.... Well, it makes the whole feature pretty much useless so I strongly disagree with #5120. Could we revert that?

@emillon
Copy link
Collaborator

emillon commented Sep 16, 2022

2 different things:

  • I agree that the feature needs some tests
  • we can extend the behavior to packages defined through an opam file

For the latter, the "classic" way to proceed would be to add the warning, and make it fatal for lang dune >= 3.5 but I'd like to evaluate if it triggers a lot on opam-repository to avoid adding to much noise.

@emillon
Copy link
Collaborator

emillon commented Sep 16, 2022

I'll add tests (first part of the above) but the for second part, I'm not sure how to proceed: if we add a warning for packages that are defined through an opam file, what would be the hint to fix the package? If the solution is to add (allow_empty), this means that the definition of the package needs to be added to dune-project (so, we're not in the situation where the package is defined by an opam file anymore).

@kit-ty-kate
Copy link
Member Author

Something like this maybe:

Error: The package <pkg> does not have any user defined stanzas attached to it.
If this is intentional, add (package (name <pkg>) (allow_empty)) to your dune-project file.

@emillon
Copy link
Collaborator

emillon commented Sep 16, 2022

In addition, while writing tests I noticed that the warning case can never happen. I'll fix that next week.

@kit-ty-kate kit-ty-kate added this to the 3.6.0 milestone Nov 4, 2022
@rgrinberg
Copy link
Member

I'm going to remove the 3.6.0 milestone. This issue isn't new and a PR fixing it doesn't seem likely in time for the release. Instead, let's discuss it in a meeting and actually assign this to someone if this is ever to get done.

@rgrinberg rgrinberg removed this from the 3.6.0 milestone Nov 8, 2022
@rgrinberg rgrinberg added the requires-team-discussion This topic requires a team discussion label Nov 8, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug requires-team-discussion This topic requires a team discussion
Projects
None yet
Development

No branches or pull requests

3 participants