Skip to content

Conversation

@johroj
Copy link

@johroj johroj commented Dec 6, 2025

This PR ensures unix-style path separators are used everywhere within CodeGeneration. This solves at least two issues:

  1. Generated files like include("a\\b.jl") when created on Windows, making the result non-portable (include's in hierarchic proto definitions use static path separator #284).
  2. If a file was both in relative_paths and included from another file, these could be considered as different files due to different path separators. This would lead to duplicated includes in the generated package file.

These changes fixes so that "/" is used as a path separator in all internals and outputs of CodeGeneration. The basic assumption is that everything already works correctly on linux, so the behaviour on windows is made more like linux through the following steps:

  • Converting all input to protojl into unix-style paths.
  • Using the functions joinpath_unix, relpath_unix and normpath_unix everywhere in CodeGeneration.jl. These behave exactly like the Base counterparts on unix systems, but use unix separators also on windows. This helps make code generation independent of the platform.
  • An additional check when writing the include statements ensuring that the path is free from windows path separators.

In order to minimize the risk that this issue appears again in the future, calling joinpath, normpath or relpath within CodeGeneration will now result in a helpful error message. To get the normal platform-dependent behavior, Base.<function> is still possible.

In all cases where the unit checks that the output contains an include, the included path has now been hard-coded to use / as a separator. This means that we now test for the same result regardless of which platform the tests run on. (Related info: Base.jl uses / as path separators, so I cannot see any reason why this should not be safe).

@codecov
Copy link

codecov bot commented Dec 6, 2025

Codecov Report

❌ Patch coverage is 91.66667% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 93.70%. Comparing base (e492385) to head (a6b0346).
⚠️ Report is 4 commits behind head on master.

Files with missing lines Patch % Lines
src/codegen/modules.jl 85.71% 3 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master     #286      +/-   ##
==========================================
- Coverage   93.99%   93.70%   -0.30%     
==========================================
  Files          25       25              
  Lines        3014     3033      +19     
==========================================
+ Hits         2833     2842       +9     
- Misses        181      191      +10     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@johroj
Copy link
Author

johroj commented Dec 8, 2025

See discussions in #284

@johroj
Copy link
Author

johroj commented Feb 1, 2026

Replaced by another pull request.

@johroj johroj closed this Feb 1, 2026
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.

1 participant