robur/builder-web
4
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

remove usage of cstruct, require mirage-crypto 1.0.0 #1

Merged
reynir merged 5 commits from no-cstruct into main 2024-09-05 15:12:38 +00:00
Conversation 3 Commits 5 Files changed 20 +123 -136
hannes commented 2024-09-02 14:16:13 +00:00
Owner
Copy link

ready for review, but for compiling requires:

//cc @reynir

ready for review, but for compiling requires: - builder in main https://github.com/robur-coop/builder/pull/49 - dream alpha7 https://github.com/ocaml/opam-repository/pull/26465 - caqti with https://github.com/paurkedal/ocaml-caqti/pull/120 //cc @reynir
hannes added 1 commit 2024-09-02 14:16:14 +00:00
hannes commented 2024-09-02 14:16:48 +00:00
Author
Owner
Copy link

I also:

  • moved from pbkdf & scrypt-kdf to kdf.pbkdf, kdf.scrypt
  • moved from hex to ohex
I also: - moved from pbkdf & scrypt-kdf to kdf.pbkdf, kdf.scrypt - moved from hex to ohex
hannes added 1 commit 2024-09-02 14:38:13 +00:00
reynir approved these changes 2024-09-04 09:00:30 +00:00
reynir left a comment
Owner
Copy link

Thanks! This looks great. Pinning caqti this seems to work for me. I will merge once caqti has been released.

Thanks! This looks great. Pinning caqti this seems to work for me. I will merge once caqti has been released.
auth/builder_web_auth.ml
@ -44,3 +44,3 @@
match user_info.password_hash with
| `Pbkdf2_sha256 (password_hash, salt, params) ->
Cstruct.equal
String.equal
reynir commented 2024-09-04 06:09:43 +00:00
Owner
Copy link

Here we should probably have used Eqaf in the first place. But let's save that for another PR.

Here we should probably have used Eqaf in the first place. But let's save that for another PR.
👍 1
bin/migrations/m20210706.ml
@ -77,3 +77,3 @@
Db.collect_list builds () >>= fun builds ->
Grej.list_iter_result (fun (id, opam_sha, env_sha, pkg_sha) ->
let input_id = Mirage_crypto.Hash.SHA256.digest (Cstruct.concat [ opam_sha ; env_sha ; pkg_sha ]) in
let input_id = Digestif.SHA256.(to_raw_string (digestv_string [ opam_sha ; env_sha ; pkg_sha ])) in
reynir commented 2024-09-04 06:16:40 +00:00
Owner
Copy link

I like this! We now avoid allocating an intermediate string :D

I like this! We now avoid allocating an intermediate string :D
hannes marked this conversation as resolved
builder-web.opam Outdated
@ -24,3 +23,3 @@
"hex"
"ohex" {>= "0.2.0"}
"lwt" {>= "5.7.0"}
"caqti" {>= "2.1.1"}
reynir commented 2024-09-04 08:48:53 +00:00
Owner
Copy link

This will need to be updated once we have a caqti release.

This will need to be updated once we have a caqti release.
👍 1
hannes marked this conversation as resolved
db/builder_db.ml Outdated
@ -224,7 +224,7 @@ module Build = struct
Fpath.pp t.script
t.platform
Fmt.(Dump.option int64) t.main_binary
Fmt.(Dump.option (using Cstruct.to_string string)) t.input_id
reynir commented 2024-09-04 06:22:35 +00:00
Owner
Copy link

Reading the documentation for Fmt.using I'm confused what it does. But the new code looks good to me.

Reading the documentation for `Fmt.using` I'm confused what it does. But the new code looks good to me.
hannes marked this conversation as resolved
hannes commented 2024-09-04 16:05:10 +00:00
Author
Owner
Copy link

FWIW caqti is released https://github.com/ocaml/opam-repository/pull/26479

FWIW caqti is released https://github.com/ocaml/opam-repository/pull/26479
hannes added 1 commit 2024-09-05 14:58:29 +00:00
hannes added 2 commits 2024-09-05 15:02:21 +00:00
hannes commented 2024-09-05 15:02:59 +00:00
Author
Owner
Copy link

CI run at https://github.com/robur-coop/builder-web/runs/29735284723 looks promising. IMHO good to merge. And maybe we should cut a release to opam-repository?

CI run at https://github.com/robur-coop/builder-web/runs/29735284723 looks promising. IMHO good to merge. And maybe we should cut a release to opam-repository?
reynir merged commit 47b1759d0f into main 2024-09-05 15:12:38 +00:00
reynir deleted branch no-cstruct 2024-09-05 15:12:39 +00:00
Sign in to join this conversation.
Reviewers
No reviewers
reynir
Labels
Clear labels
No items
No labels
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
2 participants
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference: robur/builder-web#1
No description provided.
Delete branch "no-cstruct"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?