Skip to content

Conversation

jurgenvinju
Copy link
Member

@jurgenvinju jurgenvinju commented May 22, 2025

  • this branch is a pure extension of another cleaner PR branch that was already merged: clean-factor-path-config
  • this branch adds basic functionality to compute file names from module names and module names from file names
  • a reusable function for computing the latest relevant source file or binary file or library file based on the PathConfig and the timestamps of the relevant files
  • the parameters for the above functions are computed from a LanguageFileConfig constructor, that explain which extensions, prefixes and filename escapes to use.
  • this code is all Rascal-independent; should work for any DSL that can be configured using a LanguageFileConfig. The default is set for Rascal's file extensions and prefixes (.tpl, $, /rascal), however.
  • this code is robust for any loc scheme; however it is important that the entries in srcs and libs are directories (so mavenized or jarified if originally a jar).
  • uses a new and robust inverse of Location::relativize: namely Location::resolve. Resolve is about finding the right prefix that a relative path belongs in. Resolve on a single location is simply outside + relative.path, but when a list of path folders is given IO::exists is used to find the right prefix and then returning the full loc as outside + relative.path.

An overview:

module name (str) to file path (loc) file path (loc) to module name (str) PathConfig field used
srcsFile srcsModule srcs
binFile binModule bin
libsFile libsModule libs

@jurgenvinju jurgenvinju self-assigned this May 22, 2025
@jurgenvinju jurgenvinju requested a review from PaulKlint May 22, 2025 12:04
@rodinaarssen
Copy link
Member

We should probably revisit #2265 after finishing this PR.

@jurgenvinju jurgenvinju requested a review from rodinaarssen May 22, 2025 12:09
@PaulKlint

This comment was marked as outdated.

Copy link

codecov bot commented May 22, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 47%. Comparing base (31f629d) to head (fe06925).

Additional details and impacted files
@@           Coverage Diff            @@
##              main   #2270    +/-   ##
========================================
  Coverage       47%     47%            
- Complexity    6476    6477     +1     
========================================
  Files          768     767     -1     
  Lines        63807   63701   -106     
  Branches      9526    9511    -15     
========================================
+ Hits         30252   30257     +5     
+ Misses       31267   31159   -108     
+ Partials      2288    2285     -3     

☔ 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.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@jurgenvinju jurgenvinju marked this pull request as draft May 26, 2025 09:18
Copy link
Member

@PaulKlint PaulKlint left a comment

Choose a reason for hiding this comment

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

Where did you move the Manifest-related code from util::Reflective to?

Copy link
Member

@PaulKlint PaulKlint left a comment

Choose a reason for hiding this comment

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

Where did you move the Manifest-related code from util::Reflective to?

@jurgenvinju jurgenvinju changed the title maintain pathconfig new reusable functions for using PathConfig May 26, 2025
@jurgenvinju
Copy link
Member Author

Where did you move the Manifest-related code from util::Reflective to?

I thought I kept it where it was? Just added @deprecated to those functions.

@jurgenvinju
Copy link
Member Author

This is ready to merge:

  • the main additions in util::PathConfig are conservative but comprehensive API to map source to binary files and back through searching in PathConfig srcs and libs
  • no changes in util::Reflective
  • some side-damage discovered while testing; spaces and newlines and types for local variables
  • accidental removal of the unused Task library (we communicated this with @anyahelene years ago already, but never took the time).

@jurgenvinju
Copy link
Member Author

@PaulKlint Where did you move the Manifest-related code from util::Reflective to?

This branch makes no changes to util::Reflective. I think we removed that code earlier because it was not used anywhere by anybody. All the Manifest and pom related code is now in PathConfig.java

@jurgenvinju jurgenvinju requested a review from toinehartman July 9, 2025 14:42
Copy link
Member

@toinehartman toinehartman left a comment

Choose a reason for hiding this comment

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

I like this. I don't really understand what motivated these changes, but I think especially the LanguageFileConfiguration will provide some nice consistency when working a lot with paths and module names.

@jurgenvinju
Copy link
Member Author

The motivation is the requirement that util::PathConfig can stand by itself, as a library module for configuring any file-based language processing tool; Notably independent of the Rascal-specific code in util::Reflective.

Without this semantics of path search and file mappings readily available, the syntax of PathConfig is a lot less useful.

To make sure these functions are useful and complete in some sense, we made sure that they could be used to implement the rascal type checker and Compiler. There is no plan to replace the current rascal implementation, however. Afaik.

@jurgenvinju jurgenvinju marked this pull request as ready for review August 11, 2025 06:27
@jurgenvinju jurgenvinju merged commit 15b0369 into main Aug 11, 2025
6 checks passed
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