You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@felix.apache.org by GitBox <gi...@apache.org> on 2020/03/02 10:42:50 UTC
[GitHub] [felix-dev] DominikSuess opened a new pull request #1: FELIX-6232 -
adding option to install parallel version of bundle
DominikSuess opened a new pull request #1: FELIX-6232 - adding option to install parallel version of bundle
URL: https://github.com/apache/felix-dev/pull/1
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [felix-dev] DominikSuess commented on a change in pull request #1:
FELIX-6232 - adding option to install parallel version of bundle
Posted by GitBox <gi...@apache.org>.
DominikSuess commented on a change in pull request #1: FELIX-6232 - adding option to install parallel version of bundle
URL: https://github.com/apache/felix-dev/pull/1#discussion_r388191268
##########
File path: webconsole/src/main/java/org/apache/felix/webconsole/internal/core/BundlesServlet.java
##########
@@ -1642,8 +1648,10 @@ private void installBundle( String location, File bundleFile, int startLevel, bo
Bundle[] bundles = BundleContextUtil.getWorkingBundleContext(this.getBundleContext()).getBundles();
for ( int i = 0; i < bundles.length; i++ )
{
+ boolean isSameBSN = (bundles[i].getSymbolicName() != null && bundles[i].getSymbolicName().equals( symbolicName ));
Review comment:
I appreciate the suggestions - but to be honest I just moved the check fro the if statement below. I tend to change as little as possible to reduce the impact of anything going wrong. I suggest that this PR gets accepted and a subsequent cleanup commit would be made.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [felix-dev] kwin commented on a change in pull request #1:
FELIX-6232 - adding option to install parallel version of bundle
Posted by GitBox <gi...@apache.org>.
kwin commented on a change in pull request #1: FELIX-6232 - adding option to install parallel version of bundle
URL: https://github.com/apache/felix-dev/pull/1#discussion_r386325480
##########
File path: webconsole/src/main/java/org/apache/felix/webconsole/internal/core/BundlesServlet.java
##########
@@ -1710,6 +1718,43 @@ private String getSymbolicName( File bundleFile )
// fall back to "not found"
return null;
}
+
+ private String getBundleVersion( File bundleFile )
+ {
+ JarFile jar = null;
+ try
+ {
+ jar = new JarFile( bundleFile );
Review comment:
try with resources (https://docs.oracle.com/javase/tutorial/essential/exceptions/tryResourceClose.html)?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [felix-dev] DominikSuess commented on a change in pull request #1:
FELIX-6232 - adding option to install parallel version of bundle
Posted by GitBox <gi...@apache.org>.
DominikSuess commented on a change in pull request #1: FELIX-6232 - adding option to install parallel version of bundle
URL: https://github.com/apache/felix-dev/pull/1#discussion_r388229240
##########
File path: webconsole/src/main/java/org/apache/felix/webconsole/internal/core/BundlesServlet.java
##########
@@ -1617,14 +1622,15 @@ private FileItem getParameter( Map params, String name )
}
- private void installBundle( String location, File bundleFile, int startLevel, boolean start, boolean refreshPackages )
+ private void installBundle( String location, File bundleFile, int startLevel, boolean start, boolean refreshPackages, boolean parallelVersion)
throws IOException
{
if ( bundleFile != null )
{
// try to get the bundle name, fail if none
String symbolicName = getSymbolicName( bundleFile );
Review comment:
@cziegeler please review if adjustments match your expectations
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [felix-dev] DominikSuess commented on a change in pull request #1:
FELIX-6232 - adding option to install parallel version of bundle
Posted by GitBox <gi...@apache.org>.
DominikSuess commented on a change in pull request #1: FELIX-6232 - adding option to install parallel version of bundle
URL: https://github.com/apache/felix-dev/pull/1#discussion_r388229007
##########
File path: webconsole/src/main/java/org/apache/felix/webconsole/internal/core/BundlesServlet.java
##########
@@ -1710,6 +1718,43 @@ private String getSymbolicName( File bundleFile )
// fall back to "not found"
return null;
}
+
+ private String getBundleVersion( File bundleFile )
+ {
+ JarFile jar = null;
+ try
+ {
+ jar = new JarFile( bundleFile );
Review comment:
The Felix Webconsole project is currently marked to be Java 6 compliant - this was just reproducing a pattern used already before for bsn - I just changed this based on suggestion by Carsten below to make sure to use a single method to obtain bsn & version
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [felix-dev] amitjoy commented on a change in pull request #1:
FELIX-6232 - adding option to install parallel version of bundle
Posted by GitBox <gi...@apache.org>.
amitjoy commented on a change in pull request #1: FELIX-6232 - adding option to install parallel version of bundle
URL: https://github.com/apache/felix-dev/pull/1#discussion_r386368563
##########
File path: webconsole/src/main/java/org/apache/felix/webconsole/internal/core/BundlesServlet.java
##########
@@ -1642,8 +1648,10 @@ private void installBundle( String location, File bundleFile, int startLevel, bo
Bundle[] bundles = BundleContextUtil.getWorkingBundleContext(this.getBundleContext()).getBundles();
for ( int i = 0; i < bundles.length; i++ )
{
+ boolean isSameBSN = (bundles[i].getSymbolicName() != null && bundles[i].getSymbolicName().equals( symbolicName ));
Review comment:
BSN cannot be null according to Felix implementation but according to Javadoc, it can. I agree completely that the null check is superfluous here.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [felix-dev] DominikSuess commented on a change in pull request #1:
FELIX-6232 - adding option to install parallel version of bundle
Posted by GitBox <gi...@apache.org>.
DominikSuess commented on a change in pull request #1: FELIX-6232 - adding option to install parallel version of bundle
URL: https://github.com/apache/felix-dev/pull/1#discussion_r388191268
##########
File path: webconsole/src/main/java/org/apache/felix/webconsole/internal/core/BundlesServlet.java
##########
@@ -1642,8 +1648,10 @@ private void installBundle( String location, File bundleFile, int startLevel, bo
Bundle[] bundles = BundleContextUtil.getWorkingBundleContext(this.getBundleContext()).getBundles();
for ( int i = 0; i < bundles.length; i++ )
{
+ boolean isSameBSN = (bundles[i].getSymbolicName() != null && bundles[i].getSymbolicName().equals( symbolicName ));
Review comment:
I appreciate the suggestions - but to be honest I just moved the check fro the if statement below out. I tend to change as little as possible to reduce the impact of anything going wrong. I suggest that this PR gets accepted and a subsequent cleanup commit would be made.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [felix-dev] kwin commented on a change in pull request #1:
FELIX-6232 - adding option to install parallel version of bundle
Posted by GitBox <gi...@apache.org>.
kwin commented on a change in pull request #1: FELIX-6232 - adding option to install parallel version of bundle
URL: https://github.com/apache/felix-dev/pull/1#discussion_r386324692
##########
File path: webconsole/src/main/java/org/apache/felix/webconsole/internal/core/BundlesServlet.java
##########
@@ -1642,8 +1648,10 @@ private void installBundle( String location, File bundleFile, int startLevel, bo
Bundle[] bundles = BundleContextUtil.getWorkingBundleContext(this.getBundleContext()).getBundles();
for ( int i = 0; i < bundles.length; i++ )
{
+ boolean isSameBSN = (bundles[i].getSymbolicName() != null && bundles[i].getSymbolicName().equals( symbolicName ));
Review comment:
I think you can just do `symbolicName.equals(bundles[i].getSymbolicName())` as `symbolicName` can't be `null` here!
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [felix-dev] jsedding commented on a change in pull request #1:
FELIX-6232 - adding option to install parallel version of bundle
Posted by GitBox <gi...@apache.org>.
jsedding commented on a change in pull request #1: FELIX-6232 - adding option to install parallel version of bundle
URL: https://github.com/apache/felix-dev/pull/1#discussion_r387001151
##########
File path: webconsole/src/main/java/org/apache/felix/webconsole/internal/core/BundlesServlet.java
##########
@@ -1642,8 +1648,10 @@ private void installBundle( String location, File bundleFile, int startLevel, bo
Bundle[] bundles = BundleContextUtil.getWorkingBundleContext(this.getBundleContext()).getBundles();
for ( int i = 0; i < bundles.length; i++ )
{
+ boolean isSameBSN = (bundles[i].getSymbolicName() != null && bundles[i].getSymbolicName().equals( symbolicName ));
Review comment:
You could use `Objects.equals`, it handles `null` arguments.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [felix-dev] cziegeler merged pull request #1: FELIX-6232 - adding
option to install parallel version of bundle
Posted by GitBox <gi...@apache.org>.
cziegeler merged pull request #1: FELIX-6232 - adding option to install parallel version of bundle
URL: https://github.com/apache/felix-dev/pull/1
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [felix-dev] bosschaert commented on issue #1: FELIX-6232 - adding
option to install parallel version of bundle
Posted by GitBox <gi...@apache.org>.
bosschaert commented on issue #1: FELIX-6232 - adding option to install parallel version of bundle
URL: https://github.com/apache/felix-dev/pull/1#issuecomment-593343108
Hi @DominikSuess the change looks good to me.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [felix-dev] cziegeler commented on a change in pull request #1:
FELIX-6232 - adding option to install parallel version of bundle
Posted by GitBox <gi...@apache.org>.
cziegeler commented on a change in pull request #1: FELIX-6232 - adding option to install parallel version of bundle
URL: https://github.com/apache/felix-dev/pull/1#discussion_r388195649
##########
File path: webconsole/src/main/java/org/apache/felix/webconsole/internal/core/BundlesServlet.java
##########
@@ -1642,8 +1648,10 @@ private void installBundle( String location, File bundleFile, int startLevel, bo
Bundle[] bundles = BundleContextUtil.getWorkingBundleContext(this.getBundleContext()).getBundles();
for ( int i = 0; i < bundles.length; i++ )
{
+ boolean isSameBSN = (bundles[i].getSymbolicName() != null && bundles[i].getSymbolicName().equals( symbolicName ));
Review comment:
I agree
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [felix-dev] kwin commented on a change in pull request #1:
FELIX-6232 - adding option to install parallel version of bundle
Posted by GitBox <gi...@apache.org>.
kwin commented on a change in pull request #1: FELIX-6232 - adding option to install parallel version of bundle
URL: https://github.com/apache/felix-dev/pull/1#discussion_r386387233
##########
File path: webconsole/src/main/java/org/apache/felix/webconsole/internal/core/BundlesServlet.java
##########
@@ -1642,8 +1648,10 @@ private void installBundle( String location, File bundleFile, int startLevel, bo
Bundle[] bundles = BundleContextUtil.getWorkingBundleContext(this.getBundleContext()).getBundles();
for ( int i = 0; i < bundles.length; i++ )
{
+ boolean isSameBSN = (bundles[i].getSymbolicName() != null && bundles[i].getSymbolicName().equals( symbolicName ));
Review comment:
It can't be null due to the if condition at https://github.com/apache/felix-dev/pull/1/files#diff-b66e715ba83980920efaa568f6819c45L1628!
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [felix-dev] cziegeler commented on a change in pull request #1:
FELIX-6232 - adding option to install parallel version of bundle
Posted by GitBox <gi...@apache.org>.
cziegeler commented on a change in pull request #1: FELIX-6232 - adding option to install parallel version of bundle
URL: https://github.com/apache/felix-dev/pull/1#discussion_r386392472
##########
File path: webconsole/src/main/java/org/apache/felix/webconsole/internal/core/BundlesServlet.java
##########
@@ -1617,14 +1622,15 @@ private FileItem getParameter( Map params, String name )
}
- private void installBundle( String location, File bundleFile, int startLevel, boolean start, boolean refreshPackages )
+ private void installBundle( String location, File bundleFile, int startLevel, boolean start, boolean refreshPackages, boolean parallelVersion)
throws IOException
{
if ( bundleFile != null )
{
// try to get the bundle name, fail if none
String symbolicName = getSymbolicName( bundleFile );
Review comment:
I suggest to get bsn and version in one method, otherwise you open the file twice
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services