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

Missing deps when building @fmt ? #2464

Closed
hhugo opened this issue Jul 29, 2019 · 9 comments · Fixed by #2471
Closed

Missing deps when building @fmt ? #2464

hhugo opened this issue Jul 29, 2019 · 9 comments · Fixed by #2471
Assignees

Comments

@hhugo
Copy link
Collaborator

hhugo commented Jul 29, 2019

related PRs: #1824

@hhugo hhugo changed the title dune build @fmt doesn' dune build @fmt doesn't respect .ocamlformat and .ocamlformat-ignore files Jul 29, 2019
@hhugo hhugo changed the title dune build @fmt doesn't respect .ocamlformat and .ocamlformat-ignore files Missing deps when building @fmt ? Jul 29, 2019
@hhugo
Copy link
Collaborator Author

hhugo commented Jul 29, 2019

It seems that the @fmt alias doesn't depend on .ocamlformat and .ocamlformat-ignore files

@nojb
Copy link
Collaborator

nojb commented Jul 29, 2019

This is probably related to #2404

nojb referenced this issue Jul 29, 2019
Signed-off-by: Nicolás Ojeda Bär <n.oje.bar@gmail.com>
@nojb
Copy link
Collaborator

nojb commented Jul 29, 2019

I tried to reproduce but it seems to work correctly in my machine. @hhugo, could you provide detailed reproduction instructions? Thanks!

@rgrinberg
Copy link
Member

In particular it would be nice to see the $ dune rules -m @fmt. It will give us an immediate answer if the file is considered in the rules.

@hhugo
Copy link
Collaborator Author

hhugo commented Jul 29, 2019

I got the issue with js_of_ocaml

$ dune --version
1.11.0
$ git clone git@github.com:ocsigen/js_of_ocaml.git jsoo-clone
$ cd jsoo-clone
$ opam install ocamlformat.0.10
$ dune build @fmt --auto-promote
$ git status
	modified:   compiler/lib/annot_lexer.ml
	modified:   compiler/lib/annot_parser.ml
	modified:   compiler/lib/annot_parser.mli
	modified:   compiler/lib/js_lexer.ml
	modified:   compiler/lib/js_parser.ml
	modified:   compiler/lib/js_parser.mli
	modified:   tools/wikidoc/odoc_import.ml
	modified:   tools/wikidoc/odoc_wiki.ml
$ cat compiler/lib/.ocamlformat-ignore 
annot_lexer.ml
annot_parser.ml
annot_parser.mli
js_lexer.ml
js_parser.ml
js_parser.mli
ocaml_compiler.cppo.ml
$ cat tools/wikidoc/.ocamlformat 
disable

@nojb
Copy link
Collaborator

nojb commented Jul 29, 2019

Thanks, I was able to reproduce and am investigating.

@nojb
Copy link
Collaborator

nojb commented Jul 29, 2019

It seems to be a problem registering the dependency of .ocamlformat files when they are not in the workspace root (which is why it worked in my simple test).

@nojb
Copy link
Collaborator

nojb commented Jul 29, 2019

The regression is due to no longer using the --name FILE flag in the invocation of ocamlformat. It turns out that some ocamlformat options will be looked up in .ocamformat{,-ignore} files in the parent directories of FILE (this is all that is said in the man page of ocamlformat).

Before #2404 FILE was the filename in the source directory and the .ocamlformat{,-ignore} was looked up there. After #2404 this option --name is no longer passed but the .ocamlformat{,-ignore} are not copied into _build so they are not found by ocamlformat.

Using --name is not completely obvious now because as far as I can see we do not have easy access to the source filename for %{input-file}, so I will try instead to copy the .ocamlformat{,-ignore} files into _build.

@nojb
Copy link
Collaborator

nojb commented Jul 29, 2019

@hhugo fix in #2471. Thanks for the report!

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 a pull request may close this issue.

3 participants