Skip to content

Conversation

@thatportugueseguy
Copy link
Contributor

Passage - used to store and manage access to shared secrets

CHANGES:
  • Update refresh command to handle users and groups via the @<user_or_group> target syntax
  • Add --version flag to passage command
  • Fix the command description for new

CHANGES:

- Update refresh command to handle users and groups via the @<user_or_group> target syntax
- Add --version flag to passage command
- Fix the command description for `new`
Copy link
Member

@jmid jmid left a comment

Choose a reason for hiding this comment

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

Thanks!

There are runtest failures caused by small differences.
What do you make of these?

On Alpine grep is from the busybox utilities IIRC, meaning that not all options are supported:
https://opam.ci.ocaml.org/github/ocaml/opam-repository/commit/f4f8c2c217d0c6d17fd84e2dc018a63411abb1e2/variant/distributions,alpine-3.22-ocaml-4.14,passage.0.2.0,tests

#=== ERROR while compiling passage.0.2.0 ======================================#
# context              2.5.0~rc1 | linux/x86_64 | ocaml-base-compiler.4.14.2 | pinned(https://github.com/ahrefs/passage/releases/download/0.2.0/passage-0.2.0.tbz)
# path                 ~/.opam/4.14/.opam-switch/build/passage.0.2.0
# command              ~/.opam/opam-init/hooks/sandbox.sh build dune build -p passage -j 71 @install @runtest
# exit-code            1
# env-file             ~/.opam/log/passage-7-8b1e20.env
# output-file          ~/.opam/log/passage-7-8b1e20.out
### output ###
# File "tests/replace_comment_command.t", line 1, characters 0-0:
# /usr/bin/git --no-pager diff --no-index --color=always -u _build/default/tests/replace_comment_command.t _build/default/tests/replace_comment_command.t.corrected
# diff --git a/_build/default/tests/replace_comment_command.t b/_build/default/tests/replace_comment_command.t.corrected
# index c30dc85..07087bd 100644
# --- a/_build/default/tests/replace_comment_command.t
# +++ b/_build/default/tests/replace_comment_command.t.corrected
# @@ -29,34 +29,28 @@ Should succeed - replacing single-line comments with multiline comments
#    $ passage cat 00/secret1
#    (00/secret1) secret: single line
#    
# -  replaced again comments
# -  line 2
# +  replaced again comments\nline 2
#  
#  Should succeed - replacing multiline comments with multiline comments
#    $ echo "new comments\nline 2 of said new comments" | passage replace-comment 00/secret1
#    $ passage cat 00/secret1
#    (00/secret1) secret: single line
#    
# -  new comments
# -  line 2 of said new comments
# +  new comments\nline 2 of said new comments
#  
#  Should succeed - replacing multiline comments with multiline comments - in multiline secret
#    $ setup_multiline_secret_with_comments 00/secret2
#    $ echo "new comments\nline 2 of said new comments" | passage replace-comment 00/secret2
#    $ passage cat 00/secret2
#    
# -  new comments
# -  line 2 of said new comments
# +  new comments\nline 2 of said new comments
#    
#    (00/secret2) secret: line 1
#    (00/secret2) secret: line 2
#  
#  Should fail - comments with empty lines in the middle
#    $ echo "uno commento\n\ndos commentos" | passage replace-comment 00/secret1
# -  E: empty lines are not allowed in the middle of the comments
# -  [1]
#    $ passage cat 00/secret1
#    (00/secret1) secret: single line
#    
# -  new comments
# -  line 2 of said new comments
# +  uno commento\n\ndos commentos
# File "tests/edit_command.t", line 1, characters 0-0:
# /usr/bin/git --no-pager diff --no-index --color=always -u _build/default/tests/edit_command.t _build/default/tests/edit_command.t.corrected
# diff --git a/_build/default/tests/edit_command.t b/_build/default/tests/edit_command.t.corrected
# index bafc72e..244dd4e 100644
# --- a/_build/default/tests/edit_command.t
# +++ b/_build/default/tests/edit_command.t.corrected
# @@ -71,18 +71,142 @@ newly encrypted text should not have leftover content from previous encrypted te
#    502
#    $ check_age_file_format $PASSAGE_DIR/secrets/00/long_to_short_secret.age
#    OK: age file starts with expected -----BEGIN AGE ENCRYPTED FILE-----
# -  OK: age file only has 1 occurrence of -----BEGIN AGE ENCRYPTED FILE-----
# +  grep: unrecognized option: fixed-strings
# +  BusyBox v1.37.0 (2025-11-21 22:40:56 UTC) multi-call binary.
# +  
# +  Usage: grep [-HhnlLoqvsrRiwFE] [-m N] [-A|B|C N] { PATTERN | -e PATTERN... | -f FILE... } [FILE]...
[...]

On Fedora there are escaping differences causing test failures:
https://opam.ci.ocaml.org/github/ocaml/opam-repository/commit/f4f8c2c217d0c6d17fd84e2dc018a63411abb1e2/variant/distributions,fedora-41-ocaml-4.14,passage.0.2.0,tests

#=== ERROR while compiling passage.0.2.0 ======================================#
# context              2.4.1 | linux/x86_64 | ocaml-base-compiler.4.14.2 | pinned(https://github.com/ahrefs/passage/releases/download/0.2.0/passage-0.2.0.tbz)
# path                 ~/.opam/4.14/.opam-switch/build/passage.0.2.0
# command              ~/.opam/opam-init/hooks/sandbox.sh build dune build -p passage -j 255 @install @runtest
# exit-code            1
# env-file             ~/.opam/log/passage-7-85bf9d.env
# output-file          ~/.opam/log/passage-7-85bf9d.out
### output ###
# File "tests/replace_comment_command.t", line 1, characters 0-0:
# /usr/bin/git --no-pager diff --no-index --color=always -u _build/default/tests/replace_comment_command.t _build/default/tests/replace_comment_command.t.corrected
# diff --git a/_build/default/tests/replace_comment_command.t b/_build/default/tests/replace_comment_command.t.corrected
# index c30dc85..07087bd 100644
# --- a/_build/default/tests/replace_comment_command.t
# +++ b/_build/default/tests/replace_comment_command.t.corrected
# @@ -29,34 +29,28 @@ Should succeed - replacing single-line comments with multiline comments
#    $ passage cat 00/secret1
#    (00/secret1) secret: single line
#    
# -  replaced again comments
# -  line 2
# +  replaced again comments\nline 2
#  
#  Should succeed - replacing multiline comments with multiline comments
#    $ echo "new comments\nline 2 of said new comments" | passage replace-comment 00/secret1
#    $ passage cat 00/secret1
#    (00/secret1) secret: single line
#    
# -  new comments
# -  line 2 of said new comments
# +  new comments\nline 2 of said new comments
#  
#  Should succeed - replacing multiline comments with multiline comments - in multiline secret
#    $ setup_multiline_secret_with_comments 00/secret2
#    $ echo "new comments\nline 2 of said new comments" | passage replace-comment 00/secret2
#    $ passage cat 00/secret2
#    
# -  new comments
# -  line 2 of said new comments
# +  new comments\nline 2 of said new comments

[...]

@jmid
Copy link
Member

jmid commented Nov 27, 2025

FTR, #28976 addresses the Centos errors.

@thatportugueseguy
Copy link
Contributor Author

There are runtest failures caused by small differences.
What do you make of these?

We've had these issues since the beginning, different distributions handle escaping and whitespace differently, so we get these errors on tests that are very hard to handle with cram tests. Regarding the alpine fail, i'll fix it for the next release (there's another one to be made soon). I'll also update our x-ci-accept-failures values to match the other fails

And the truth is: currently we only care about debian (and passage is mostly used internally).

FTR, #28976 addresses the Centos errors.

That's awesome, thanks for the fixes.

@jmid jmid closed this Nov 28, 2025
@jmid jmid reopened this Nov 28, 2025
@jmid
Copy link
Member

jmid commented Nov 28, 2025

I just did a close-open dance to retrigger a CI run now that #28976 has been merged.

Copy link
Member

@jmid jmid left a comment

Choose a reason for hiding this comment

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

Here's a small suggestion to silence some of the red flags.

I've also created #28989 as an aid for Centos 10 and OpenSUSE Tumbleweed users where installing the prerequisite conf-libpcre is going to fail.

Looking at the CI log, on macOS homebrew I see relevant test suite failures, for example:
https://opam.ci.ocaml.org/github/ocaml/opam-repository/commit/f4f8c2c217d0c6d17fd84e2dc018a63411abb1e2/variant/macos,macos-homebrew-ocaml-4.14-amd64,passage.0.2.0,tests

# File "tests/everyone_group.t", line 1, characters 0-0:
# /usr/bin/git --no-pager diff --no-index --color=always -u _build/default/tests/everyone_group.t _build/default/tests/everyone_group.t.corrected
# diff --git a/_build/default/tests/everyone_group.t b/_build/default/tests/everyone_group.t.corrected
# index 2016edf..d873951 100644
# --- a/_build/default/tests/everyone_group.t
# +++ b/_build/default/tests/everyone_group.t.corrected
# @@ -41,14 +41,75 @@ EDIT - should allow everyone to edit an existing secret
#    (04/secret1) secret: single line
#  
#    $ PASSAGE_IDENTITY=bobby.bob.key EDITOR=$OVERWRITE_WITH_BYE passage edit 04/secret1
# +  Your system does not have /dev/shm, which means that it may
# +  be difficult to entirely erase the temporary non-encrypted
# +  password file after editing.
# +  
# +  Are you sure you would like to continue? [y/N] passage: internal error, uncaught exception:
# +           End_of_file
# +           Raised at Stdlib.input_line.scan in file "stdlib.ml", line 456, characters 14-31
# +           Called from Passage__Prompt.yesno in file "lib/prompt.ml", line 12, characters 12-24
# +           Called from Passage__Util.Editor.shm_check in file "lib/util.ml", line 28, characters 9-226
# +           Called from CamlinternalLazy.force_lazy_block in file "camlinternalLazy.ml", line 31, characters 17-27
# +           Re-raised at CamlinternalLazy.force_lazy_block in file "camlinternalLazy.ml", line 36, characters 4-11
# +           Called from Passage__Util.Editor.with_secure_tmpfile in file "lib/util.ml", line 40, characters 12-32
# +           Called from Dune__exe__Main.Edit.edit_secret in file "bin/main.ml", line 53, characters 27-61
# +           Called from Cmdliner_term.app.(fun) in file "cmdliner_term.ml", line 24, characters 19-24
# +           Called from Cmdliner_eval.run_parser in file "cmdliner_eval.ml", line 35, characters 37-44
# +  [125]
#    $ PASSAGE_IDENTITY=dobby.dob.key EDITOR=$OVERWRITE_WITH_BYE passage edit 04/secret1
# -  I: secret unchanged
# +  Your system does not have /dev/shm, which means that it may
# +  be difficult to entirely erase the temporary non-encrypted
# +  password file after editing.
# +  
# +  Are you sure you would like to continue? [y/N] passage: internal error, uncaught exception:
# +           End_of_file
# +           Raised at Stdlib.input_line.scan in file "stdlib.ml", line 456, characters 14-31
# +           Called from Passage__Prompt.yesno in file "lib/prompt.ml", line 12, characters 12-24
# +           Called from Passage__Util.Editor.shm_check in file "lib/util.ml", line 28, characters 9-226
# +           Called from CamlinternalLazy.force_lazy_block in file "camlinternalLazy.ml", line 31, characters 17-27
# +           Re-raised at CamlinternalLazy.force_lazy_block in file "camlinternalLazy.ml", line 36, characters 4-11
# +           Called from Passage__Util.Editor.with_secure_tmpfile in file "lib/util.ml", line 40, characters 12-32
# +           Called from Dune__exe__Main.Edit.edit_secret in file "bin/main.ml", line 53, characters 27-61
# +           Called from Cmdliner_term.app.(fun) in file "cmdliner_term.ml", line 24, characters 19-24
# +           Called from Cmdliner_eval.run_parser in file "cmdliner_eval.ml", line 35, characters 37-44
# +  [125]

What do you make of this? 🤔
Something to address or should we quickfix by disabling runtest on macOS?

Finally, would you consider adding an x-maintenance-intent entry?
https://github.com/ocaml/opam-repository/blob/master/governance/policies/archiving.md

"sha512=cd5bf893e62d4b6a0d88187604747385a41c52d0c757ed4667d529499c8b0f9a5e042964e8caf2455b6032b9684f07210a4eb1577e7496e759585717677a46b8"
]
}
x-commit-hash: "92bc02a41e427353a821125143e0f44d425ed07f"
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
x-commit-hash: "92bc02a41e427353a821125143e0f44d425ed07f"
x-commit-hash: "92bc02a41e427353a821125143e0f44d425ed07f"
x-ci-accept-failures: [ # the test suite exhibits small escaping differences on the below platforms
"centos-9"
"alpine-3.22"
"fedora-41"
"fedora-42"
"fedora-43"
]

Here's a concrete suggestion to silence the red CI flags caused by escaping differences.

@thatportugueseguy
Copy link
Contributor Author

I have added a new PR (#29020) for the changes mentioned above. I have incorporated your suggestions on those changes.

Regarding x-maintenance-intent, we'll add that at a later point.

Can this PR be merged even with these tests not passing? Thank you in advance

Copy link
Member

@jmid jmid left a comment

Choose a reason for hiding this comment

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

I have added a new PR (#29020) for the changes mentioned above. I have incorporated your suggestions on those changes.

Thanks! 🙏

Regarding x-maintenance-intent, we'll add that at a later point.

OK 👍

Can this PR be merged even with these tests not passing? Thank you in advance

PRs with failing tests are often merged.

The current 30 jobs failed seem a bit much, as they will trigger again on the next release of dune, menhir, ... and thus add noise.

In my first x-ci-accept-failures I had somehow missed the existing field in the package! 😅 Below I've made a revised proposal to silence some of those red CI lights. It is no big deal, hopefully easily click-and-acceptable, and will help keep the CI noise level down in here. 🙂

thatportugueseguy and others added 3 commits December 3, 2025 12:00
Co-authored-by: Jan Midtgaard <mail@janmidtgaard.dk>
Co-authored-by: Jan Midtgaard <mail@janmidtgaard.dk>
Co-authored-by: Jan Midtgaard <mail@janmidtgaard.dk>
Copy link
Member

@jmid jmid left a comment

Choose a reason for hiding this comment

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

OK, thanks!

For context of anyone new reading:
The already merged #29020 offers passage.0.3.0 with a number of these issues already fixed.

We have to reach a compromise between an acceptable number of failures for
opam-repo maintainers and the constraints and interest of you, as the author:

And the truth is: currently we only care about debian (and passage is mostly used internally).

I've therefore taken another look at the CI results, in the interest of getting this across the line. There are very few platforms where runtests is running and passing - I count 7 out of the 71 or so from "distributions" and down here:
https://opam.ci.ocaml.org/github/ocaml/opam-repository/commit/9cf3539900e79f39a61be41771d4a5225c9945c7

  • debian-12-ocaml-4.14
  • ubuntu-22.04-ocaml-4.14
  • ubuntu-24.04-ocaml-4.14
  • ubuntu-25.04-ocaml-4.14
  • ubuntu-25.04-ocaml-5.4
  • ubuntu-25.10-ocaml-4.14
  • riscv64-ocaml-4.14

I therefore suggest to enable runtest only on Debian and Ubuntu and on OCaml 4.14. This can be expressed as two constraints (one on ocaml for with-test and one guarding a runtest invocation).

Collectively these will address a pile of the red CI lights (7 runtest timeouts + 2 macOS runtest failures), and thus not introduce too many false alarms for repo-maintainers trying to track regressions, while also not asking you to hunt and fix failures that you do not care about.

A few things will remain, which cannot be blamed on this release:

  • Centos 10 and OpenSUSE Tumbleweed system package installation errors of libpcre-devel
  • the usual opam-2.0 faiures
  • libevent.0.8.1 (a dependency) failing to build on macOS 4.14 arm64

Does this sound as a reasonable solution? (sorry for a bit of back-and-forth)

thatportugueseguy and others added 2 commits December 4, 2025 16:05
Co-authored-by: Jan Midtgaard <mail@janmidtgaard.dk>
Co-authored-by: Jan Midtgaard <mail@janmidtgaard.dk>
@thatportugueseguy
Copy link
Contributor Author

Does this sound as a reasonable solution?

Yes, i think so. Let's see if everything works as expected, and i'll port these changes to the repo for later releases.

(sorry for a bit of back-and-forth)

All good. Thanks for caring and showing possible solutions. Maintainig the opam ci happy is always a bit of a chore in this project, so it's good if we can have a better solution :)

Copy link
Member

@jmid jmid left a comment

Choose a reason for hiding this comment

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

Oh dear... There are now timeouts during runtests for

  • debian-11-ocaml-4.14
  • ubuntu-25.10-ocaml-4.14

We've now observed this both with 4.14 and 5.4 and on different distributions.
Furthermore this happens when executing opam reinstall --with-test passage.0.2.0
whereas a pure package installation without --with-test succeeds.

I can also see similar timeouts in the 0.3.1 release: #29022

As such, this indicates that some condition in your test suite is (a) non-deterministic and (b) may trigger across distributions.

At this point it seems the easiest would be to disable runtest, which I propose below.

P.s. system package installation failures and workflow timeouts are unaffected by x-ci-accept-failures because the shell-script catching failures will only do so, if it gets to run as a handler after opam reinstall ... and finds a non-zero exit code.

"-j"
jobs
"@install"
"@runtest" {with-test & (os-distribution="debian" | os-distribution="ubuntu")}
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
"@runtest" {with-test & (os-distribution="debian" | os-distribution="ubuntu")}

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants