fix: pass O_SHARE_DELETE when digesting files#8262
Merged
emillon merged 1 commit intoocaml:mainfrom Jul 27, 2023
Merged
Conversation
rgrinberg
reviewed
Jul 24, 2023
Member
rgrinberg
left a comment
There was a problem hiding this comment.
Can we leave the old code on Unix? Channels have mutexes that we don't really need here
Collaborator
Author
|
I'll try the other following approaches:
|
Collaborator
Author
|
Ah, even better: adding |
e3754f1 to
bccc862
Compare
Collaborator
Author
|
There's still a part I don't completely understand (why the file is closed too late in some cases) but it might be related to the details of threading and win32unix. |
hhugo
reviewed
Jul 25, 2023
nojb
approved these changes
Jul 25, 2023
Collaborator
nojb
left a comment
There was a problem hiding this comment.
LGTM. Understanding why the file is not closed in the first place would be ideal of course, but this fix looks solid.
bccc862 to
ae8941e
Compare
Collaborator
Author
|
I added a comment explaining the situation. let me know what you think. |
Fixes ocaml#8268 This flag corresponds to what `Stdlib.open_in` does. When passed, the files can be removed while they are still open. This condition can happen when background digests are enabled. This is a no-op everywhere else. This allows enabling background digests on Windows. Signed-off-by: Etienne Millon <me@emillon.org>
ae8941e to
86d940c
Compare
Collaborator
LGTM |
Collaborator
|
LGTM |
emillon
added a commit
to emillon/opam-repository
that referenced
this pull request
Jul 28, 2023
CHANGES: - Add `dune show rules` as alias of the `dune rules` command. (ocaml/dune#8000, @Alizter) - Fix `%{deps}` to expand properly in `(cat ...)` when containing 2 or more items. (ocaml/dune#8196, @Alizter) - Add `dune show installed-libraries` as an alias of the `dune installed-libraries` command. (ocaml/dune#8135, @Alizter) - Fix the `severity` of error messages sent over RPC which was missing. (ocaml/dune#8193, @Alizter) - Add `dune build --dump-gc-stats FILE` argument to dump garbage collection stats to a named file. (ocaml/dune#8072, @Alizter) - Fix bug with ppx and Reason syntax due to missing dependency in sandboxed action (ocaml/dune#7932, fixes ocaml/dune#7930, @Alizter) - Add `dune describe package-entries` to print all package entries (ocaml/dune#7480, @moyodiallo) - Improve `dune describe external-lib-deps` by adding the internal dependencies for more information. (ocaml/dune#7478, @moyodiallo) - Re-enable background file digests on Windows. The files are now open in a way that prevents race condition around deletion. (ocaml/dune#8262, fixes ocaml/dune#8268, @emillon)
emillon
added a commit
to emillon/opam-repository
that referenced
this pull request
Jul 31, 2023
CHANGES: - Add `dune show rules` as alias of the `dune rules` command. (ocaml/dune#8000, @Alizter) - Fix `%{deps}` to expand properly in `(cat ...)` when containing 2 or more items. (ocaml/dune#8196, @Alizter) - Add `dune show installed-libraries` as an alias of the `dune installed-libraries` command. (ocaml/dune#8135, @Alizter) - Fix the `severity` of error messages sent over RPC which was missing. (ocaml/dune#8193, @Alizter) - Add `dune build --dump-gc-stats FILE` argument to dump garbage collection stats to a named file. (ocaml/dune#8072, @Alizter) - Fix bug with ppx and Reason syntax due to missing dependency in sandboxed action (ocaml/dune#7932, fixes ocaml/dune#7930, @Alizter) - Add `dune describe package-entries` to print all package entries (ocaml/dune#7480, @moyodiallo) - Improve `dune describe external-lib-deps` by adding the internal dependencies for more information. (ocaml/dune#7478, @moyodiallo) - Re-enable background file digests on Windows. The files are now open in a way that prevents race condition around deletion. (ocaml/dune#8262, fixes ocaml/dune#8268, @emillon)
pmwhite
pushed a commit
to pmwhite/dune
that referenced
this pull request
Aug 10, 2023
Fixes ocaml#8268 This flag corresponds to what `Stdlib.open_in` does. When passed, the files can be removed while they are still open. This condition can happen when background digests are enabled. This is a no-op everywhere else. This allows enabling background digests on Windows. Signed-off-by: Etienne Millon <me@emillon.org>
pmwhite
pushed a commit
to pmwhite/dune
that referenced
this pull request
Aug 10, 2023
Fixes ocaml#8268 This flag corresponds to what `Stdlib.open_in` does. When passed, the files can be removed while they are still open. This condition can happen when background digests are enabled. This is a no-op everywhere else. This allows enabling background digests on Windows. Signed-off-by: Etienne Millon <me@emillon.org>
nberth
pushed a commit
to nberth/opam-repository
that referenced
this pull request
Jun 18, 2024
CHANGES: - Add `dune show rules` as alias of the `dune rules` command. (ocaml/dune#8000, @Alizter) - Fix `%{deps}` to expand properly in `(cat ...)` when containing 2 or more items. (ocaml/dune#8196, @Alizter) - Add `dune show installed-libraries` as an alias of the `dune installed-libraries` command. (ocaml/dune#8135, @Alizter) - Fix the `severity` of error messages sent over RPC which was missing. (ocaml/dune#8193, @Alizter) - Add `dune build --dump-gc-stats FILE` argument to dump garbage collection stats to a named file. (ocaml/dune#8072, @Alizter) - Fix bug with ppx and Reason syntax due to missing dependency in sandboxed action (ocaml/dune#7932, fixes ocaml/dune#7930, @Alizter) - Add `dune describe package-entries` to print all package entries (ocaml/dune#7480, @moyodiallo) - Improve `dune describe external-lib-deps` by adding the internal dependencies for more information. (ocaml/dune#7478, @moyodiallo) - Re-enable background file digests on Windows. The files are now open in a way that prevents race condition around deletion. (ocaml/dune#8262, fixes ocaml/dune#8268, @emillon)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #8268
This flag corresponds to what
Stdlib.open_indoes. When passed, the files can be removed while they are still open. This condition can happen when background digests are enabled. This is a no-op everywhere else.This allows enabling background digests on Windows.