Skip to content
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.

Sign up for GitHub

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

Add bin folders of dependencies to the classpath when they're open in the workspace #2094

Merged
merged 2 commits into from
Dec 9, 2024
Merged

Add bin folders of dependencies to the classpath when they're open in the workspace #2094

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
d5bef4b
Fix issue that target folders of open dependencies were ignored
sungshik Dec 6, 2024
0da31bb
Remove redundant FIXME
sungshik Dec 9, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions src/org/rascalmpl/library/util/PathConfig.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -543,8 +543,10 @@ public static PathConfig fromSourceProjectRascalManifest(ISourceLocation manifes
switch (mode) {
case INTERPRETER:
srcsWriter.appendAll(childConfig.getSrcs());
libsWriter.append(childConfig.getBin());
Copy link
Contributor Author

@sungshik sungshik Dec 6, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

It might be conceptually nicer (?) to:

  1. Generalize the bin field from ISourceLocation to List<ISourceLocation> (and rename the field to bins)
  2. Add the bin locations from the dependencies to that list

But, I expect this will touch a lot more places in the existing code, so I thought I'd go for the solution with the smallest footprint first...

Copy link
Member

Choose a reason for hiding this comment

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

The bin folder must be in one place; this follows the target folder convention in other build systems like mvn. Also it is required to be unambiguous and unique, because this is what ends up being packaged into a runtime module like a jar or zip or doll.

Copy link
Member

Choose a reason for hiding this comment

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

I think this does the trick! Surgical precision 👍

break;
case COMPILER:
// FIXME (?): Add something to `srcsWriter` here?
libsWriter.append(setTargetScheme(projectLoc));
break;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Davy asked to be mindful of possible similar changes for COMPILER, so I added this FIXME provisionally. I'm not sure if the sources need to be explicitly added, or if this is already covered as part of the libs.

Copy link
Member

Choose a reason for hiding this comment

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

No the compiler never uses libraries from source positions. That's the main difference between interpreter and compiler mode.

Copy link
Member

Choose a reason for hiding this comment

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

That's the important feature of the Compiler to be able to work with binary dependencies. It's also important to keep a "single source of truth" , the exact bill of materials, for library dependencies: we never depend on both the source and the binary ofa library.

}
Expand Down