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

Fix watchexec missing files in subdirectories #1983

Merged
merged 1 commit into from
Oct 30, 2023

Conversation

itowlson
Copy link
Contributor

Fixes #1981.

Signed-off-by: itowlson <ivan.towlson@fermyon.com>
@@ -93,6 +94,7 @@ impl WatchCommand {

let spin_bin = std::env::current_exe()?;
let manifest_file = spin_common::paths::resolve_manifest_file_path(&self.app_source)?;
let manifest_file = manifest_file.absolutize()?.to_path_buf(); // or watchexec misses files in subdirectories
Copy link
Collaborator

Choose a reason for hiding this comment

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

BUT WHY

Copy link
Contributor Author

Choose a reason for hiding this comment

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

BECAUSE REASONS LANN REASONS

We could probably absolutise at the last moment before we pass the directory to watchexec - I just get paranoid about capturing the true location as early as possible (a) so I don't need to remember to do it at all the last moments and (b) in case some other piece of code sneakily switches out the current directory from underneath me. thousand yard stare

We can iterate on this for sure though.

Copy link
Collaborator

@lann lann left a comment

Choose a reason for hiding this comment

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

I'd really like to cut back on the amount of absolutizing we do as it infects logging and test output everywhere but a fix is a fix. 🤷

@itowlson itowlson merged commit d783dec into fermyon:main Oct 30, 2023
9 checks passed
This pull request was closed.
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.

Bug: spin watch broken for nested files
4 participants