Skip to content

Remove devkit dependency#11

Merged
thatportugueseguy merged 5 commits intoahrefs:masterfrom
arvidj:remove-devkit-dependency
Jan 20, 2026
Merged

Remove devkit dependency#11
thatportugueseguy merged 5 commits intoahrefs:masterfrom
arvidj:remove-devkit-dependency

Conversation

@arvidj
Copy link
Contributor

@arvidj arvidj commented Jan 15, 2026

With this change, passage is a easier to install from opam because the dependency cone is greatly reduced. In particular:

  • no more dependency on libevent which is hard to install on macOS
  • no more dependency on libpcre which is no longer packaged in debian trixie
  • Just less dependencies:
| with devkit & --with-{doc,dev-setup,test}:    | 140 dependencies |
| with devkit:                                  | 125 dependencies |
| without devkit & --with-{doc,dev-setup,test}: | 118 dependencies |
| without devkit:                               | 101 dependencies |

Most things passage used from devkit can be found in the OCaml stdlib these days.

Builds & passes tests, code is mainly AI written but I've reviewed it. Seems to be working fine but review & suggestions for additional testing is welcome!

Finally, I think the dependency cone can be reduced even further by removing bos, extunix, replacing re2 with re, and making ppx_expect --with-test. Happy to work on it or to provide advice :)

@arvidj arvidj marked this pull request as ready for review January 15, 2026 07:46
@arvidj arvidj changed the title remove devkit dependency Remove devkit dependency Jan 15, 2026
@thatportugueseguy thatportugueseguy merged commit 71dc6ca into ahrefs:master Jan 20, 2026
4 checks passed
thatportugueseguy added a commit to arvidj/passage that referenced this pull request Jan 20, 2026
* origin/master:
  Remove devkit dependency (ahrefs#11)
  update Cmdliner dependency to allow version 2.0.0 (ahrefs#10)
thatportugueseguy added a commit to arvidj/passage that referenced this pull request Jan 20, 2026
…extect-with-test

* origin/master:
  Replace re2 with re (ahrefs#12)
  Remove devkit dependency (ahrefs#11)
  update Cmdliner dependency to allow version 2.0.0 (ahrefs#10)
Comment on lines -693 to +691
Devkit.Files.save_as (Path.project target_file) ~mode:0o600 (fun oc -> Out_channel.output_string oc contents)
let target_file = Path.project target_file in
let flags = [ Open_wronly; Open_creat; Open_trunc; Open_binary ] in
Out_channel.(with_open_gen flags 0o600 target_file (fun oc -> output_string oc contents))
Copy link

Choose a reason for hiding this comment

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

this is not nice, see comment in save_as why

Copy link
Contributor Author

Choose a reason for hiding this comment

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

See #15, let's move discussion there.

Comment on lines -94 to +97
Devkit.Control.with_open_out_temp_file ~temp_dir ~mode:[ Open_wronly; Open_creat; Open_excl ] f
let tmpfile, tmpfile_oc =
Filename.open_temp_file ~mode:[ Open_wronly; Open_creat; Open_excl ] ~temp_dir "" ".passage"
in
Fun.protect ~finally:(fun () -> try Sys.remove tmpfile with _ -> ()) (fun () -> f (tmpfile, tmpfile_oc))
Copy link

Choose a reason for hiding this comment

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

this leaks fd

Copy link
Contributor Author

Choose a reason for hiding this comment

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

See #15

Comment on lines +6 to +7
let inject_list = List.map inject
let project_list = List.map project
Copy link

Choose a reason for hiding this comment

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

proper replacement would be = id with correct signature

Copy link
Contributor Author

Choose a reason for hiding this comment

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

See #15

Comment on lines +144 to 145
Out_channel.with_open_text (show_path recipients_file_path) (fun oc ->
List.iter (fun line -> Printf.fprintf oc "%s\n" line) recipients_names_with_root_group)
Copy link

Choose a reason for hiding this comment

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

probably should be (have been) save_as

Copy link
Contributor Author

Choose a reason for hiding this comment

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

See #15

Comment on lines +10 to +13
let to_string exn =
match exn with
| Unix.Unix_error (e, f, s) -> sprintf "Unix_error %s(%s) %s" f s (Unix.error_message e)
| exn -> to_string exn
Copy link

Choose a reason for hiding this comment

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

this is strange way to "register" exn printer

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hmm, I'm not sure what to do here. The code was introduced here 9af2e7a#diff-17dd93d35399c2557c20172cf0d2e6dd89cdd2ac8fb085fd2fcf68f1614a54c5 by @thatportugueseguy IIUC.

I'm guessing that what you're alluding to is that we could:

let () =
  Printexc.register_printer (fun exn ->
    match exn with
    | Unix.Unix_error (e, f, s) -> Some (sprintf "Unix_error %s(%s) %s" f s (Unix.error_message e))
    | _ -> None)

at the top this module? This would give the same effect as @thatportugueseguy's code (in die) but would also affect all calls to Printexc.to_string.

Copy link
Contributor

Choose a reason for hiding this comment

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

This is a part of the Devkit exn printer's code. I removed the parts that weren't relevant for passage, and incorporated the rest in the die function, as we didn't have a use for it anywhere else.

This isn't registering the printer because it's not exposed. Do you think it should be more inlined, then?

Copy link

Choose a reason for hiding this comment

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

yes, use register_printer, affecting all the code is good obv

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done in #15


let inject x = x
let project x = x
let inject_list = List.map inject
Copy link

Choose a reason for hiding this comment

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

idem

Copy link
Contributor Author

Choose a reason for hiding this comment

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

See #15


module Secret_name = struct
include Devkit.Fresh (String) ()
type t = string
Copy link

Choose a reason for hiding this comment

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

this defeats the purpose of having Secret_name.t, previously it was abstract, now it is string alias

Copy link
Contributor Author

Choose a reason for hiding this comment

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

See #15

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