Skip to content

new reusable functions for using PathConfig #2270

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

Draft
wants to merge 40 commits into
base: main
Choose a base branch
from

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
Contributor

@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.

}
data LanguageFileConfig = fileConfig(
str packageSep = "::",
str binExt = "tpl",
Copy link
Contributor

Choose a reason for hiding this comment

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

Nice! I see how this works for the current Rascal compilation scheme. How can we use this for the compiler (code generation) as well? Will that have a separate LanguageFileConfig where binExt = "java" or something?

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