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.

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

Support dynamic enable/disable of classpath entries #614

Closed
wants to merge 1 commit into from

Conversation

laeubi
Copy link
Contributor

@laeubi laeubi commented Dec 25, 2022

This is WIP for:

This outlines the idea described in #613 for adding a way to mark classpath entries as disabled and add an implementation based on platform filtering.

One can then have a classpath entry like this:

<classpathentry kind="src">
    <attributes>
	    <attribute name="filter" value="(platform.os=win32)"/>
    </attributes>
</classpathentry>

What already works:

  • When adding such an attribute manually in the classpath file, the entry is disabled when not running under windows.

What is missing:

  • There is no UI yet to set a filter
  • I could not yet find out how JDT maps CPE_SOURCE entries to the actual things to compile, actually I though about this could be some special king of excluding files (that simply excludes everything) so hints would be appreciated.

Beside that I'd like to encourage everyone to participate in discussing and sharing ideas about this proposal, but please focus on the enable/disable part (the filtering might or might not be implemented in jdt directly and is just here to have a working example who such thing could be used!

@laeubi
Copy link
Contributor Author

laeubi commented Dec 25, 2022

This shows how this could be used in jdt.debug to skip disabled items for system libs:

@laeubi
Copy link
Contributor Author

laeubi commented Jan 13, 2023

@stephan-herrmann @iloveeclipse can you review or suggest someone?

@stephan-herrmann
Copy link
Contributor

@stephan-herrmann @iloveeclipse can you review or suggest someone?

From a quick look, the PR only introduces a new property, but I could not find any code using that property. So who would be calling org.eclipse.jdt.core.IClasspathEntry.isDisabled() with which effect?

Also I'm not quite sure what kind of dynamism "dynamic enable/disable..." should refer to. Wouldn't it suffice to implement "conditional computation of classpaths", i.e., once computed there is no further dynamism?

As for the split of computing vs applying a filter, I suggest to think of an analogy to classpath containers: it's a concept defined by JDT but implemented by others (like PDE, m2e). JDT doesn't claim to understand how a container is populated, but it knows that extensions can do so, to the end that JDT can then use the result for its resolved classpath. So far source folders didn't have any such need, but when JDT would define a classpath filter extension point, then any extender can mark arbitrary cp entries as invisible (this could also apply to context specific dependencies).

In eclipse-tycho/tycho/issues/632 (which I only skimmed) I also saw mentioning of the "test" flag for comparison, which is special in the regard that JDT already performs two separate builds for a project using this flag: one build for main sources, one build for test sources with visibility of main sources plus test dependencies. If we were to generalize the platform thing with the main/test distinction, then the new extension point used for filtering might need to be more complex, like:

  • listing all relevant build categories (one extender might define "main" vs. "test", another would contribute "linux" vs. "win32", etc.)
  • compute visibility of any classpath entry for any given set of categories

Then JDT could do smth like

List<Set<Category>> configurations = getAndCombineCategoriesFromExtensions(); // full cartesian product??
for (Set<Category> configuration : configurations) {
     resolvedClasspath = applyExtensionFilters(resolvedClasspath, configuration);
     build (resolvedClasspath);
}

That would cover the case of full builds, whereas incremental builds and change notification would require additional care.

I should note, that I don't propose to change the story of "test" vs "main" just yet, only using the example to steer us towards a maximally generic solution (if this can be done without unwanted complexity).

@stephan-herrmann
Copy link
Contributor

I looked at eclipse-jdt/eclipse.jdt.debug#169 only after commenting, so now I wonder, if the request is only relevant for the runtime classpath? Or compile time classpath? Or both? Should compile time apply different filtering than launching?

@laeubi
Copy link
Contributor Author

laeubi commented Jan 14, 2023

From a quick look, the PR only introduces a new property, but I could not find any code using that property. So who would be calling org.eclipse.jdt.core.IClasspathEntry.isDisabled() with which effect?

I added a call here but it is sadly a different repository:

also see this discussion:

Also I'm not quite sure what kind of dynamism "dynamic enable/disable..." should refer to. Wouldn't it suffice to implement "conditional computation of classpaths", i.e., once computed there is no further dynamism?

Dynamic means, the state is computed (once) by some code running in the IDE an not persisted e.g. in the .classpath file (but the content of that file might influence the computation) based on some properties of the host system, the IDE or whatever state, but it must not really change "dynamically" over time after it was computed.

So far source folders didn't have any such need, but when JDT would define a classpath filter extension point, then any extender can mark arbitrary cp entries as invisible (this could also apply to context specific dependencies).

Yes something might work as well thats why it is a draft at them moment, so probably an extension point would be good that is then implemented by jdt or others and finally results in a boolean true/false in the end (here I have coded it with just one example).

I should note, that I don't propose to change the story of "test" vs "main" just yet, only using the example to steer us towards a maximally generic solution (if this can be done without unwanted complexity).

This sounds very interesting, especially in the context of

where one wants each source-folder to compile with a different target-release so if you have a better, more general idea how to cover both cases that's great, I just wanted to came up with a minimal starting point so thing start to get some spin but I'm open to other approaches as long as they are approaching sometime :-)

I looked at eclipse-jdt/eclipse.jdt.debug#169 only after commenting, so now I wonder, if the request is only relevant for the runtime classpath? Or compile time classpath? Or both? Should compile time apply different filtering than launching?

For me, this would be pure "compile-time" even though of course runtime is affected if one compiles some source and others are ignored, but I think this would work without special consideration.

@stephan-herrmann
Copy link
Contributor

So if you think, an extension point might be the right thing, then let's discuss the desired protocol.

In the end, JDT needs to ask extensions, which of a project's classpath entries should be enabled, so likely there will be a method taking an IClasspathEntry and returning boolean.

Then, which information will the extension use, to compute its answer? I could imagine the following sources:

  1. the actual environment of the running Eclipse instance
  2. an extension announces several configurations, and JDT builds for each configuration in turn (so the configuration to be built should be a parameter of the method)
  3. I could also think that preferences be used to select configurations
    (a) of course the extension can always use its own preference, with no pre-planning in the extension point
    (b) also JDT could use a preference to select from a given set of configurations which one(s) should be built

For (2), where configurations must be communicated between JDT and extensions, we need to decide how a "configuration" should be represented. Is a String sufficient (who ensures uniqueness)? Or should we introduce a type IBuildConfiguration? To JDT, this type could be mostly opaque, just some getName() would be useful, should we ever have to show a configuration to the user (e.g., in progress bar "Building configuration FOO", or in preferences for en/disabling available configurations etc.). Other than that the meaning of each IBuildConfiguration would be known only to the extension (and JDT should probably remember which extension is responsible for filtering which configuration...).

Ah, so this brings us to the point, where potentially several extensions are involved, so how to handle combinations of configurations? That's where in my earlier comment I asked about the full cartesian product (one extension filters by Java version, another extension is interested in different windowing systems ...).

If the extension point controls the actual content of the resolved classpath, then several aspects of the IDE will automatically benefit from it: compilation (possible several builds), runtime (needs to select one config). OTOH, tooling features like search and all that depends on it may be at conflict with this multiverse of configurations (perhaps that's where preferences will help?), so will we need to support both views: unfiltered and filtered. That would get a bit tricky.

If the multitude of configurations gets overwhelming for tooling (and performance?), then perhaps the preferences approach is more suitable: we'd select one configuration at a time, and use it for all IDE operations from build, over refactor, to launch. We'd leave it to build tools to invoke things for several configurations in turn.

Or: a source folder could statically declare the configuration it is intended for, and then the compiler will check that it only ever uses dependencies of the same configuration.

This discussion also reminds me of how b3aggr files handle configurations (as triplets, right?), defining a list of concrete configurations for which a given aggregation should be validated.

While an extension point can provide a very powerful addition, it needs to be designed carefully, because once published the options to change it are quite limited, so we better do it right the first time.

@laeubi
Copy link
Contributor Author

laeubi commented Jan 15, 2023

I think such an extension just get the IClasspathEntry as the rest it implementation specific, if there are multiple ones, they should be and'ed together so if one think it should be disabled it must be disabled for all.

So something like:

interface ClasspathFilter {

     boolean isEnabled(IJavaProject project, IClasspathEntry entry);
  
}

I also won't assume that such a extension does performance critical operations, most of the time it would be:

  1. Check if the type of classpath entry is something that's of interest for the extension (e.g. is a a source type and has it an attribute release, or has it an attribute filter)
  2. Compare this with the system state (e.g. what is the projects JRE or does some system properties match)

and in most of the cases the check will end at step 1 because most of the time filtering is not required.

@stephan-herrmann
Copy link
Contributor

Thanks for mentioning the IJavaProject as I tend to forget that IClasspathEntry contains no back pointer.

I think such an extension just get the IClasspathEntry as the rest it implementation specific

This would certainly work for the case of filtering by the current platform.

This would not work for a use case like the main/test distinction, where multiple builds are performed, and thus the extension needs to be asked relative to a specific variant (enabled for main? enabled for test?). Perhaps this argument is irrelevant, as we already have a solution for main/test, but what about multi-release support? Doesn't this need multiple builds for the same project, too?

It would be sad if we create an extension point for today's requirement, and tomorrow we find, that it lacks just one parameter to also satisfy another relevant requirement.

BTW, multiple builds is also the reason I mentioned performance, not the time spent testing enablement :)

@laeubi
Copy link
Contributor Author

laeubi commented Jan 15, 2023

I think multiple builds should be handled differently because here I want that some item are just ignored (e.g. I want them to be handled like I already can choose exclude from classpath) and these are specific to one project and the other case is that I want some items to be handled differently (e.g. compiled with different settings), where I agree that those "different settings" are global and apply to multiple projects.

Combining both approaches seem to complicate things a lot and as you mentioned the test/main thing is already there, so one probably want to generalize that for other use-cases but at the moment it is there an likley needs other consideration if generalized.

@laeubi
Copy link
Contributor Author

laeubi commented Jan 16, 2023

I think multiple builds should be handled differently because here I want that some item are just ignored (e.g. I want them to be handled like I already can choose exclude from classpath)

After thinking more about it I'm probably on the wrong track here regarding my initial idea of enable/disable to support:

and the parts for

are better solved as you described here:

so I'll close this for now and open another proposal in favor of this for the multi-platform compile.

@HannesWell
Copy link
Contributor

The concept proposed #613 would for example be useful for the org.eclipse.swt bundle development, where we currently have three different .classpath_${os} files, one for each OS that reference different src-folders.
When provisioning a dev workspace for SWT the right one has to be renamed to .classpath.
If entries could be enabled/disabled automatically by a property this would be much more convenient.

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.

3 participants