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

[new release] xdg, stdune, ordering, fiber, dyn, dune, dune-site, dune-rpc, dune-rpc-lwt, dune-private-libs, dune-glob, dune-configurator, dune-build-info and dune-action-plugin (3.0.0) #20721

Closed
wants to merge 8 commits into from

Conversation

rgrinberg
Copy link
Member

XDG Base Directory Specification

CHANGES:

@smorimoto
Copy link
Member

dune-build-info and xdg have the following error:

#=== ERROR while compiling dune-build-info.3.0.0 ==============================#
# context              2.0.10 | linux/x86_64 | ocaml-base-compiler.4.02.3 | pinned(https://github.com/ocaml/dune/releases/download/3.0.0/fiber-3.0.0.tbz)
# path                 ~/.opam/4.02/.opam-switch/build/dune-build-info.3.0.0
# command              ~/.opam/opam-init/hooks/sandbox.sh build dune build -p dune-build-info -j 31 @install
# exit-code            1
# env-file             ~/.opam/log/dune-build-info-2610-d2d111.env
# output-file          ~/.opam/log/dune-build-info-2610-d2d111.out
### output ###
# File "otherlibs/build-info/src/.build_info.objs/byte/_unknown_", line 1, characters 0-0:
# (cd _build/default && /home/opam/.opam/4.02/bin/ocamlc.opt -w -40 -alert -unstable -g -bin-annot -I otherlibs/build-info/src/.build_info.objs/byte -no-alias-deps -open Build_info__ -o otherlibs/build-info/src/.build_info.objs/byte/build_info__Build_info_data.cmi -c -intf otherlibs/build-info/src/build_info_data.mli)
# /home/opam/.opam/4.02/bin/ocamlc.opt: unknown option '-alert'.

I think it is related to ocaml/dune#5245, does anyone have any ideas?

@rgrinberg
Copy link
Member Author

rgrinberg commented Feb 13, 2022 via email

@smorimoto
Copy link
Member

As this happens with OCaml compilers prior to 4.08, I think it's necessary to set the constraint to make it available only above 4.08, but won't that be a problem for people? If not, we can certainly move forward with it.

@rgrinberg
Copy link
Member Author

rgrinberg commented Feb 13, 2022 via email

@smorimoto
Copy link
Member

Indeed. I just opened a PR for that on the dune side.

@smorimoto
Copy link
Member

Should we move the 3.0.0 tag? Or just publish 3.0.1 instead?

@rgrinberg
Copy link
Member Author

rgrinberg commented Feb 15, 2022 via email

@dinosaure
Copy link
Contributor

It seems that cram tests is broken on ocamlformat.0.{19,20,21}.0. I can reproduce it locally. One of this error is:

Warning: Ocamlformat disabled because [--enable-outside-detected-project] is not set and no [.ocamlformat] was found within the project (root: ../../../../../.sandbox)

It's fixed when we add into tests the option --enable-outside-detected-project with ocamlformat. (/cc @gpetiot)

@kit-ty-kate kit-ty-kate mentioned this pull request Feb 15, 2022
@dinosaure
Copy link
Contributor

dinosaure commented Feb 15, 2022

It seems that we are not able to dune build -p iter @runtest anymore (iter.1.3.0 & iter.1.2.1) (/cc @c-cube). The error is related about the %{deps} variable which is not authorized anymore into the (diff? (is it expected @rgrinberg?). I did this patch which fix the @runtest:

diff --git a/dune b/dune
index 59adf47..7496755 100644
--- a/dune
+++ b/dune
@@ -1,8 +1,8 @@
 
 (alias
  (name runtest)
- (deps README.md)
+ (deps (:readme README.md))
  (action (progn
-          (run ocaml-mdx test %{deps})
-          (diff? %{deps} %{deps}.corrected))))
+          (run ocaml-mdx test %{readme})
+          (diff? %{readme} %{readme}.corrected))))
 

EDIT: the same issue remains for lwt-pipe.0.1
EDIT: the same issue remains for printbox.0.2 & printbox-text.0.6

@dinosaure
Copy link
Contributor

It seems that dune build -p ppx_blob @runtest does not work anymore (/cc @johnwhitington) (hacl_x25519 has the same error but the project is archived). It seems that we need to put the full-path (relative to _build/default) of the file instead of the filename now. A patch like this one fixes the issue:

diff --git a/test/test.ml b/test/test.ml
index 8569593..e74747d 100644
--- a/test/test.ml
+++ b/test/test.ml
@@ -1,6 +1,6 @@
 let suite = [
   ("path relative to source file", `Quick, fun () ->
-    Alcotest.(check string) "file contents" "foo\n" [%blob "test_file"]
+    Alcotest.(check string) "file contents" "foo\n" [%blob "test/test_file"]
   );
 ]
 

rgrinberg and others added 8 commits February 15, 2022 11:40
…e-rpc, dune-rpc-lwt, dune-private-libs, dune-glob, dune-configurator, dune-build-info and dune-action-plugin (3.0.0)

CHANGES:

- Remove `uchar` and `seq` dummy ocamlfind libraries from dune's builtin
  library database (ocaml/dune#5260, @kit-ty-kate)

- Add a `DUNE_DIFF_COMMAND` environment variable to match `--diff-command`
  command-line parameter (@raphael-proust, fix ocaml/dune#5369, ocaml/dune#5375)

- Add support for odoc-link rules (ocaml/dune#5045, @lubegasimon)

- Dune will no longer generate documentation for hidden modules (ocaml/dune#5045,
  @lubegasimon)

- Parse the `native_pack_linker` field of `ocamlc -config` (ocaml/dune#5281, @TheLortex)

- Fix plugins with dot in the name (ocaml/dune#5182, @bobot, review @rgrinberg)

- Don't generate the dune-site build part when not needed (ocaml/dune#4861, @bobot,
  review @kit-ty-kate)

- Fix installation of implementations of virtual libraries (ocaml/dune#5150, fix ocaml/dune#3636,
  @rgrinberg)

- Run tests in all modes defined. Previously, jsoo was excluded. (@hhugo,
  ocaml/dune#5049, fix ocaml/dune#4951)

- Allow to configure the alias to run the jsoo tests (@hhugo, ocaml/dune#5049, ocaml/dune#4999)

- Set jsoo compilation flags in the `env` stanza (@hhugo, ocaml/dune#5049, ocaml/dune#1613)

- Allow to configure jsoo separate compilation in the `env` stanza. Previously,
  it was hard coded to always be enabled in the `dev` profile. (@hhugo, ocaml/dune#5049,
  fix ocaml/dune#970)

- Fix build-info version in jsoo executables (@hhugo, ocaml/dune#5049, fix ocaml/dune#4444)

- Pass `-no-check-prims` when building bytecode for jsoo (@hhugo, ocaml/dune#5049, ocaml/dune#4027)

- Fix jsoo builds when dynamically linked foreign archives are disabled
  (@hhugo, ocaml/dune#5049)

- Disallow empty packages starting from 3.0.  Empty packages may be
  re-enabled by adding the `(allow_empty)` to the package stanza in
  the dune-project file. (ocaml/dune#4867, fix ocaml/dune#2882, @kit-ty-kate, @rgrinberg)

- Add `link_flags` field to the `executable` field of `inline_tests` (ocaml/dune#5088,
  fix ocaml/dune#1530, @jvillard)

- In watch mode, use fsevents instead of fswatch on OSX (ocaml/dune#4937, ocaml/dune#4990, fixes
  ocaml/dune#4896 @rgrinberg)

- Remove `inotifywait` watch mode backend on Linux. We now use the inotify API
  exclusively (ocaml/dune#4941, @rgrinberg)

- Report cycles between virtual libraries and their implementation (ocaml/dune#5050,
  fixes ocaml/dune#2896, @rgrinberg)

- Warn when lang versions have an ignored suffix. `(lang dune 2.3.4)` or `(lang
  dune 2.3suffix)` were silently parsed as `2.3` and we know suggest to remove
  the prefix. (ocaml/dune#5040, @emillon)

- Allow users to specify dynamic dependencies in rules. For example `(deps
  %{read:foo.gen})` (ocaml/dune#4662, fixes ocaml/dune#4089, @jeremiedimino)

- Sandbox infer rules for menhir. Fixes possible "inconsistent assumptions"
  errors (ocaml/dune#5015, @rgrinberg)

- Experimental support for ctypes stubs (ocaml/dune#3905, fixes ocaml/dune#135, @mbacarella)

- Fix interpretation of `binaries` defined in the `env stanza`. Binaries
  defined in `x/dune` wouldn't be visible in `x/*/**/dune. (ocaml/dune#4975, fixes ocaml/dune#4976,
  @Leonidas-from-XIV, @rgrinberg)

- Do not list private libraries in package listings (ocaml/dune#4945, fixes ocaml/dune#4799,
  @rgrinberg)

- Allow spaces in cram test paths (ocaml/dune#4980, fixes ocaml/dune#4162, @rgrinberg)

- Improve error handling of misbehaving cram scripts. (ocaml/dune#4981, fix ocaml/dune#4230,
  @rgrinberg)

- Fix `foreign_stubs` inside a `tests` stanza. Previously, dune would crash
  when this field was present (ocaml/dune#4942, fix ocaml/dune#4946, @rgrinberg)

- Add the `enabled_if` field to `inline_tests` within the `library` stanza.
  This allows us to disable executing the inline tests while still allowing for
  compilation (ocaml/dune#4939, @rgrinberg)

- Generate a `dune-project` when initializing projects with `dune init proj ...`
  (ocaml/dune#4881, closes ocaml/dune#4367, @shonfeder)

- Allow spaces in the directory argument of the `subdir` stanza (ocaml/dune#4943, fixes
  ocaml/dune#4907, @rgrinberg)

- Add a `%{toolchain}` expansion variable (ocaml/dune#4899, fixes ocaml/dune#3949, @rgrinberg)

- Include dependencies of executables when creating toplevels (either `dune
  top` or `dune utop`) (ocaml/dune#4882, fixes ocaml/dune#4872, @Gopiancode)

- Fixes `opam` META file requires entry for private libs (ocaml/dune#4841, fixes ocaml/dune#4839, @toots)

- Fixes `dune exec` not adding .exe on Windows (ocaml/dune#4371, fixes ocaml/dune#3322, @MisterDA)

- Allow multiple cinaps stanzas in the same directory (ocaml/dune#4460, @rgrinberg)

- Fix `$ dune subst` in empty git repositories (ocaml/dune#4441, fixes ocaml/dune#3619, @rgrinberg)

- Improve interpretation of ansi escape sequence when spawning processes (ocaml/dune#4408,
  fixes ocaml/dune#2665, @rgrinberg)

- Allow `(package pkg)` in dependencies even if `pkg` is an installed package
  (ocaml/dune#4170, @bobot)

- Allow `%{version:pkg}` to work for external packages (ocaml/dune#4104, @kit-ty-kate)

- Add `(glob_files_rec <dir>/<glob>)` for globbing files recursively (ocaml/dune#4176,
  @jeremiedimino)

- Automatically generate empty `.mli` files for executables and tests (ocaml/dune#3768,
  fixes ocaml/dune#3745, @craigfe)

- Add `ocaml` command subgroup for OCaml related commands such as `utop`, `top`,
  and `merlin` (ocaml/dune#3936, @rgrinberg).

- Detect unknown variables more eagerly (ocaml/dune#4184, @jeremiedimino)

- Improve location of variables and macros in error messages (ocaml/dune#4205,
  @jeremiedimino)

- Auto-detect `dune-project` files as `dune` files in Emacs (ocaml/dune#4222, @shonfeder)

- Dune no longer automatically create or edit `dune-project` files
  (ocaml/dune#4239, fixes ocaml/dune#4108, @jeremiedimino)

- Warn if `dune-project` is not found (fatal in release mode) (ocaml/dune#5343, @emillon)

- Cleanup temporary files after running `$ dune exec`. (ocaml/dune#4260, fixes ocaml/dune#4243,
  @rgrinberg)

- Add a new subcommand `dune ocaml dump-dot-merlin` that prints a mix of all the
  merlin configuration of a directory (defaulting to the current directory) in
  the Merlin configuration syntax. (ocaml/dune#4250, @voodoos)

- Enable cram tests by default (ocaml/dune#4262, @rgrinberg)

- Drop support for opam 1.x (ocaml/dune#4280, @jeremiedimino)

- Stop calling `ocamlfind` to determine the library search path or
  library installation directory. This makes the behavior of Dune
  simpler and more reproducible (ocaml/dune#4281, @jeremiedimino)

- Remove the `external-lib-deps` command. This command was only
  approximative and the cost of maintainance was getting too high. We
  removed it to make room for new more important features (ocaml/dune#4298,
  @jeremiedimino)

- It is now possible to define action dependencies through a chain
  of aliases. (ocaml/dune#4303, @aalekseyev)

- If an .ml file is not used by an executable, Dune no longer report
  parsing error in this file (ocaml/dune#4330, @jeremiedimino)

- Add support for sandboxing using hard links (ocaml/dune#4360, Andrey Mokhov)

- Fix dune crash when `subdir` is an absolute path (ocaml/dune#4366, @anmonteiro)

- Changed the implementation of actions attached to aliases, as in
  `(rule (alias runtest) (action (run ./test)))`. A visible result for
  users is that such actions are now memoized for longer. For
  instance:
  ```
  $ echo '(rule (alias runtest) (action (echo "X=%{env:X=0}\n")))` > dune
  $ X=1 dune runtest
  X=1
  $ X=2 dune runtest
  X=2
  $ X=1 dune runtest
  ```
  Previously, Dune would have re-executed the action again at the last
  line. Now it remembers the result of the first execution.

- Fix a bug where dune would always re-run all actions that produce symlinks,
  even if their dependencies did not change. (ocaml/dune#4405, @aalekseyev)

- Fix a bug that was causing Dune to re-hash generated files more
  often than necessary (ocaml/dune#4419, @jeremiedimino)

- Fields allowed in the config file are now also allowed in the
  workspace file (ocaml/dune#4426, @jeremiedimino)

- Add options to control how Dune should handle stdout and stderr of
  actions when then succeed. It is now possible to ask Dune to ignore
  the stdout of actions when they succeed or to request that the
  stderr of actions must be empty. This allows to reduce the noise of
  large builds (ocaml/dune#4422, ocaml/dune#4515, @jeremiedimino)

- The `@all` alias no longer depends directly on copies of files from the source
  directory (ocaml/dune#4461, @nojb)

- Allow dune-file as an alternative file name for dune files (needs to be
  enabled in the dune-project file) (ocaml/dune#4428, @nojb)

- Drop support for upgrading jbuilder projects (ocaml/dune#4473, @jeremiedimino)

- Extend the environment variable `BUILD_PATH_PREFIX_MAP` to rewrite
  the root of the build dir (or sandbox) to `/workspace_root` (ocaml/dune#4466,
  @jeremiedimino)

- Simplify the implementation of build cache. We stop using the cache daemon to
  access the cache and instead write to and read from it directly. The new cache
  implementation is based on Jenga's cache library, which was thoroughly tested
  on large-scale builds. Using Jenga's cache library will also make it easier
  for us to port Jenga's cloud cache to Dune. (ocaml/dune#4443, ocaml/dune#4465, Andrey Mokhov)

- More informative error message when Dune can't read a target that's supposed
  to be produced by the action. Old message is still produced on ENOENT, but other
  errors deserve a more detailed report. (ocaml/dune#4501, @aalekseyev)

- Fixed a bug where a sandboxed action would fail if it declares no dependencies in
  its initial working directory or any directory it `chdir`s into. (ocaml/dune#4509, @aalekseyev)

- Fix a crash when clearing temporary directories (ocaml/dune#4489, ocaml/dune#4529, Andrey Mokhov)

- Dune now memoizes all errors when running in the file-watching mode. This
  speeds up incremental rebuilds but may be inconvenient in rare cases, e.g. if
  a build action fails due to a spurious error, such as running out of memory.
  Right now, the only way to force such actions to be rebuilt is to restart
  Dune, which clears all memoized errors. In future, we would like to provide a
  way to rerun all actions failed due to errors without restarting the build,
  e.g. via a Dune RPC call. (ocaml/dune#4522, Andrey Mokhov)

- Remove `dune compute`. It was broken and unused (ocaml/dune#4540,
  @jeremiedimino)

- No longer generate an approximate merlin files when computing the
  ocaml flags fails, for instance because they include the contents of
  a file that failed to build. This was a niche feature and it was
  getting in the way of making Dune's core better. (ocaml/dune#4607, @jeremiedimino)

- Make Dune display the progress indicator in all output modes except quiet
  (ocaml/dune#4618, @aalekseyev)

- Report accurate process timing information in trace mode (enabled with
  `--trace-file`) (ocaml/dune#4517, @rgrinberg)

- Do not log `live_words` and `free_words` in trace file. This allows using
  `Gc.quick_stat` which does not scan the heap. (ocaml/dune#4643, @emillon)

- Don't let command run by Dune observe the environment variable
  `INSIDE_EMACS` in order to improve reproducibility (ocaml/dune#4680,
  @jeremiedimino)

- Fix `root_module` when used in public libraries (ocaml/dune#4685, fixes ocaml/dune#4684,
  @rgrinberg, @craigfe)

- Fix `root_module` when used with preprocessing (ocaml/dune#4683, fixes ocaml/dune#4682,
  @rgrinberg, @craigfe)

- Display Coq profile flags in `dune printenv` (ocaml/dune#4767, @ejgallego)

- Introduce mdx stanza 0.2, requiring mdx >= 1.9.0, with a new generic `deps`
  field and the possibility to statically link `libraries` in the test
  executable. (ocaml/dune#3956, ocaml/dune#5391, fixes ocaml/dune#3955)

- Improve lookup of optional or disabled binaries. Previously, we'd treat every
  executable with missing libraries as optional. Now, we treat make sure to
  look at the library's optional or enabled_if status (ocaml/dune#4786).

- Always use 7 char hash prefix in build info version (ocaml/dune#4857, @jberdine, fixes
  ocaml/dune#4855)

- Allow to explicitly disable/enable the use of `dune subst` by adding a
  new `(subst <disable|enable>)` stanza to the `dune-project` file.
  (ocaml/dune#4864, @kit-ty-kate)

- Simplify the way `dune` discovers the root of the workspace. It now
  stops at the first `dune-workspace` file it encounters, and fails if
  it finds neither a `dune-workspace` nor a `dune-project` file
  (ocaml/dune#4921, fixes ocaml/dune#4459, @jeremiedimino)

- Dune no longer reads installed META files for libraries distributed with the
  compiler, instead using its own internal database. (ocaml/dune#4946, @nojb)

- Add support for `(empty_module_interface_if_absent)` in executable and library
  stanzas. (ocaml/dune#4955, @nojb)

- Add support for `%{bin-available:...}` (ocaml/dune#4995, @jeremiedimino)

- Make sure running `git` or `hg` in a sandboxed action, such as a
  cram test cannot escape the sandbox and pick up some random git or
  mercurial repository on the file system (ocaml/dune#4996, @jeremiedimino)

- Allow `%{read:...}` in more places such as `(enabled_if ...)`
  (ocaml/dune#4994, @jeremiedimino)

- Run each action in its own process group so that we don't leave
  stray processes behind when killing actions (ocaml/dune#4998, @jeremiedimino)

- Add an option `expand_aliases_in_sandbox` (ocaml/dune#5003, @jeremiedimino)

- Allow to cancel the initial scan via Control+C (ocaml/dune#4460, fixes ocaml/dune#4364
  @jeremiedimino)

- Add experimental support for directory targets (ocaml/dune#3316, ocaml/dune#5025, Andrey Mokhov),
  enabled via `(using directory-targets 0.1)` in `dune-project`.

- Delete old `promote-into`, `promote-until-clean` and `promote-until-clean-into`
  syntax (ocaml/dune#5091, Andrey Mokhov).

- Add link_flags in the env stanza (ocaml/dune#5215)

- Bootstrap: ignore errors when trying to remove generated files. (ocaml/dune#5407,
  @damiendoligez)
@kit-ty-kate
Copy link
Member

@johnwhitington
Copy link
Contributor

cc Bertrand Bonnefoy-Claudet @bbc2 the most recent committer (maintainer?) of pxx_blob. I'm afraid it's been years since I touched it or understood PPX.

@kit-ty-kate
Copy link
Member

@rgrinberg this looks rather peculiar as well:

#=== ERROR while compiling ojs_base.0.7.0 =====================================#
# context              2.0.10 | linux/x86_64 | ocaml-base-compiler.4.13.1 | file:///home/opam/opam-repository
# path                 ~/.opam/4.13/.opam-switch/build/ojs_base.0.7.0
# command              ~/.opam/opam-init/hooks/sandbox.sh build dune build -p ojs_base -j 31 --promote-install-files=false @install
# exit-code            1
# env-file             ~/.opam/log/ojs_base-24569-e7966f.env
# output-file          ~/.opam/log/ojs_base-24569-e7966f.out
### output ###
# File "server/dune", line 7, characters 13-36:
# 7 |  (preprocess (pps xtmpl_ppx lwt_ppx))
#                  ^^^^^^^^^^^^^^^^^^^^^^^
# (cd _build/default && .ppx/0e52ba3df38a95b1b52a2bc615777cd5/ppx.exe --cookie 'library-name="ojs_server"' -o server/tmpl.pp.ml --impl server/tmpl.ml -corrected-suffix .ppx-corrected -diff-cmd - -dump-ast)
# Fatal error: exception Not_found

@rgrinberg
Copy link
Member Author

rgrinberg commented Feb 15, 2022

@rgrinberg this looks rather peculiar as well:

#=== ERROR while compiling ojs_base.0.7.0 =====================================#
# context              2.0.10 | linux/x86_64 | ocaml-base-compiler.4.13.1 | file:///home/opam/opam-repository
# path                 ~/.opam/4.13/.opam-switch/build/ojs_base.0.7.0
# command              ~/.opam/opam-init/hooks/sandbox.sh build dune build -p ojs_base -j 31 --promote-install-files=false @install
# exit-code            1
# env-file             ~/.opam/log/ojs_base-24569-e7966f.env
# output-file          ~/.opam/log/ojs_base-24569-e7966f.out
### output ###
# File "server/dune", line 7, characters 13-36:
# 7 |  (preprocess (pps xtmpl_ppx lwt_ppx))
#                  ^^^^^^^^^^^^^^^^^^^^^^^
# (cd _build/default && .ppx/0e52ba3df38a95b1b52a2bc615777cd5/ppx.exe --cookie 'library-name="ojs_server"' -o server/tmpl.pp.ml --impl server/tmpl.ml -corrected-suffix .ppx-corrected -diff-cmd - -dump-ast)
# Fatal error: exception Not_found

I just confirmed this issue with 2.9.x as well. It seems like an odd bug though so I will have a look. It's not related to 3.0 however.

@rgrinberg
Copy link
Member Author

rgrinberg commented Feb 15, 2022

EDIT: I made a mistake when testing this, this is indeed a 3.0 issue. I'm investigating.

Here's the full trace. This issue is ppxlib related. cc @pitag-ha rings a bell perhaps? I'd also suggest not to leak Not_found exceptions in the public API

File "server/dune", line 7, characters 13-36:
7 |  (preprocess (pps xtmpl_ppx lwt_ppx))
                 ^^^^^^^^^^^^^^^^^^^^^^^
Fatal error: exception Not_found
Raised at Stdlib__List.find in file "list.ml", line 224, characters 10-25
Called from Xtmpl_ppx__Ppx_xtmpl.expand_xtmpl in file "ppx/ppx_xtmpl.ml", line 325, characters 15-32
Called from Ppxlib__Ast_pattern_generated.pconst_string.(fun) in file "src/ast_pattern_generated.ml", line 612, characters 27-42
Called from Ppxlib__Ast_pattern_generated.pstr_eval.(fun) in file "src/ast_pattern_generated.ml", line 2697, characters 27-42
Called from Ppxlib__Ast_pattern.(^::).(fun) in file "src/ast_pattern.ml", line 100, characters 18-33
Called from Ppxlib__Ast_pattern.map_result.(fun) in file "src/ast_pattern.ml", line 163, characters 53-71
Called from Ppxlib__Ast_pattern.parse in file "src/ast_pattern.ml", line 11, characters 6-31
Called from Ppxlib__Extension.For_context.convert in file "src/extension.ml", line 247, characters 14-68
Called from Ppxlib__Context_free.map_node in file "src/context_free.ml", line 220, characters 12-46
Called from Ppxlib__Context_free.map_top_down#expression in file "src/context_free.ml", line 457, characters 12-111
Called from Ppxlib__Ast_traverse.map_with_expansion_context#value_binding in file "src/ast_traverse.ml", line 140, characters 21-61
Called from Stdlib__List.map in file "list.ml", line 92, characters 20-23
Called from Ppxlib_ast__Ast.map_with_context#structure_item_desc in file "ast/ast.ml", line 6717, characters 20-54
Called from Ppxlib_ast__Ast.map_with_context#structure_item in file "ast/ast.ml", line 6703, characters 24-62
Called from Ppxlib__Context_free.map_top_down#structure.loop in file "src/context_free.ml", line 618, characters 36-71
Called from Ppxlib__Context_free.map_top_down#structure.loop in file "src/context_free.ml", line 668, characters 31-60
Called from Ppxlib__Driver.Transform.merge_into_generic_mappers.map_impl in file "src/driver.ml", line 247, characters 17-43
Called from Ppxlib__Driver.apply_transforms.(fun) in file "src/driver.ml", line 523, characters 20-28
Called from Stdlib__List.fold_left in file "list.ml", line 121, characters 24-34
Called from Ppxlib__Driver.apply_transforms in file "src/driver.ml", line 505, characters 4-953
Called from Ppxlib__Driver.map_structure_gen in file "src/driver.ml", line 566, characters 4-259
Called from Ppxlib__Driver.process_ast in file "src/driver.ml", line 928, characters 8-112
Called from Ppxlib__Driver.process_file in file "src/driver.ml", line 969, characters 15-80
Called from Ppxlib__Driver.standalone in file "src/driver.ml", line 1391, characters 9-27
Re-raised at Location.report_exception.loop in file "parsing/location.ml", line 932, characters 14-25
Called from Ppxlib__Driver.standalone in file "src/driver.ml", line 1394, characters 4-59
Called from Dune__exe___ppx in file ".ppx/0e52ba3df38a95b1b52a2bc615777cd5/_ppx.ml-gen", line 1, characters 9-36

@pitag-ha
Copy link
Member

The problem in xtmpl_ppx seems strongly related to the problem in ppx_blob, doesn't it? (the Not_found seems to be triggered by an unsuccessful search for a file: https://framagit.org/zoggy/xtmpl/-/blob/master/ppx/ppx_xtmpl.ml#L72)

rings a bell perhaps?

It very broadly rings a bell from an issue reporting a regression in ppxlib: ppxlib passes the path of the current module to the rewriter and it seems like the latest ppxlib version messes up that path in certain circumstances. It doesn't seem to be the same flawful pattern as in the path in the ppx_blob test though. Do you think that can be related anyways?

@pitag-ha
Copy link
Member

I'd also suggest not to leak Not_found exceptions in the public API

What are you referring to? That ppx rewriters such as xtmpl_ppx shoudln't leak Not_found exceptions?

@rgrinberg
Copy link
Member Author

Yes, I thought the exception was raised by ppxlib, but it's raised by the rewriter in question. So indeed it should be the rewriter that should be fixed not to emit Not_found.

@bbc2
Copy link
Contributor

bbc2 commented Feb 15, 2022

About ppx_blob, I would be happy to push a fix and make a release. @dinosaure's suggested fix looks good to me although my (recompiled) ocamllsp doesn't seem to like the new path (unlike the new Dune).

In such a situation, would making a new version of ppx_blob with a constraint like dune >= 3.0.0 make sense? Or is there a smarter way to do that?

@rgrinberg
Copy link
Member Author

Okay, I understand the ppx issue now. The culprit is this PR: ocaml/dune#4466

The issue is that once dune sets BUILD_PATH_PREFIX_MAP, compiler-libs will now mangle paths. The result is that the returned value of Ocaml_common.Location.absolute_path is always junk.

Looking up artifacts this way sounds wrong to me, but we shouuld try and keep old builds working so I will disable this behavior for projects pre 3.0

@@ -33,6 +33,7 @@ conflicts: [
"dune-release" {< "1.3.0"}
"js_of_ocaml-compiler" {< "3.6.0"}
"jbuilder" {= "transition"}
"base-effects"
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kit-ty-kate i don't understand this change, we don't use effects at all in dune. How are we incompatible with it?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

base-effects is a package that depends on a compiler with the "effect syntax" so everytime a project uses e.g.

let effect = ... in

it will fail with the effect syntax.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, so effect is now a keyword. I think I can just rename our uses of effect then.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As you wish but this is basically OCaml 6.0 and the proposal hasn’t really been made yet. It only lives in opam-repository as ocaml-variants.4.12.0+domains+effects

@@ -9,10 +9,10 @@ doc: "https://dune.readthedocs.io/"
bug-reports: "https://github.com/ocaml/dune/issues"
depends: [
"dune" {>= "3.0"}
"result"
"result" {>= "1.5"}
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kit-ty-kate where did this constraint come from? I think I can actually just remove this dependency. We don't use the result package at all anymore.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh never mind. We require it because of Lwt.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This one was just a guess, given anything below 1.5 doesn’t have compatible types with the standard library (Result.t != Stdlib.Result.t)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note that it won’t be the case in Lwt 5.6.0: ocsigen/lwt#925

@kit-ty-kate
Copy link
Member

Superseded by #20758

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment