fix(compile): do not error embedding same symlink via multiple methods

[dsherret]

Closes #27012

[irbull]

In the case of a --include in deno compile, if the same sym link is inserted twice (which can happen if a parent and child directory are both included), which path will it be found under? I guess with this code change, it will only be found under the first path, right?

[dsherret]

If you insert a parent and then insert the child, the child would overwrite the child within the parent (we could be smarter about knowing if a parent was previously inserted to skip the work, but I think it's not worth it)

[irbull]

For complex virtual file systems, you may actually want both entries. I hit this problem when trying to add a node_modules directory to a deno compile (probably not a good idea anyways). But I think that is a much bigger problem than what needs to be solved here. Thanks for the fix.

[dsherret]

I'm not sure what you mean be including both entries. Wouldn't there only be one symlink?

[irbull]

Given a file structure like this:

.
├── assets
│   └── my_static_asset.txt
├── folder
│   ├── assets -> ../assets
│   └── sub_folder
│       └── assets -> ../../assets
└── print_asset.ts

and deno compile --include folder/ --include folder/sub_folder/ print_asset.ts

Would I expect:
I could read the my_static_assets under:
import.meta.dirname + "/folder/sub_folder/assets/my_static_asset.txt"
import.meta.dirname + "/folder/assets/my_static_asset.txt"

The same symlink is found twice when that directory is recursed. I'm not sure if that works anyways.

[irbull]

I pulled down this PR and gave it a spin.

1. It does fix the Panic 🚀
2. With the structure above, it compiles but the symbolic links are not resolved if --include folder/ is provided. That is, given the structure above and:

deno compile --include folder/ print_asset.ts 

Then the following fails to resolve:

const content = await Deno.readTextFile(import.meta.dirname + "/folder/assets/my_static_asset.txt");

But this works fine if:

deno compile --include . print_asset.ts 

is used. However, this wasn't working before this change-set, so I would consider that separate issue.

That's a long way of saying 👍 by me, and I can file an another issue for the more complex layout if you'd like.

[dsherret]

In this case, you need to also include the assets directory, otherwise only the symlinks are included in the folder directory.

> $env:DENORT_BIN="V:\deno\target\debug\denort.exe" ; V:/deno/target/debug/deno compile --include folder --include folder/sub_folder print_asset.ts && ./print_asset.exe                 
Compile file:///V:/scratch/print_asset.ts to print_asset.exe
error: Uncaught (in promise) NotFound: path not found: readfile '.../folder/sub_folder/assets/my_static_asset.txt': path not found: readfile 'C:\Users\david\AppData\Local\Temp\deno-compile-print_asset.exe\scratch/folder/sub_folder/assets/my_static_asset.txt': path not found
    at Object.readTextFileSync (ext:deno_fs/30_fs.js:765:10)
    at file:///C:/Users/david/AppData/Local/Temp/deno-compile-print_asset.exe/scratch/print_asset.ts:1:18
> $env:DENORT_BIN="V:\deno\target\debug\denort.exe" ; V:/deno/target/debug/deno compile --include assets --include folder --include folder/sub_folder print_asset.ts && ./print_asset.exe
Compile file:///V:/scratch/print_asset.ts to print_asset.exe
console.log(1);

console.log(1);

[irbull]

+1. Yep, that fixes things. I'm not sure if it's obvious that the targets of the sym links need to be included, but that's probably a good thing in the end.

I wrote a small tool that prints the contents of the import.meta.dirname and with this change-set applied things are much better. 👍 from me, but maybe Bartek can give this a final review. Thanks again for the fix.

[dsherret]

I opened these issues for the future:

• List all files included in deno compile with output? #27021
• Deno compile - Warning for dangling symlink in embedded files #27023

[irbull]

I found another unrelated issue while testing this (this fails before this change-set is applied, so it shouldn't hold up this work). I filed an issue: #27024