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

Exec watch mode spawns processes with initial cwd #10262

Merged
merged 1 commit into from
Mar 14, 2024

Conversation

gridbugs
Copy link
Collaborator

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.

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.

Needs a CHANGES entry

@emillon
Copy link
Collaborator

emillon commented Mar 13, 2024

Can you first comment on #10241 so we can merge it before this one?

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 gridbugs force-pushed the exec-watch-cwd-fix branch from f072bb5 to 1f5cc53 Compare March 14, 2024 06:51
@gridbugs gridbugs merged commit 10fdc5d into ocaml:main Mar 14, 2024
25 of 27 checks passed
@gridbugs gridbugs deleted the exec-watch-cwd-fix branch March 14, 2024 06:52
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
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

dune exec path shouldn't change when using watch mode
3 participants