Update the Orbit repository#1292
Conversation
ddk-target/ddk.target
Outdated
| <unit id="org.eclipse.lsp4j.sdk.feature.group" version="0.0.0"/> | ||
| </location> | ||
| <location includeAllPlatforms="false" includeConfigurePhase="false" includeMode="planner" includeSource="true" type="InstallableUnit"> | ||
| <repository location="https://download.eclipse.org/tools/orbit/simrel/maven-osgi/2025-09"/> |
There was a problem hiding this comment.
can we use here 2026-03 as well?
There was a problem hiding this comment.
done. I used orbit-aggregation/release/4.39.0 which should be the same as 2026-03.
ddk-target/ddk.target
Outdated
| <unit id="org.apache.commons.lang" version="2.6.0.v20220406-2305"/> | ||
| <unit id="org.apache.commons.logging" version="1.2.0.v20180409-1502"/> | ||
| <unit id="org.apache.log4j" version="1.2.24.v20221221-2012"/> | ||
| <unit id="org.mockito.mockito-core" version="4.8.1.v20221103-2317"/> |
There was a problem hiding this comment.
can we also take the mockito from the new orbit?
|
|
||
| // org.eclipse.osgi needed for NLS | ||
| // org.apache.logging.log4j needed for logging in generated StandaloneSetup | ||
| private static final List<String> REQUIRED_BUNDLES = newArrayList("org.eclipse.xtext.xbase.lib", "org.eclipse.xtend.lib", "org.eclipse.emf.ecore", "com.avaloq.tools.ddk.check.core", "com.avaloq.tools.ddk.check.runtime.core", "com.avaloq.tools.ddk.check.lib", "com.avaloq.tools.ddk.xtext", "org.eclipse.xtext", "org.eclipse.osgi", "org.eclipse.xtend", "org.eclipse.core.runtime", "org.eclipse.xtext.xbase", "org.apache.logging.log4j"); |
There was a problem hiding this comment.
I think we should be able to remove this class as part of the removal of the check project for customers. Can you check that, and if so drop it first in the other tictket?
There was a problem hiding this comment.
at this point I am not sure if we can safely remove this. to me it looks like it is also test project manager for the ddk automated tests.
eaf957e to
9cb947c
Compare
| com.avaloq.tools.ddk.check.typing, | ||
| com.avaloq.tools.ddk.check.util, | ||
| com.avaloq.tools.ddk.check.validation | ||
| Import-Package: org.apache.logging.log4j, |
There was a problem hiding this comment.
why do we change from import package too require bundle in all the plugins?
There was a problem hiding this comment.
I was encountering Eclipse startup issues when trying to deploy the DDK features (i.e. export DDK plugin on host and reboot) on an ASMD workspace. [Debug log here:] (https://github.com/user-attachments/files/26257056/Debug_Log.txt)
Similar to apache/logging-log4j2#2655
It seems that ProviderUtil.lazyInit was locking up because it could not find a list of providers, so I decided
to change to require bundles to make sure that log4j.core gets loaded first before log4j.api.
But maybe there's a better way to do this?
There was a problem hiding this comment.
It seems that locking up of ProviderUtil.lazyInit was caused in the ASMD workspace, when the require-bundle log4j.api gets loaded first before log4j.core.
I removed all require-bundle entries referring to logging.log4j in the ASMD target and moved them all to import-package section. Looks like the trouble with the startup issues disappeared.
| Require-Bundle: org.eclipse.xtext, | ||
| org.eclipse.xtext.xtext.generator, | ||
| org.eclipse.xtext.util, | ||
| org.apache.logging.log4j.core, |
There was a problem hiding this comment.
The org.apache.logging.log4j.core bundle is the implementation and should only be a dependency of the bundle that configures logging, not of every bundle that simply logs. Depending on core everywhere creates an unnecessary tight coupling to the implementation.
Can you revisit the commit to remove the dependency on org.apache.logging.log4j.core from most places?
There was a problem hiding this comment.
I removed these changes adding entries to require-bundle and limiting changes to import-package instead.
The changes look okay with my testing until now.
rubenporras
left a comment
There was a problem hiding this comment.
The org.apache.logging.log4j.core bundle is the implementation and should only be a dependency of the bundle that configures logging, not of every bundle that simply logs. Depending on core everywhere creates an unnecessary tight coupling to the implementation.
Can you revisit the commit to remove the dependency on org.apache.logging.log4j.core from most places?
0ac4ff4 to
29f183f
Compare
| org.eclipse.jdt.internal.ui.text, | ||
| org.apache.log4j | ||
| Import-Package: org.eclipse.jdt.internal.ui.text, | ||
| org.apache.log4j, org.apache.logging.log4j |
There was a problem hiding this comment.
should this not be
| org.apache.log4j, org.apache.logging.log4j | |
| org.apache.logging.log4j | |
| ```? |
| com.avaloq.tools.ddk.check.ide.contentassist.antlr.internal | ||
| Import-Package: org.apache.log4j, | ||
| org.apache.logging.log4j | ||
| Import-Package: org.apache.log4j, org.apache.logging.log4j |
There was a problem hiding this comment.
should this not be ```suggestion
Import-Package: org.apache.logging.log4j
| Bundle-Version: 17.2.0.qualifier | ||
| Bundle-Vendor: Avaloq Group AG | ||
| Bundle-RequiredExecutionEnvironment: JavaSE-21 | ||
| Import-Package: org.apache.logging.log4j |
There was a problem hiding this comment.
move import package to line 23 to keep order?
| @@ -25,8 +25,7 @@ Require-Bundle: org.eclipse.xtext;visibility:=reexport, | |||
| com.avaloq.tools.ddk.check.core, | |||
| org.eclipse.xtext.xbase.lib | |||
| Import-Package: org.apache.log4j, | |||
There was a problem hiding this comment.
should org.apache.log4j not be removed?
| @@ -30,11 +30,11 @@ Export-Package: com.avaloq.tools.ddk.check.ui.test, | |||
| com.avaloq.tools.ddk.check.ui.test.util, | |||
| com.avaloq.tools.ddk.check | |||
| Import-Package: org.slf4j, org.apache.log4j, | |||
There was a problem hiding this comment.
same, I do not comment the rest of the PR to not repeat in unecessarily.
There was a problem hiding this comment.
done.
However, there remains some MANIFEST.MF where I cannot remove org.apache.log4j as import.
The following Activator classes are Xtext generated, and based on my understanding, the latest Xtext version
still uses log4j, so I cannot use Log4j 2 for these at this point.
CheckcfgActivator (com.avaloq.tools.ddk.checkcfg.ui)
ExportActivator (com.avaloq.tools.ddk.xtext.export.ui)
FormatActivator (com.avaloq.tools.ddk.xtext.format.ui)
ExpressionActivator (com.avaloq.tools.ddk.xtext.expression.ui)
HelloworldActivator (com.avaloq.tools.ddk.sample.helloworld.ui)
ScopeActivator (com.avaloq.tools.ddk.xtext.scope.ui)
Also FormatValueConverterService (com.avaloq.tools.ddk.xtext.format) uses org.eclipse.xtext.conversion.impl.IDValueConverter that has dependency on Log4J 1.x, so that also stays
at this point.
bcacad3 to
99159ab
Compare
| @@ -1,5 +1,5 @@ | |||
| /* | |||
| * generated by Xtext | |||
| * generated by Xtext 2.27.0.M3 | |||
rubenporras
left a comment
There was a problem hiding this comment.
can you remove the file with the comment change from the PR?
No description provided.