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

pkg: use our unpack code for rsync urls #10556

Merged
merged 3 commits into from
May 23, 2024
Merged

Conversation

emillon
Copy link
Collaborator

@emillon emillon commented May 22, 2024

This removes an important use of fetch_others.

(the test was printing the expected hash instead of the computed one)

Copy link
Member

@rgrinberg rgrinberg left a comment

Choose a reason for hiding this comment

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

LGTM.

Note that we should probably explicitly error on URL's of the form rsync://. We don't yet properly support using rsync.

This removes an important use of `fetch_others`.

(the test was printing the expected hash instead of the computed one)

Signed-off-by: Etienne Millon <me@emillon.org>
Signed-off-by: Etienne Millon <me@emillon.org>
Signed-off-by: Etienne Millon <me@emillon.org>
@emillon
Copy link
Collaborator Author

emillon commented May 23, 2024

Thanks. I added some checks for this and unpack.

@emillon emillon merged commit 1dd8383 into ocaml:main May 23, 2024
25 of 28 checks passed
@emillon emillon deleted the pkg-fetch-local branch May 23, 2024 08:47
MA0010 pushed a commit to MA0010/dune that referenced this pull request Jun 5, 2024
* pkg: use our unpack code for rsync urls

This removes an important use of `fetch_others`.

(the test was printing the expected hash instead of the computed one)

Signed-off-by: Etienne Millon <me@emillon.org>

* refactor: use check_checksum in the download case

Signed-off-by: Etienne Millon <me@emillon.org>

* Add defensive code

Signed-off-by: Etienne Millon <me@emillon.org>

---------

Signed-off-by: Etienne Millon <me@emillon.org>
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.

None yet

2 participants