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] atdgen-codec-runtime, atd, atdgen-runtime, atds, atdj and atdgen (2.2.0) #15967

Closed
wants to merge 1 commit into from

Conversation

rgrinberg
Copy link
Member

Runtime for atdgen generated bucklescript converters

CHANGES:
  • Add support for merging double annotations (<ocaml from="ProtoA"><ocaml predef>)

  • Add tests for annotation merging and target-specific annotations

…nd atdgen (2.2.0)

CHANGES:

* Add support for merging double annotations (`<ocaml from="ProtoA"><ocaml predef>`)

* Add tests for annotation merging and target-specific annotations
@kit-ty-kate
Copy link
Member

The packages are failing to build:

#=== ERROR while compiling atd.2.2.0 ==========================================#
# context              2.0.6 | linux/x86_64 | ocaml-base-compiler.4.10.0 | pinned(https://github.com/ahrefs/atd/releases/download/2.2.0/atd-2.2.0.tbz)
# path                 ~/.opam/4.10/.opam-switch/build/atd.2.2.0
# command              ~/.opam/4.10/bin/dune build -p atd -j 72 @install
# exit-code            1
# env-file             ~/.opam/log/atd-23-9af213.env
# output-file          ~/.opam/log/atd-23-9af213.out
### output ###
# File "atdgen/test/dune", line 199, characters 40-51:
# 199 |  (deps    (:atd test_annot_error.atd) %{bin:atdgen})
#                                               ^^^^^^^^^^^
# Error: Program atdgen not found in the tree or in PATH
#  (context: default)

- File "atdgen/test/dune", line 199, characters 40-51:
- 199 |  (deps    (:atd test_annot_error.atd) %{bin:atdgen})
-                                               ^^^^^^^^^^^
- Error: Program atdgen not found in the tree or in PATH
-  (context: default)

@rgrinberg
Copy link
Member Author

Hmm, I have no idea why dune is looking for the executable. It's only necessary for running the tests. In any case, I'm a bit short on time, so I cannot continue to investigate.

@pirbo do you want take over the releasee? Feel free to make whatever changes are necessary to make sure the tests aren't getting in the way. Please don't send patches to opam-repository however, just tag & release from upstream.

@rgrinberg rgrinberg closed this Mar 9, 2020
@kit-ty-kate
Copy link
Member

kit-ty-kate commented Mar 9, 2020

from afar the error seems a bit like the one encountered in #15879 for rpc but with tests rules instead of documentation rules.

@rgrinberg
Copy link
Member Author

rgrinberg commented Mar 9, 2020 via email

@pirbo
Copy link
Contributor

pirbo commented Mar 30, 2020

All my patches needed to fix 2.1.0 have been upstreamed @XVilka.
By uptreaming them, I detected a new potential problem in 2.2.0 for which I gave my workaround in ahrefs/atd#207 (comment) but as it was more hallucinating what the test was supposed to be that fixing something obvious (and as I never ever used ppx-deriving-sexp myself), I didn't turn that into an PR.

The problem here seems to be again simply a missing (package ...) restriction in a runtest rule. I can have a quick look at this particular issue but sorry my hands are too full (as everybody else I know) to be involve in the general maintainance of atd.

@avsm
Copy link
Member

avsm commented Mar 30, 2020

@rgrinberg indicated that the atd release would happen after dune 2.5.0 is out, as there's a bugfix in there that requires the dune fix.

@pirbo
Copy link
Contributor

pirbo commented Mar 30, 2020

Good and I'm happy to de-involve myself :-)
I still put my conclusion as I spent the last hour on that: It looked to me that simply removing %{bin:atdgen} from the deps of test_annot_error.stderr fixes the problem (and dune build @atdgen/runtest still works perfectly in a cleaned env as dune knows it need to build %{bin:atdgen} when it appears in the "rule").

@XVilka
Copy link
Contributor

XVilka commented Apr 14, 2020

Just for the record - Dune 2.5.0 had been merged

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 this pull request may close these issues.

5 participants