Fix the conflict of bundle when updating. #385
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
yaohaizh
changed the title
fbricon
reviewed
Oct 13, 2017
|
|
||
| private static BundleInfo getBundleInfo(String bundleLocation) throws IOException { | ||
| JarFile jarFile = new JarFile(bundleLocation); | ||
| Manifest manifest = jarFile.getManifest(); |
There was a problem hiding this comment.
might want to check if manifest != null
fbricon
reviewed
Oct 13, 2017
| } | ||
|
|
||
| return new BundleInfo(bundleVersion, symbolicName); | ||
| } |
There was a problem hiding this comment.
private static BundleInfo getBundleInfo(String bundleLocation) throws IOException {
try (JarFile jarFile = new JarFile(bundleLocation)){
Manifest manifest = jarFile.getManifest();
if (manifest != null) {
Attributes mainAttributes = manifest.getMainAttributes();
String bundleVersion = mainAttributes.getValue(Constants.BUNDLE_VERSION);
String symbolicName = mainAttributes.getValue(Constants.BUNDLE_SYMBOLICNAME);
if (StringUtils.isNotBlank(symbolicName) && symbolicName.indexOf(";") >= 0) {
symbolicName = symbolicName.substring(0, symbolicName.indexOf(";"));
}
return new BundleInfo(bundleVersion, symbolicName);
}
}
return null;
}There was a problem hiding this comment.
alternatively, throw IOException instead of returning null
fbricon
reviewed
Oct 13, 2017
| @@ -57,17 +84,27 @@ public static void loadBundles(Collection<String> bundleLocations) throws CoreEx | |||
|
|
|||
| String location = getBundleLocation(bundleLocation, true); | |||
|
|
|||
| Bundle bundle = context.getBundle(location); | |||
| boolean needInstall = true; | |||
| BundleInfo bundleInfo = getBundleInfo(bundleLocation); | |||
There was a problem hiding this comment.
check if bundleInfo is null?
|
Other than making getBundleInfo(...) more robust, this PR looks good. We'll include it in the next release. |
yaohaizh
force-pushed
the
bundle_update
branch
from
871a841 to
fb150a3
Compare
Signed-off-by: Yaohai Zheng <[email protected]>
yaohaizh
force-pushed
the
bundle_update
branch
from
fb150a3 to
b68aa62
Compare
|
Update the changes. |
|
Merged as 7dc20a2 (after adding a null check on bundle version) |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Signed-off-by: Yaohai Zheng [email protected]
Fix the issue: microsoft/vscode-java-debug#54
In this change, it makes sure the extension symbolic name is unique and doesn't support multiple version of bundles. These limitation should not be problems for jdt.ls extensions.
@fbricon @gorkem @tsmaeder