Fix the case where there is an @ in the absolute path of sysroot and XEUS_CPP_RESOURCE_DIR in Emscripten build

[mcbarton]

Description

Please include a summary of changes, motivation and context for this PR.

I've been working around an issue with the Emscripten build for months locally. If your unlucky and happen to have a @ symbol in your absolute path for SYSROOT_PATH then it will get truncated at this point when preloading, and won't build. This recently got made worse by the introduction of XEUS_CPP_RESOURCE_DIR , which in my case also has an @ in the absolute path. This PR fixes this by creating a symlink in the build directory to these locations, avoiding a @ in the path.

Fixes # (issue)

Type of change

Please tick all options which are relevant.

• Bug fix
• New feature
• Added/removed dependencies
• Required documentation updates

[codecov-commenter]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 81.78%. Comparing base (3a888b6) to head (ffb0c4e).

Additional details and impacted files

@@           Coverage Diff           @@
##             main     #333   +/-   ##
=======================================
  Coverage   81.78%   81.78%           
=======================================
  Files          20       20           
  Lines         950      950           
  Branches       87       87           
=======================================
  Hits          777      777           
  Misses        173      173           

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[vgvassilev]

Looks reasonable to me. Let's hear from the other reviewers.

[mcbarton]

@anutosh491 @SylvainCorlay can you review this PR for me? All this PR does it create a symlink to the respective paths, in the correct locations if you happen to have an @ in the absolute paths for these directories. Once this symlink has been created, the paths are re adjusted to use a relative path through through these symlinks. Thereby avoiding the issue of path truncation when preloading the directory.

[anutosh491]

cc @SylvainCorlay what's your take on this

Here is my take

1. This looks like a very special case to me. I don't think its natural to have @ in your path (its unusual but not necessarily wrong to be fair)
2. How does one even test this before taking it in ? Like I've been tagged for a review and I don't know what to comment on (I can't test this locally)
3. I searched through emscripten's github and I don't see any issues challenging their design of using the "@" character for preloading. If it was a usual case where people are having issues with having "@" in their path, this would have been raised !

Also I'm not in favour of special handling for every other special character that every other compiler (that we might use) would introduce in future.

I personally think @mcbarton should deal with this locally. I am scared of addressing changes that aren't proven issues fundamentally/universally !

[mcbarton]

cc @SylvainCorlay what's your take on this

Here is my take

1. This looks like a very special case to me. I don't think its natural to have @ in your path (its unusual but not necessarily wrong to be fair)

2. How does one even test this before taking it in ? Like I've been tagged for a review and I don't know what to comment on (I can't test this locally)

3. I searched through emscripten's github and I don't see any issues challenging their design of using the "@" character for preloading. If it was a usual case where people are having issues with having "@" in their path, this would have been raised !

Also I'm not in favour of special handling for every other special character that every other compiler (that we might use) would introduce in future.

I personally think @mcbarton should deal with this locally. I am scared of addressing changes that aren't proven issues fundamentally/universally !

As discussed in our group chat on Discord @anutosh491 , just because an issue/bug may be non universal, doesn't mean it should be ignored (which seems to be your point 1 and point 3), when a simple solution exists (e.g. creating a symlink, so a relative path can be used). It is not as if we are not using relative paths in some cases already. I know its just about to be removed, but the XEUS_CPP_DATA_DIR uses a relative path

xeus-cpp/CMakeLists.txt

Line 27 in 3a888b6

set(XEUS_CPP_DATA_DIR "share/xeus-cpp")

xeus-cpp/CMakeLists.txt

Line 453 in 3a888b6

PUBLIC "SHELL: --preload-file ${XEUS_CPP_DATA_DIR}@/share/xeus-cpp"

With regards to you being unable to test the solution locally, I already gave you one in the Discord chat (e.g. rename the micromamba environment names to have an @ in them). With regards to addressing this locally, I already have been doing so for months, and its getting frustrating. I am having to fetch this patch whenever I want to develop a new feature for xeus-cpp (and making sure I omit the patch when adding a commit), or review a PR. I don't believe I am providing an overly complicated solution to this bug.

[anutosh491]

With regards to you being unable to test the solution locally, I already gave you one in the Discord chat (e.g. rename the micromamba environment names to have an @ in them)

Exactly my point !!!

I need to purposefully make something wrong to get this right. Which makes it unnatural.
When I mean this is a special case, I mean it doesn't appear in the wild and can't be reproduced as it is.

I truly believe if something is in master, it should atleast be a fundamental issue that should be reproducible to some level naturally (stressing on this)

already have been doing so for months, and its getting frustrating

I don't see anything wrong with this. I am convinced this is a special case and not a bug.

1. You can always maintain a patch and "git apply" that patch before building
2. You can use "git add -p" and never pick up and omit that patch while committing.

[vgvassilev]

1. This looks like a very special case to me. I don't think its natural to have @ in your path (its unusual but not necessarily wrong to be fair)

A university system already uses this. They likely use a valid setup that's not unique to that institution. If we want to position xeus-cpp-lite as a tool for learning we must fix such cases, no matter what.