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

Add package for Topiary v0.1.0 #23771

Merged
merged 1 commit into from
May 17, 2023
Merged

Add package for Topiary v0.1.0 #23771

merged 1 commit into from
May 17, 2023

Conversation

Niols
Copy link
Contributor

@Niols Niols commented May 9, 2023

This pull requests adds a package for Topiary in version 0.1.0 (“Benevolent Beech”). From the point of view of opam-repository and OCaml users, Topiary is an alternative formatter for OCaml. The upstream changelog can be found here.

The main point to note about this package is that is is not an OCaml package: Topiary is written in Rust. However, it can be interesting to publish in opam-repository because it allows formatting OCaml files and could be useful for the community. Some context on this question as well as the idea behind the packaging solution can be found in this discuss thread.

Apart from the problem of whether we accept packaging a Rust program in opam, there is also a technical issue: Topiary requires Cargo/Rustc in version at least 1.65. This version is fairly recent and might not be present in all distributions. In fact, the CI of the tweag/topiary-opam repository keeps track of this. Out of 11 distributions provided by the ocaml/opam DockerHub images:

  • 3 can install Topiary without issues (Fedora, Ubuntu and Ubuntu LTS)
  • 5 cannot install Topiary due to not having a recent enough version of Cargo/Rustc (Alpine, Debian stable, Debian testing, Debian unstable, Oracle Linux)
  • 3 cannot install Topiary due to other problems (Archlinux, CentOS, openSUSE) (The problem with Archlinux is only due to a download issue and I expect it to be resolved rather soon in the ocaml/opam DockerHub images.)

The errors are always fairly understandable. In the case of Debian stable that does not even have the 2021 edition of Cargo/Rustc:

<><> Error report <><><><><><><><><><><><><><><><><><><><><><><><><><><><><><><>
+- The following actions failed
| - build conf-rust-2021 1
+- 
- No changes have been performed
[ERROR] The compilation of conf-rust-2021 failed at "/usr/bin/rustc --edition 2021 test.rs".

#=== ERROR while compiling conf-rust-2021.1 ===================================#
# context              2.0.10 | linux/x86_64 | ocaml-base-compiler.5.0.0 | file:///home/opam/opam-repository
# path                 ~/.opam/5.0/.opam-switch/build/conf-rust-2021.1
# command              /usr/bin/rustc --edition 2021 test.rs
# exit-code            1
# env-file             ~/.opam/log/conf-rust-2021-7-ebe0e0.env
# output-file          ~/.opam/log/conf-rust-2021-7-ebe0e0.out
### output ###
# error: argument for `--edition` must be one of: 2015|2018. (instead was `2021`)

and in the other cases, eg. Debian unstable, that have the right edition but still not recent enough:

<><> Processing actions <><><><><><><><><><><><><><><><><><><><><><><><><><><><>
-> installed conf-rust-2021.1
[ERROR] The compilation of topiary failed at "/usr/bin/cargo build --release --package topiary".

#=== ERROR while compiling topiary.dev ========================================#
# context              2.0.10 | linux/x86_64 | ocaml-base-compiler.5.0.0 | pinned(git+file:///home/opam/topiary-opam#HEAD#e24ecacb)
# path                 ~/.opam/5.0/.opam-switch/build/topiary.dev
# command              /usr/bin/cargo build --release --package topiary
# exit-code            101
# env-file             ~/.opam/log/topiary-7-736596.env
# output-file          ~/.opam/log/topiary-7-736596.out
### output ###
# error: package `tree-sitter v0.20.10` cannot be built because it requires rustc 1.65 or newer, while the currently active rustc version is 1.63.0
# Either upgrade to rustc 1.65 or newer, or use
# cargo update -p [email protected] --precise ver
# where `ver` is the latest version of `tree-sitter` supporting rustc 1.63.0

I therefore expect the CI of opam-repository to report the same kind of issues.

Now I am still not convinced whether it is fine to publish a Rust executable in opam-repository but it is not really for me to decide. The discuss thread linked above seemed to show that people were indeed interested in getting Topiary distributed via opam, but we could also decide that it is not the right way and that Topiary should be acquired from other sources, be it via Nix, Cargo itself, or by opam pinning the tweag/topiary-opam repository. In a few years, after Topiary stabilises and after package sets get updated, Topiary should be available easily as a normal system dependency.

@Niols Niols force-pushed the topiary-0.1.0 branch from 9c4b766 to 7322d85 Compare May 9, 2023 19:00
@mseri
Copy link
Member

mseri commented May 15, 2023

This seems reasonable to me. @avsm what do you think?

@Niols
Copy link
Contributor Author

Niols commented May 16, 2023

Updated numbers with this week's Docker images:

  • 5 can install Topiary without issues (Alpine, Archlinux, Fedora, Ubuntu and Ubuntu LTS)
  • 4 cannot install Topiary due to not having a recent enough version of Cargo/Rustc (Debian stable, Debian testing, Debian unstable, Oracle Linux)
  • 2 cannot install Topiary due to other problems (CentOS, openSUSE)

The change is due to Cargo/Rustc 1.65 reaching Alpine's packages and Archlinux resolving their download issue.

It would be interesting to understand the errors on CentOS and openSUSE but I have no idea how to investigate this. We find basically the same errors in tweag/topiary-opam and ocaml/opam-repository's CIs (which is reassuring tbh).

@avsm
Copy link
Member

avsm commented May 17, 2023

I'm fine with this too. Let's use this as an opportunity to improve the cross-language configuration support within opam-repo, since Rust is here to stay!

@mseri
Copy link
Member

mseri commented May 17, 2023

It would be interesting to understand the errors on CentOS and openSUSE but I have no idea how to investigate this. We find basically the same errors in tweag/topiary-opam and ocaml/opam-repository's CIs (which is reassuring tbh).

Maybe this can helpe: of you look at the top of the logs, there is a code that you can copy/paste to reproduce the issue with docker. For example, for one of the suse tests:

cd $(mktemp -d)
git clone --recursive "https://github.com/ocaml/opam-repository.git" && cd "opam-repository" && git fetch origin "refs/pull/23771/head" && git reset --hard 7322d850
git fetch origin master
git merge --no-edit 297ab9ebb38254eee222f5bb4365ae80baa9caa1
cat > ../Dockerfile <<'END-OF-DOCKERFILE'
FROM ocaml/opam:opensuse-15.4-ocaml-4.14@sha256:9a042c220301a34388f327380c81451949db1e8d464a0293bcdc5271aa4744d6
USER 1000:1000
WORKDIR /home/opam
RUN sudo ln -f /usr/bin/opam-dev /usr/bin/opam
RUN opam init --reinit -ni
ENV OPAMDOWNLOADJOBS="1"
ENV OPAMERRLOGLEN="0"
ENV OPAMSOLVERTIMEOUT="500"
ENV OPAMPRECISETRACKING="1"
RUN rm -rf opam-repository/
COPY --chown=1000:1000 . opam-repository/
RUN opam repository set-url --strict default opam-repository/
RUN opam update --depexts || true
ENV OPAMCRITERIA="-removed,-count[avoid-version,changed],-count[version-lag,request],-count[version-lag,changed],-count[missing-depexts,changed],-changed"
ENV OPAMFIXUPCRITERIA="-removed,-count[avoid-version,changed],-count[version-lag,request],-count[version-lag,changed],-count[missing-depexts,changed],-changed"
ENV OPAMUPGRADECRITERIA="-removed,-count[avoid-version,changed],-count[version-lag,request],-count[version-lag,changed],-count[missing-depexts,changed],-changed"
RUN opam pin add -k version -yn topiary.0.1.0 0.1.0
RUN opam reinstall topiary.0.1.0; \
    res=$?; \
    test "$res" != 31 && exit "$res"; \
    export OPAMCLI=2.0; \
    build_dir=$(opam var prefix)/.opam-switch/build; \
    failed=$(ls "$build_dir"); \
    partial_fails=""; \
    for pkg in $failed; do \
    if opam show -f x-ci-accept-failures: "$pkg" | grep -qF "\"opensuse-15.4\""; then \
    echo "A package failed and has been disabled for CI using the 'x-ci-accept-failures' field."; \
    fi; \
    test "$pkg" != 'topiary.0.1.0' && partial_fails="$partial_fails $pkg"; \
    done; \
    test "${partial_fails}" != "" && echo "opam-repo-ci detected dependencies failing: ${partial_fails}"; \
    exit 1

END-OF-DOCKERFILE
docker build -f ../Dockerfile .

@mseri
Copy link
Member

mseri commented May 17, 2023

I agree that the errors are very explicit and clear, and they are common to the rust packages, so I don't think they should block the merge. Thanks for the contribution!

@mseri mseri merged commit fe808f5 into ocaml:master May 17, 2023
@Niols Niols deleted the topiary-0.1.0 branch May 17, 2023 09:34
@Niols
Copy link
Contributor Author

Niols commented May 17, 2023

Maybe this can helpe: of you look at the top of the logs, [...].

Thank you for that! I will look into that and try to see whether it's a problem coming from upstream or from my packaging in OPAM. Hopefully I can figure that out before v0.2.0!

Thanks for the contribution!

Thank you! 🎉

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.

3 participants