-
Notifications
You must be signed in to change notification settings - Fork 409
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
Exec watch mode spawns processes with initial cwd #10262
Merged
Merged
Conversation
This file contains 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
rgrinberg
approved these changes
Mar 13, 2024
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Needs a CHANGES entry
Can you first comment on #10241 so we can merge it before this one? |
gridbugs
force-pushed
the
exec-watch-cwd-fix
branch
from
March 14, 2024 06:50
1f97cfb
to
f072bb5
Compare
Fixes a bug where commands run from `dune exec -w` would have the cwd of the project root rather than the directory where the command was run. Signed-off-by: Stephen Sherratt <stephen@sherra.tt>
gridbugs
force-pushed
the
exec-watch-cwd-fix
branch
from
March 14, 2024 06:51
f072bb5
to
1f5cc53
Compare
Leonidas-from-XIV
added a commit
to Leonidas-from-XIV/opam-repository
that referenced
this pull request
Mar 26, 2024
CHANGES: ### Added - Add link flags to to `ocamlmklib` for ctypes stubs (ocaml/dune#8784, @frejsoya) - Remove some unnecessary limitations in the expansions of percent forms in install stanza. For example, the `%{env:..}` form can be used to select files to be installed. (ocaml/dune#10160, @rgrinberg) - Allow artifact expansion percent forms (`%{cma:..}`, `%{cmo:..}`, etc.) in more contexts. Previously, they would be randomly forbidden in some fields. (ocaml/dune#10169, @rgrinberg) - Allow `%{inline_tests}` in more contexts (ocaml/dune#10191, @rgrinberg) - Remove limitations on percent forms in the `(enabled_if ..)` field of libraries (ocaml/dune#10250, @rgrinberg) - Support dialects in `dune describe pp` (ocaml/dune#10283, @emillon) - Allow defining executables or melange emit stanzas with the same name in the same folder under different contexts. (ocaml/dune#10220, @rgrinberg, @jchavarri) ### Fixed - coq: Delay Coq rule setup checks so OCaml-only packages can build in hybrid Coq/OCaml projects when `coqc` is not present. Thanks to @vzaliva for the test case and report (ocaml/dune#9845, fixes ocaml/dune#9818, @rgrinberg, @ejgallego) - Fix conditional source selection with `select` on `bigarray` in OCaml 5 (ocaml/dune#10011, @moyodiallo) - melange: fix inconsistency in virtual library implementation. Concrete modules within a virtual library can now refer to its virtual modules too (ocaml/dune#10051, fixes ocaml/dune#7104, @anmonteiro) - melange: fix a bug that would cause stale `import` paths to be emitted when moving source files within `(include_subdirs ..)` (ocaml/dune#10286, fixes ocaml/dune#9190, @anmonteiro) - Dune file formatting: output utf8 if input is correctly encoded (ocaml/dune#10113, fixes ocaml/dune#9728, @moyodiallo) - Fix expanding dependencies and locks specified in the cram stanza. Previously, they would be installed in the context of the cram test, rather than the cram stanza itself (ocaml/dune#10165, @rgrinberg) - Fix bug with `dune exec --watch` where the working directory would always be set to the project root rather than the directory where the command was run (ocaml/dune#10262, @gridbugs) - Regression fix: sign executables that are promoted into the source tree (ocaml/dune#10263, fixes ocaml/dune#9272, @emillon) - Fix crash when decoding dune-package for libraries with `(include_subdirs qualified)` (ocaml/dune#10269, fixes ocaml/dune#10264, @emillon) ### Changed - Remove the `--react-to-insignificant-changes` option. (ocaml/dune#10083, @rgrinberg)
Leonidas-from-XIV
added a commit
to Leonidas-from-XIV/opam-repository
that referenced
this pull request
Apr 3, 2024
CHANGES: ### Added - Add link flags to to `ocamlmklib` for ctypes stubs (ocaml/dune#8784, @frejsoya) - Remove some unnecessary limitations in the expansions of percent forms in install stanza. For example, the `%{env:..}` form can be used to select files to be installed. (ocaml/dune#10160, @rgrinberg) - Allow artifact expansion percent forms (`%{cma:..}`, `%{cmo:..}`, etc.) in more contexts. Previously, they would be randomly forbidden in some fields. (ocaml/dune#10169, @rgrinberg) - Allow `%{inline_tests}` in more contexts (ocaml/dune#10191, @rgrinberg) - Remove limitations on percent forms in the `(enabled_if ..)` field of libraries (ocaml/dune#10250, @rgrinberg) - Support dialects in `dune describe pp` (ocaml/dune#10283, @emillon) - Allow defining executables or melange emit stanzas with the same name in the same folder under different contexts. (ocaml/dune#10220, @rgrinberg, @jchavarri) ### Fixed - coq: Delay Coq rule setup checks so OCaml-only packages can build in hybrid Coq/OCaml projects when `coqc` is not present. Thanks to @vzaliva for the test case and report (ocaml/dune#9845, fixes ocaml/dune#9818, @rgrinberg, @ejgallego) - Fix conditional source selection with `select` on `bigarray` in OCaml 5 (ocaml/dune#10011, @moyodiallo) - melange: fix inconsistency in virtual library implementation. Concrete modules within a virtual library can now refer to its virtual modules too (ocaml/dune#10051, fixes ocaml/dune#7104, @anmonteiro) - melange: fix a bug that would cause stale `import` paths to be emitted when moving source files within `(include_subdirs ..)` (ocaml/dune#10286, fixes ocaml/dune#9190, @anmonteiro) - Dune file formatting: output utf8 if input is correctly encoded (ocaml/dune#10113, fixes ocaml/dune#9728, @moyodiallo) - Fix expanding dependencies and locks specified in the cram stanza. Previously, they would be installed in the context of the cram test, rather than the cram stanza itself (ocaml/dune#10165, @rgrinberg) - Fix bug with `dune exec --watch` where the working directory would always be set to the project root rather than the directory where the command was run (ocaml/dune#10262, @gridbugs) - Regression fix: sign executables that are promoted into the source tree (ocaml/dune#10263, fixes ocaml/dune#9272, @emillon) - Fix crash when decoding dune-package for libraries with `(include_subdirs qualified)` (ocaml/dune#10269, fixes ocaml/dune#10264, @emillon) ### Changed - Remove the `--react-to-insignificant-changes` option. (ocaml/dune#10083, @rgrinberg)
gridbugs
added a commit
to gridbugs/dune
that referenced
this pull request
Apr 4, 2024
This fixes a bug where running an executable from the current project with `dune exec --watch` would be unable to find the executable unless the command was run from the project's root directory. The problem was introduced when we started setting the cwd of processes spawned by exec in watch mode to the user's current directory (ocaml#10262). If the program argument to exec refers to a file to be built in the current project (such as an executable implemented in the current project) then the path to the executable to spawn will be a path inside the _build directory relative to the project root. Since the cwd of the new process was set to the user's current directory, this relative path was being resolved within the current directory, whereas it should have been resolved relative to the project root. The fix was to convert relative paths into absolute paths relative to the project root (this was already being done for exec when not in watch mode).
gridbugs
added a commit
to gridbugs/dune
that referenced
this pull request
Apr 4, 2024
This fixes a bug where running an executable from the current project with `dune exec --watch` would be unable to find the executable unless the command was run from the project's root directory. The problem was introduced when we started setting the cwd of processes spawned by exec in watch mode to the user's current directory (ocaml#10262). If the program argument to exec refers to a file to be built in the current project (such as an executable implemented in the current project) then the path to the executable to spawn will be a path inside the _build directory relative to the project root. Since the cwd of the new process was set to the user's current directory, this relative path was being resolved within the current directory, whereas it should have been resolved relative to the project root. The fix was to convert relative paths into absolute paths relative to the project root (this was already being done for exec when not in watch mode). Signed-off-by: Stephen Sherratt <stephen@sherra.tt>
gridbugs
pushed a commit
to gridbugs/dune
that referenced
this pull request
Apr 5, 2024
This fixes a bug where running an executable from the current project with `dune exec --watch` would be unable to find the executable unless the command was run from the project's root directory. The problem was introduced when we started setting the cwd of processes spawned by exec in watch mode to the user's current directory (ocaml#10262). If the program argument to exec refers to a file to be built in the current project (such as an executable implemented in the current project) then the path to the executable to spawn will be a path inside the _build directory relative to the project root. Since the cwd of the new process was set to the user's current directory, this relative path was being resolved within the current directory, whereas it should have been resolved relative to the project root. The fix was to convert relative paths into absolute paths relative to the project root (this was already being done for exec when not in watch mode). Signed-off-by: Stephen Sherratt <stephen@sherra.tt>
gridbugs
pushed a commit
to gridbugs/dune
that referenced
this pull request
Apr 5, 2024
This fixes a bug where running an executable from the current project with `dune exec --watch` would be unable to find the executable unless the command was run from the project's root directory. The problem was introduced when we started setting the cwd of processes spawned by exec in watch mode to the user's current directory (ocaml#10262). If the program argument to exec refers to a file to be built in the current project (such as an executable implemented in the current project) then the path to the executable to spawn will be a path inside the _build directory relative to the project root. Since the cwd of the new process was set to the user's current directory, this relative path was being resolved within the current directory, whereas it should have been resolved relative to the project root. The fix was to convert relative paths into absolute paths relative to the project root (this was already being done for exec when not in watch mode). Signed-off-by: Stephen Sherratt <stephen@sherra.tt>
gridbugs
pushed a commit
to gridbugs/dune
that referenced
this pull request
May 29, 2024
This fixes a bug where running an executable from the current project with `dune exec --watch` would be unable to find the executable unless the command was run from the project's root directory. The problem was introduced when we started setting the cwd of processes spawned by exec in watch mode to the user's current directory (ocaml#10262). If the program argument to exec refers to a file to be built in the current project (such as an executable implemented in the current project) then the path to the executable to spawn will be a path inside the _build directory relative to the project root. Since the cwd of the new process was set to the user's current directory, this relative path was being resolved within the current directory, whereas it should have been resolved relative to the project root. The fix was to convert relative paths into absolute paths relative to the project root (this was already being done for exec when not in watch mode). Signed-off-by: Stephen Sherratt <stephen@sherra.tt>
gridbugs
pushed a commit
to gridbugs/dune
that referenced
this pull request
Jun 4, 2024
This fixes a bug where running an executable from the current project with `dune exec --watch` would be unable to find the executable unless the command was run from the project's root directory. The problem was introduced when we started setting the cwd of processes spawned by exec in watch mode to the user's current directory (ocaml#10262). If the program argument to exec refers to a file to be built in the current project (such as an executable implemented in the current project) then the path to the executable to spawn will be a path inside the _build directory relative to the project root. Since the cwd of the new process was set to the user's current directory, this relative path was being resolved within the current directory, whereas it should have been resolved relative to the project root. The fix was to convert relative paths into absolute paths relative to the project root (this was already being done for exec when not in watch mode). Signed-off-by: Stephen Sherratt <stephen@sherra.tt>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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 a bug where commands run from
dune exec -w
would have the cwd of the project root rather than the directory where the command was run.Fixes #10236
I haven't added any tests as a good test is added in #10241.