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

Add examples of relative path to apphost-embed-install-location.md #319

Merged
merged 2 commits into from
Aug 2, 2024

Conversation

elinor-fung
Copy link
Member

@elinor-fung elinor-fung commented Jul 31, 2024

Examples of relative path and corresponding behaviour per: dotnet/sdk#42455 (comment)

cc @agocke @richlander @vitek-karas @dotnet/appmodel

`C:\dir\app.exe` or `/home/dir/app` with `AppHostRelativeDotNet` set to:
- `my_dotnet`: the app will look at `C:\dir\my_dotnet` or `/home/dir/my_dotnet`
- `./my_dotnet`: the app will look at `C:\dir\my_dotnet` or `/home/dir/my_dotnet`
- `../my_dotnet` the app will look at `C:\my_dotnet` or `/home/my_dotnet`
Copy link
Member

Choose a reason for hiding this comment

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

Thanks! It would be good to know what the behavior would be if ../../ ends up going past drive root. Is there a check for that case or its undefined?

Copy link
Member

@AaronRobinsonMSFT AaronRobinsonMSFT Aug 1, 2024

Choose a reason for hiding this comment

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

I think the meta point here is whatever the user defines as the relative path it is unmodified and we defer to the under lying OS file system APIs. I wouldn't do much more than provide an example of how we use it and state the precise semantics are at the mercy of the underlying file system.

Copy link
Member Author

Choose a reason for hiding this comment

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

I updated the text indicate that the path is written into the apphost unmodified and we simply append it to the app location and call OS APIs to determine validity / existence.

Copy link
Member

@agocke agocke left a comment

Choose a reason for hiding this comment

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

LGTM, thanks

@elinor-fung elinor-fung merged commit 272c7f8 into main Aug 2, 2024
3 checks passed
@elinor-fung elinor-fung deleted the elinor-fung-patch-1 branch August 2, 2024 00:21
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.

4 participants