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

Issue #10164 - MetaInfConfiguration Overhaul #10816

Closed
wants to merge 12 commits into from
Closed

Issue #10164 - MetaInfConfiguration Overhaul #10816

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
26a65f8
Issue #10164 - improve MetaInfConfiguration use of Resource
joakime Jul 28, 2023
59d3e12
Fully compilable overhaul.
joakime Aug 4, 2023
2118fc3
Merge remote-tracking branch 'origin/jetty-12.0.x' into fix/12.0.x/me…
joakime Aug 9, 2023
322c533
Merge remote-tracking branch 'origin/jetty-12.0.x' into fix/12.0.x/me…
joakime Oct 23, 2023
a30d0f2
Fixes #10164 - Reduce mounts as a result of MetaInfConfiguration exec…
joakime Oct 23, 2023
bd022ee
Merge remote-tracking branch 'origin/jetty-12.0.x' into fix/12.0.x/me…
joakime Oct 31, 2023
6469f4e
Fixes #10164 - cleanup unused methods
joakime Nov 1, 2023
ad9e358
Fixes #10164 - ensure only valid URIs are added to WebAppClassLoader
joakime Nov 1, 2023
47417b0
Merge remote-tracking branch 'origin/jetty-12.0.x' into fix/12.0.x/me…
joakime Nov 9, 2023
dfe7ae7
Revert NullMap
joakime Nov 10, 2023
77f63e4
Fix java.lang.StackOverflowError
joakime Nov 10, 2023
c1f997a
Cleanup slf4j usage with newer paxexam
joakime Nov 10, 2023
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
23 changes: 23 additions & 0 deletions jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/URIUtil.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@

import java.io.File;
import java.io.IOException;
import java.net.MalformedURLException;
import java.net.URI;
import java.net.URL;
import java.net.URLClassLoader;
Expand Down Expand Up @@ -1841,6 +1842,28 @@ public static URI toURI(String resource)
: Paths.get(resource).toUri();
}

/**
* <p>Convert a URI to a URL without checked exceptions</p>
*
* @param uri the URI to convert from
* @return The {@link URL} of the URI
* @throws RuntimeException if unable to convert the URI to URL
* @see URI#toURL()
*/
public static URL toURL(URI uri)
Copy link
Contributor

Choose a reason for hiding this comment

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

What's the motivation for changing the checked exception into a runtime exception?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So it can be used in a stream

{
Objects.requireNonNull(uri);

try
{
return uri.toURL();
}
catch (MalformedURLException e)
{
throw new RuntimeException(e);
}
}

/**
* <p>
* Unwrap a URI to expose its container path reference.
Expand Down
6 changes: 3 additions & 3 deletions ...annotations/src/main/java/org/eclipse/jetty/ee10/annotations/AnnotationConfiguration.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -692,7 +692,7 @@ protected Resource getJarFor(WebAppContext context, ServletContainerInitializer
}

/**
* Check to see if the ServletContainerIntializer loaded via the ServiceLoader came
* Check to see if the ServletContainerInitializer loaded via the ServiceLoader came
* from a jar that is excluded by the fragment ordering. See ServletSpec 3.0 p.85.
*
* @param context the context for the jars
Expand Down Expand Up @@ -754,7 +754,7 @@ public boolean isFromExcludedJar(WebAppContext context, ServletContainerInitiali
boolean included = false;
for (Resource r : orderedJars)
{
included = r.equals(sciResource);
included = r.isContainedIn(sciResource);
Copy link
Contributor

Choose a reason for hiding this comment

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

See comment below.

if (included)
break;
}
Expand Down Expand Up @@ -968,7 +968,7 @@ else if (entry.getValue() == null) //can't work out provenance of SCI, so can't
{
for (Map.Entry<ServletContainerInitializer, Resource> entry : sciResourceMap.entrySet())
{
if (webInfJar.equals(entry.getValue()))
if (webInfJar.isContainedIn(entry.getValue()))
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't understand this change. We're checking here if the resource that contains the SCI is the webInfJar that we're looking at. Makes no sense to check containment.

nonExcludedInitializers.add(entry.getKey());
}
}
Expand Down
2 changes: 1 addition & 1 deletion ...y-ee10-annotations/src/main/java/org/eclipse/jetty/ee10/annotations/AnnotationParser.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -550,7 +550,7 @@ public void parse(final Set<? extends Handler> handlers, Resource r) throws Exce
if (!r.exists())
return;

if (FileID.isJavaArchive(r.getPath()))
if (FileID.isJavaArchive(r.getPath())) // TODO: this is now always false, as all Resource objects are directories
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't understand the TODO comment. FileID just checks if the path ends in ".jar", so this should be fine?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The resource being passed in is never a raw path to a jar file in this method.
Even if the underlying resource was a JAR, by the time it reaches here the Resource a mounted JAR already, meaning r.getPath() is just a directory inside the JAR.

This entire method parse(handlers , r) is basically a no-op. nothing happens.

Copy link
Contributor

Choose a reason for hiding this comment

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

You mean that after these changes `parse(handlers, r) is a no-op, because it certainly isn't now.

{
parseJar(handlers, r);
return;
Expand Down
13 changes: 6 additions & 7 deletions ...n-plugin/src/main/java/org/eclipse/jetty/ee10/maven/plugin/MavenMetaInfConfiguration.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,16 +44,15 @@ public Class<? extends Configuration> replaces()
}

/**
* Get the jars to examine from the files from which we have
* Get the webapp paths (jars &amp; dirs) to examine from the files from which we have
* synthesized the classpath. Note that the classpath is not
* set at this point, so we cannot get them from the classpath.
*
* @param context the web app context
* @return the list of jars found
* @return the list of webapp resources found
*/
@Override
protected List<Resource> findJars(WebAppContext context)
throws Exception
protected List<Resource> getWebAppPaths(WebAppContext context) throws Exception
Copy link
Contributor

Choose a reason for hiding this comment

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

Not sure that getWebAppPaths is any better than findJars. The method returns Resource not Path.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, the name can be changed, but its basically taking a WebAppContext and returning a List of Resource objects that all point to directories (either in the raw, or mounted)

Copy link
Contributor

Choose a reason for hiding this comment

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

Mmmm, not sure that characterisation is correct - I'm not at all convinced that we should be changing file:/some/where/something.jar into jar:file:/some/where/something.jar!/.

{
List<Resource> list = new ArrayList<>();
MavenWebAppContext jwac = (MavenWebAppContext)context;
Expand All @@ -77,7 +76,7 @@ protected List<Resource> findJars(WebAppContext context)
});
}

List<Resource> superList = super.findJars(context);
List<Resource> superList = super.getWebAppPaths(context);
if (superList != null)
list.addAll(superList);
return list;
Expand All @@ -87,7 +86,7 @@ protected List<Resource> findJars(WebAppContext context)
* Add in the classes dirs from test/classes and target/classes
*/
@Override
protected List<Resource> findClassDirs(WebAppContext context) throws Exception
protected List<Resource> findClassesDirs(WebAppContext context) throws Exception
{
List<Resource> list = new ArrayList<>();

Expand All @@ -113,7 +112,7 @@ protected List<Resource> findClassDirs(WebAppContext context) throws Exception
});
}

List<Resource> classesDirs = super.findClassDirs(context);
List<Resource> classesDirs = super.findClassesDirs(context);
if (classesDirs != null)
list.addAll(classesDirs);
return list;
Expand Down
24 changes: 23 additions & 1 deletion ...en-plugin/src/main/java/org/eclipse/jetty/ee10/maven/plugin/MavenWebInfConfiguration.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
import org.eclipse.jetty.ee10.webapp.WebAppClassLoader;
import org.eclipse.jetty.ee10.webapp.WebAppContext;
import org.eclipse.jetty.ee10.webapp.WebInfConfiguration;
import org.eclipse.jetty.util.URIUtil;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

Expand Down Expand Up @@ -59,7 +60,28 @@ public void configure(WebAppContext context) throws Exception
LOG.debug("Setting up classpath ...");
for (URI uri : jwac.getClassPathUris())
{
loader.addClassPath(uri.toASCIIString());
// Not all Resource types supported by Jetty can be supported by WebAppClassLoader
Copy link
Contributor

Choose a reason for hiding this comment

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

Motivation for this change? There haven't been any reported issues with the jetty maven plugin AFAIK.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This builds a URLClassLoader, which is limited in ability to only working with URLs.
No point putting everything and the kitchen sink supported by Jetty into this.
We are encountering more and more java FileSystem support in Maven, this moves the error from being relatively useless URL errors from URLClassLoader when it cannot open the instance, to reporting that it's unsupported by the Maven Plugin.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In fact, this change was directly motivated by folks using jar:file://// urls here.
Something that the URLClassLoader doesn't support.

String scheme = uri.getScheme();
if (scheme == null || scheme.equals("file"))
{
// no scheme? or "file" scheme, assume it is just a path.
loader.addClassPath(uri.getPath());
continue;
}

if (scheme.equals("jar"))
{
URI container = URIUtil.unwrapContainer(uri);
if (container.getScheme().equals("file"))
{
// Just add a reference to the
loader.addClassPath(container.getPath());
continue;
}
}
Comment on lines +72 to +81
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this converting things like "jar:file:/usr/jetty/lib/somedependency.jar!/" back to just "/usr/jetty/lib/somedependency.jar"?

Are we to assume that we will never get anything like: "jar:file:/usr/jetty/lib/somedependency.jar!/WEB-INF/classes" or "jar:file:/usr/jetty/lib/somedependency.jar!/WEB-INF/lib/some.jar" ?

Copy link
Contributor

Choose a reason for hiding this comment

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

@janbartel Can you give us a list of all the different uris that this method might see and what it is meant to do with each of them? i.e. what is "the brief" of this method?


// Anything else is a warning
LOG.warn("Skipping unsupported URI on ClassPath: {}", uri);
}
}

Expand Down
86 changes: 39 additions & 47 deletions ...10-osgi-boot/src/main/java/org/eclipse/jetty/ee10/osgi/boot/OSGiMetaInfConfiguration.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,16 +89,18 @@ public void preConfigure(final WebAppContext context) throws Exception
}

@Override
protected void scanJars(final WebAppContext context) throws Exception
protected List<Resource> getContainerPaths(WebAppContext context)
Copy link
Contributor

Choose a reason for hiding this comment

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

Returns a Resource not a Path, so name is misleading.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Returns a list of Resource directories.
Could have been a CombinedResource, but then the streaming later doesn't work.

Copy link
Contributor

Choose a reason for hiding this comment

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

No, it can't be a CombinedResource, that is something different entirely and not applicable here. A CombinedResource is not a list of Resources. The name should refer to 'Resource' rather than 'Path'.

throws Exception
{
//Check to see if there have been any bundle symbolic names added of bundles that should be
//regarded as being on the container classpath, and scanned for fragments, tlds etc etc.
//This can be defined in:
// 1. SystemProperty SYS_PROP_TLD_BUNDLES
// 2. DeployerManager.setContextAttribute CONTAINER_BUNDLE_PATTERN
// Check to see if there have been any bundle symbolic names added of bundles that should be
// regarded as being on the container classpath, and scanned for fragments, tlds etc etc.
// This can be defined in:
// 1. SystemProperty SYS_PROP_TLD_BUNDLES
// 2. DeployerManager.setContextAttribute CONTAINER_BUNDLE_PATTERN
String tmp = (String)context.getAttribute(CONTAINER_BUNDLE_PATTERN);
Pattern pattern = (tmp == null ? null : Pattern.compile(tmp));
List<String> names = new ArrayList<String>();
List<String> names = new ArrayList<>();

tmp = System.getProperty(SYS_PROP_TLD_BUNDLES);
if (tmp != null)
{
Expand All @@ -109,6 +111,8 @@ protected void scanJars(final WebAppContext context) throws Exception
}
}

ResourceFactory resourceFactory = ResourceFactory.of(context);

HashSet<Resource> matchingResources = new HashSet<>();
if (!names.isEmpty() || pattern != null)
{
Expand All @@ -124,23 +128,19 @@ protected void scanJars(final WebAppContext context) throws Exception
if (pattern.matcher(bundle.getSymbolicName()).matches())
{
//get the file location of the jar and put it into the list of container jars that will be scanned for stuff (including tlds)
matchingResources.addAll(getBundleAsResource(ResourceFactory.of(context), bundle));
matchingResources.addAll(getBundleAsResource(resourceFactory, bundle));
}
}
if (names != null)
{
//if there is an explicit bundle name, then check if it matches
if (names.contains(bundle.getSymbolicName()))
matchingResources.addAll(getBundleAsResource(ResourceFactory.of(context), bundle));
matchingResources.addAll(getBundleAsResource(resourceFactory, bundle));
}
}
}
for (Resource r : matchingResources)
{
context.getMetaData().addContainerResource(r);
Copy link
Contributor

Choose a reason for hiding this comment

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

Where are the container jars now added to context metadata?

Copy link
Contributor Author

@joakime joakime Nov 10, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

in MetaInfConfiguration.preConfigure()

}

super.scanJars(context);
Copy link
Contributor

Choose a reason for hiding this comment

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

Don't we still need to call super.getContainerPaths()?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Apparently not needed as the OSGIMetaInfConfiguration.getContainerPaths() returns the same entries via bundles instead.

Copy link
Contributor

Choose a reason for hiding this comment

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

No I don't think so. We are handling both stuff that is on the normal java path and stuff that is deployed as a bundle.

return matchingResources.stream().toList();
}

@Override
Expand All @@ -151,18 +151,12 @@ public void postConfigure(WebAppContext context) throws Exception
super.postConfigure(context);
}

/**
* Consider the fragment bundles associated with the bundle of the webapp being deployed.
*
* @see org.eclipse.jetty.ee10.webapp.MetaInfConfiguration#findJars(org.eclipse.jetty.ee10.webapp.WebAppContext)
*/
@Override
protected List<Resource> findJars(WebAppContext context)
throws Exception
protected List<Resource> getWebAppPaths(WebAppContext context) throws Exception
Copy link
Contributor

Choose a reason for hiding this comment

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

Method returns a Resource not a Path, so new name is misleading.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

it's not findJars either, how about getWebAppDirs instead?

Copy link
Contributor

Choose a reason for hiding this comment

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

It's not a straight getter, it's doing some work, so that's why I'm ok with the find prefix. We need some word that indicates we're resolving jars that should behave as if they're in WEB-INF/lib (can either be in there explicitly, or on the extraClasspath).

{
List<Resource> mergedResources = new ArrayList<Resource>();
//get jars from WEB-INF/lib if there are any
List<Resource> webInfJars = super.findJars(context);
// get jars from WEB-INF/lib if there are any
List<Resource> webInfJars = super.getWebAppPaths(context);
if (webInfJars != null)
mergedResources.addAll(webInfJars);

Expand Down Expand Up @@ -215,8 +209,8 @@ protected List<Resource> findJars(WebAppContext context)
@Override
public void configure(WebAppContext context) throws Exception
{
TreeMap<String, Resource> prependedResourcesPath = new TreeMap<String, Resource>();
TreeMap<String, Resource> appendedResourcesPath = new TreeMap<String, Resource>();
TreeMap<String, Resource> prependedResourcesPath = new TreeMap<>();
TreeMap<String, Resource> appendedResourcesPath = new TreeMap<>();

Bundle bundle = (Bundle)context.getAttribute(OSGiWebappConstants.JETTY_OSGI_BUNDLE);
if (bundle != null)
Expand All @@ -225,25 +219,21 @@ public void configure(WebAppContext context) throws Exception
Set<Bundle> fragments = (Set<Bundle>)context.getAttribute(FRAGMENT_AND_REQUIRED_BUNDLES);
if (fragments != null && !fragments.isEmpty())
{
ResourceFactory resourceFactory = ResourceFactory.of(context);
// sorted extra resource base found in the fragments.
// the resources are either overriding the resourcebase found in the
// web-bundle
// or appended.
// amongst each resource we sort them according to the alphabetical
// order
// of the name of the internal folder and the symbolic name of the
// fragment.
// the resources are either overriding the base resource base in the web-bundle or appended.
// amongst each resource we sort them according to the alphabetical order
// of the name of the internal folder and the symbolic name of the fragment.
// this is useful to make sure that the lookup path of those
// resource base defined by fragments is always the same.
// This natural order could be abused to define the order in which
// the base resources are
// looked up.
// the base resources are looked up.
for (Bundle frag : fragments)
{
String path = Util.getManifestHeaderValue(OSGiWebappConstants.JETTY_WAR_FRAGMENT_RESOURCE_PATH, frag.getHeaders());
convertFragmentPathToResource(ResourceFactory.of(context), path, frag, appendedResourcesPath);
convertFragmentPathToResource(resourceFactory, path, frag, appendedResourcesPath);
path = Util.getManifestHeaderValue(OSGiWebappConstants.JETTY_WAR_PREPEND_FRAGMENT_RESOURCE_PATH, frag.getHeaders());
convertFragmentPathToResource(ResourceFactory.of(context), path, frag, prependedResourcesPath);
convertFragmentPathToResource(resourceFactory, path, frag, prependedResourcesPath);
}
if (!appendedResourcesPath.isEmpty())
{
Expand All @@ -263,13 +253,13 @@ public void configure(WebAppContext context) throws Exception

super.configure(context);

// place the prepended resources at the beginning of the contexts's resource base
// place the prepended resources at the beginning of the context's resource base
if (!prependedResourcesPath.isEmpty())
{
Resource[] resources = new Resource[1 + prependedResourcesPath.size()];
System.arraycopy(prependedResourcesPath.values().toArray(new Resource[prependedResourcesPath.size()]), 0, resources, 0, prependedResourcesPath.size());
resources[resources.length - 1] = context.getBaseResource();
context.setBaseResource(ResourceFactory.combine(resources));
List<Resource> mergedResources = new ArrayList<>();
mergedResources.addAll(prependedResourcesPath.values());
mergedResources.add(context.getBaseResource());
context.setBaseResource(ResourceFactory.combine(mergedResources));
}
}

Expand All @@ -289,24 +279,26 @@ private List<Resource> getBundleAsResource(ResourceFactory resourceFactory, Bund
{
if (FileID.isJavaArchive(f.getName()) && f.isFile())
{
resources.add(resourceFactory.newResource(f.toPath()));
// add *.jar as jar files
resources.add(resourceFactory.newJarFileResource(f.toPath().toUri()));
Copy link
Contributor

Choose a reason for hiding this comment

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

Not sure you should change the Resource that is being returned into one that looks inside the jar file, rather than just referring to the jar itself.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

constantly opening and closing the jar is WAY slower.
this also reduces the number of mounted jars by the time the webappcontext is started.

Copy link
Contributor

Choose a reason for hiding this comment

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

Not sure why we would be constantly opening and closing the jar? We should resolve these jars, then convert to java:file: url, then open each one only once and enumerate their contents looking for resources, fragments, tlds.

}
else if (f.isDirectory() && f.getName().equals("lib"))
{
for (File f2 : file.listFiles())
for (File libFile : file.listFiles())
{
if (FileID.isJavaArchive(f2.getName()) && f2.isFile())
if (FileID.isJavaArchive(libFile.getName()) && libFile.isFile())
{
resources.add(resourceFactory.newResource(f.toPath()));
// add lib/*.jar as jar files
resources.add(resourceFactory.newJarFileResource(f.toPath().toUri()));
Copy link
Contributor

Choose a reason for hiding this comment

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

Again, not sure that you should change the returned Resource into one that looks inside the jar file rather than just representing the jar file itself.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Again, too slow.

This is like all of our old code that constantly converted from Strings to File to Strings to URI to Strings to File to URL to Strings to URI.

Keep it how it will be used.

Copy link
Contributor

Choose a reason for hiding this comment

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

But you don't know how it will be used.

}
}
}
}
resources.add(resourceFactory.newResource(file.toPath())); //TODO really???
}
else
{
resources.add(resourceFactory.newResource(file.toPath()));
// Treat bundle as jar file that needs to be opened (so that resources within it can be found)
resources.add(resourceFactory.newJarFileResource(file.toPath().toUri()));
Copy link
Contributor

Choose a reason for hiding this comment

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

See previous comments about this.

}

return resources;
Expand Down
30 changes: 4 additions & 26 deletions ...sgi/test-jetty-ee10-osgi/src/test/java/org/eclipse/jetty/ee10/osgi/test/TestOSGiUtil.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -132,39 +132,17 @@ public static void coreJettyDependencies(List<Option> res)
res.add(systemProperty("org.ops4j.pax.url.mvn.settings").value(System.getProperty("settingsFilePath")));
}

res.add(mavenBundle().groupId("org.slf4j").artifactId("slf4j-api").versionAsInProject().noStart());

/*
* Jetty 11 uses slf4j 2.0.0 by default, however we want to test with slf4j 1.7.30 for backwards compatibility.
* To do that, we need to use slf4j-simple as the logging implementation. We make a simplelogger.properties
* file available so that jetty logging can be configured
*/
// BEGIN - slf4j 1.7.x
/* slf4j-simple conflicts with both slf4j 1.7.x, and jetty-slf4j-impl. (but in different ways)

TinyBundle simpleLoggingPropertiesBundle = TinyBundles.bundle();
simpleLoggingPropertiesBundle.add("simplelogger.properties", ClassLoader.getSystemResource("simplelogger.properties"));
simpleLoggingPropertiesBundle.set(Constants.BUNDLE_SYMBOLICNAME, "simple-logger-properties");
simpleLoggingPropertiesBundle.set(Constants.FRAGMENT_HOST, "slf4j-simple");
simpleLoggingPropertiesBundle.add(FragmentActivator.class);
res.add(CoreOptions.streamBundle(simpleLoggingPropertiesBundle.build()).noStart());
res.add(mavenBundle().groupId("org.slf4j").artifactId("slf4j-simple").versionAsInProject().noStart());
*/
// END - slf4j 1.7.x

/*
* When running with slf4j >= 2.0.0, remove the slf4j simple logger above and uncomment the following lines
*/
// BEGIN - slf4j 2.x
/* SLF4J 2.0 */
// slf4j-api comes from paxexam now.
// res.add(mavenBundle().groupId("org.slf4j").artifactId("slf4j-api").versionAsInProject().noStart());
Copy link
Member

Choose a reason for hiding this comment

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

it's probably not sync with ee9 :(
btw I noticed some issues recently and logging was a pain...
I'd like to extract some parts here which can be common and if possible really fix the logging

Copy link
Contributor

Choose a reason for hiding this comment

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

@olamy but not in this PR, which is only for changes to MetaInfConfiguration.

Copy link
Member

Choose a reason for hiding this comment

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

@janbartel sure but my point is either we don't change anything in this PR for this or we keep both files in sync (because something tell me we will forget :) ) BTW they are probably already not in sync anymore.

TinyBundle loggingPropertiesBundle = TinyBundles.bundle();
loggingPropertiesBundle.add("jetty-logging.properties", ClassLoader.getSystemResource("jetty-logging.properties"));
loggingPropertiesBundle.set(Constants.BUNDLE_SYMBOLICNAME, "jetty-logging-properties");
loggingPropertiesBundle.set(Constants.FRAGMENT_HOST, "org.eclipse.jetty.logging");
loggingPropertiesBundle.add(FragmentActivator.class);
res.add(CoreOptions.streamBundle(loggingPropertiesBundle.build()).noStart());
res.add(mavenBundle().groupId("org.eclipse.jetty").artifactId("jetty-slf4j-impl").versionAsInProject().start());
// END - slf4j 2.x


res.add(mavenBundle().groupId("jakarta.el").artifactId("jakarta.el-api").versionAsInProject().start());

res.add(mavenBundle().groupId("jakarta.servlet").artifactId("jakarta.servlet-api").versionAsInProject().start());
Expand Down
Loading
Loading