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

dune subst fails on missing opam file #2910

Closed
zapashcanon opened this issue Nov 23, 2019 · 14 comments · Fixed by ocaml/opam-repository#15795
Closed

dune subst fails on missing opam file #2910

zapashcanon opened this issue Nov 23, 2019 · 14 comments · Fixed by ocaml/opam-repository#15795

Comments

@zapashcanon
Copy link
Contributor

zapashcanon commented Nov 23, 2019

Hi,

Running dune subst will fail with Error: No <package>.opam files found. when no opam file is here.

@ghost
Copy link

ghost commented Nov 26, 2019

Indeed. At the time dune subst was introduced, having <package>.opam files was mandatory. It is not anymore, so we should indeed update dune subst to cope with this case.

@zapashcanon zapashcanon changed the title dune subst fails on package named exit dune subst fails on missing opam file Nov 26, 2019
@zapashcanon
Copy link
Contributor Author

Sorry, I didn't checked properly, it has nothing to do with the name of the package. So yes indeed, it shouldn't fail on missing opam file now, I updated the issue accordingly.

@ghost
Copy link

ghost commented Nov 27, 2019

I added a reproduction case in #2931. If you are willing to spend a bit of time on this, I'd be happy to help you write a fix. Otherwse, one of us will fix it at some point.

ghost pushed a commit that referenced this issue Nov 28, 2019
@fangyi-zhou
Copy link
Contributor

I'm encountering the same problem here.
I'm using dune to generate opam file, but received an error when running dune subst. This is rather undesirable since the generated file uses dune subst on the 1st step of the build.

If anyone is happy to give some code pointers, I'm happy to submit a pull request

@ghost
Copy link

ghost commented Dec 3, 2019

@fangyi-zhou a PR would be welcome!

If you look into src/dune/watermarks.ml, there is a function make_watermark_map. This function is responsible for reading the opam file and assigning values to variables such as NAME, VERSION, ... based on the contents of the opam file.

In order to make dune subst when no opam file is present, you need to modify this function to gather the information from the dune-project file instead. To do that, add an argument ~dune_project of type Dune_project.t option to this function. It has to be an option for backward compatibility reason. This value is readily available at the only call site of this function.

If this value is Some p, then you should lookup the package name (argument name of make_watermark_map) in the map Dune_project.packages p. Using both p and the Package.t value looked up in the map, you should have enough to produce the string map at the end of this function.

If dune_project is None, we should keep the current code that reads the opam file for backward compatibility.

@fangyi-zhou
Copy link
Contributor

@diml Thanks for the guideline.
I'm wondering if it is necessary to check the consistency of opam file and dune-project file? If that's the case, which one should take precedence?

I'll try to submit a pull request this week to implement this feature

@ghost
Copy link

ghost commented Dec 4, 2019

We shouldn't need to; if there is no package stanza in the dune-project file, dune will look at opam files already. If there are both package stanzas and opam files, dune will make sure that both define the same set of packages. So if there is a dune-project file, then all the information we need is readily available in Dune_project.packages.

One case we could provide special support for is the following:

  • there are (package ...) stanzas in the dune-project file, but not maintainers, license, ... fields
  • there is no (generate_opam_files true) field, so dune doesn't control the opam files
  • there are opam files with the maintainers, `license, ... fields

In this case, Dune_project.packages p would be missing the maintainers, license, etc... We could handle this case by merging both source of information, but that doesn't seem necessary to me. This is a niche case and moving forward we are headed towards no longer having opam files in repositories anyway.

@fangyi-zhou
Copy link
Contributor

@diml I made some preliminary changes in #2955, could you have a look?

@ghost
Copy link

ghost commented Dec 10, 2019

Sure!

@k4rtik
Copy link

k4rtik commented Jan 15, 2020

Is there a workaround for this issue? Such as downgrade to a certain version of dune?

I am stuck at dune subst step for a package I am trying to create.

@ghost
Copy link

ghost commented Jan 15, 2020

Downgrading won't help here. The only workaround until #2955 is merged is to add an opam file, or ask dune to generate them.

@k4rtik
Copy link

k4rtik commented Jan 15, 2020

When I ask dune to generate them using dune build @install, I still see the following error:

$ ls
_build  dune-project  lib
$ dune build @install
$ ls
_build  dune-project  lib  <pkg>-<name>.opam
$ dune-release distrib
[-] Building source archive
Error: No <package>.opam files found.
dune-release: [ERROR] run ['dune' 'subst']: exited with 1

(redacted package name, but it is there and can be read by dune-release lint for example)

@fangyi-zhou
Copy link
Contributor

fangyi-zhou commented Jan 15, 2020 via email

@ghost
Copy link

ghost commented Jan 16, 2020

Fixed by #2955

@ghost ghost closed this as completed Jan 16, 2020
rgrinberg added a commit to rgrinberg/opam-repository that referenced this issue Feb 6, 2020
…lugin, dune-private-libs and dune-glob (2.2.0)

CHANGES:

- `dune test` is now a command alias for `dune runtest`. This is to make the CLI
  less idiosyncratic (ocaml/dune#3006, @shonfeder)

- Allow to set menhir flags in the `env` stanza using the `menhir_flags` field.
  (ocaml/dune#2960, fix ocaml/dune#2924, @bschommer)

- By default, do not show the full command line of commands executed
  by `dune` when `dune` is executed inside `dune`. This is to make
  integration tests more reproducible (ocaml/dune#3042, @diml)

- `dune subst` now works even without opam files (ocaml/dune#2955, fixes ocaml/dune#2910,
  @fangyi-zhou and @diml)

- Hint when trying to execute an executable defined in the current directory
  without using the `./` prefix (ocaml/dune#3041, fixes ocaml/dune#1094, @voodoos).

- Extend the list of modifiers that can be nested under
  `with-accepted-exit-codes` with `chdir`,  `setenv`, `ignore-<outputs>`,
  `with-stdin-from` and `with-<outputs>-to` (ocaml/dune#3027, fixes ocaml/dune#3014, @voodoos)

- It is now an error to have a preprocessing dependency on a ppx rewriter
  library that is not marked as `(kind ppx_rewriter)` (ocaml/dune#3039, @snowleopard).

- Fix permissions of files promoted to the source tree when using the
  shared cache. In particular, make them writable by the user (ocaml/dune#3043,
  fixes ocaml/dune#3026, @diml)

- Only detect internal OCaml tools with `.opt` extensions. Previously, this
  detection applied to other binaries as well (@kit-ty-kate, @rgrinberg, ocaml/dune#3051).

- Give the user a proper error message when they try to promote into a source
  directory that doesn't exist. (ocaml/dune#3073, fix ocaml/dune#3069, @rgrinberg)

- Correctly build vendored packages in `-p` mode. These packages were
  incorrectly filtered out before. (ocaml/dune#3705, @diml)

- Do not install vendored packages (ocaml/dune#3704, @diml)

- `make` now prints a message explaining the main targets available
  (ocaml/dune#3085, fix ocaml/dune#3078, @diml)

- Add a `byte_complete` executable mode to build programs as
  self-contained bytecode programs
  (ocaml/dune#3076, fixes ocaml/dune#1519, @diml)
mgree pushed a commit to mgree/opam-repository that referenced this issue Feb 19, 2020
…lugin, dune-private-libs and dune-glob (2.2.0)

CHANGES:

- `dune test` is now a command alias for `dune runtest`. This is to make the CLI
  less idiosyncratic (ocaml/dune#3006, @shonfeder)

- Allow to set menhir flags in the `env` stanza using the `menhir_flags` field.
  (ocaml/dune#2960, fix ocaml/dune#2924, @bschommer)

- By default, do not show the full command line of commands executed
  by `dune` when `dune` is executed inside `dune`. This is to make
  integration tests more reproducible (ocaml/dune#3042, @diml)

- `dune subst` now works even without opam files (ocaml/dune#2955, fixes ocaml/dune#2910,
  @fangyi-zhou and @diml)

- Hint when trying to execute an executable defined in the current directory
  without using the `./` prefix (ocaml/dune#3041, fixes ocaml/dune#1094, @voodoos).

- Extend the list of modifiers that can be nested under
  `with-accepted-exit-codes` with `chdir`,  `setenv`, `ignore-<outputs>`,
  `with-stdin-from` and `with-<outputs>-to` (ocaml/dune#3027, fixes ocaml/dune#3014, @voodoos)

- It is now an error to have a preprocessing dependency on a ppx rewriter
  library that is not marked as `(kind ppx_rewriter)` (ocaml/dune#3039, @snowleopard).

- Fix permissions of files promoted to the source tree when using the
  shared cache. In particular, make them writable by the user (ocaml/dune#3043,
  fixes ocaml/dune#3026, @diml)

- Only detect internal OCaml tools with `.opt` extensions. Previously, this
  detection applied to other binaries as well (@kit-ty-kate, @rgrinberg, ocaml/dune#3051).

- Give the user a proper error message when they try to promote into a source
  directory that doesn't exist. (ocaml/dune#3073, fix ocaml/dune#3069, @rgrinberg)

- Correctly build vendored packages in `-p` mode. These packages were
  incorrectly filtered out before. (ocaml/dune#3705, @diml)

- Do not install vendored packages (ocaml/dune#3704, @diml)

- `make` now prints a message explaining the main targets available
  (ocaml/dune#3085, fix ocaml/dune#3078, @diml)

- Add a `byte_complete` executable mode to build programs as
  self-contained bytecode programs
  (ocaml/dune#3076, fixes ocaml/dune#1519, @diml)
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants