You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@brooklyn.apache.org by ahgittin <gi...@git.apache.org> on 2015/01/27 18:39:24 UTC

[GitHub] incubator-brooklyn pull request: CLI better default, port/https co...

GitHub user ahgittin opened a pull request:

    https://github.com/apache/incubator-brooklyn/pull/478

    CLI better default, port/https control, and version reporting

            * adds config key for web-console port
            * adds CLI for https
            * port defaults to 8443+ if no port specified and using https
            * default if no CLI options is to show help and an error
            * some https test fixes
            * discovers and reports Brooklyn OSGi metadata in log, including git SHA1 (and `brooklyn info`)

You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/ahgittin/incubator-brooklyn cli-ports-and-more

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/incubator-brooklyn/pull/478.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #478
    
----
commit 1f904e0738abdf3b6ff25f1445e003a3ded8784b
Author: Alex Heneveld <al...@cloudsoftcorp.com>
Date:   2015-01-27T11:53:41Z

    CLI better default, port/https control, and version reporting
    
            * adds config key for web-console port
            * adds CLI for https
            * port defaults to 8443+ if no port specified and using https
            * default if no CLI options is to show help and an error
            * some https test fixes
            * discovers and reports Brooklyn OSGi metadata in log, including git SHA1 (and `brooklyn info`)

----


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-brooklyn pull request: CLI better default, port/https co...

Posted by ahgittin <gi...@git.apache.org>.
Github user ahgittin commented on a diff in the pull request:

    https://github.com/apache/incubator-brooklyn/pull/478#discussion_r24348386
  
    --- Diff: core/src/main/java/brooklyn/BrooklynVersion.java ---
    @@ -19,80 +19,218 @@
     package brooklyn;
     
     import static com.google.common.base.Preconditions.checkNotNull;
    -import static java.lang.String.format;
     
     import java.io.IOException;
     import java.io.InputStream;
    +import java.net.URL;
    +import java.util.Enumeration;
     import java.util.Properties;
    +import java.util.concurrent.atomic.AtomicReference;
    +import java.util.jar.Attributes;
    +
    +import javax.annotation.Nullable;
     
     import org.slf4j.Logger;
     import org.slf4j.LoggerFactory;
     
    +import brooklyn.util.exceptions.Exceptions;
    +import brooklyn.util.osgi.Osgis;
    +import brooklyn.util.osgi.Osgis.ManifestHelper;
    +import brooklyn.util.text.Strings;
    +
    +/**
    + * Wraps the version of Brooklyn.
    + * <p>
    + * Also retrieves the SHA-1 from any OSGi bundle, and checks that the maven and osgi versions match.
    + */
     public class BrooklynVersion {
     
       private static final Logger log = LoggerFactory.getLogger(BrooklynVersion.class);
       
    -  private static final String VERSION_RESOURCE_FILE = "META-INF/maven/io.brooklyn/brooklyn-core/pom.properties";
    -  private static final String VERSION_PROPERTY_NAME = "version";
    +  private static final String MVN_VERSION_RESOURCE_FILE = "META-INF/maven/org.apache.brooklyn/brooklyn-core/pom.properties";
    +  private static final String MANIFEST_PATH = "META-INF/MANIFEST.MF";
    +  private static final String BROOKLYN_CORE_SYMBOLIC_NAME = "org.apache.brooklyn.core";
    +  
    +  private static final String MVN_VERSION_PROPERTY_NAME = "version";
    +  private static final String OSGI_VERSION_PROPERTY_NAME = Attributes.Name.IMPLEMENTATION_VERSION.toString();
    +  private static final String OSGI_SHA1_PROPERTY_NAME = "Implementation-SHA-1";
    +
     
    +  private final static String VERSION_FROM_STATIC = "0.7.0-SNAPSHOT"; // BROOKLYN_VERSION
    +  private static final AtomicReference<Boolean> IS_DEV_ENV = new AtomicReference<Boolean>();
    +  
       public static final BrooklynVersion INSTANCE = new BrooklynVersion();
    +  
    +  private final Properties versionProperties = new Properties();
    +  
    +  public BrooklynVersion() {
    +      // we read the maven pom metadata and osgi metadata and make sure it's sensible
    +      // everything is put into a single map for now (good enough, but should be cleaned up)
    +      readPropertiesFromMavenResource(BrooklynVersion.class.getClassLoader());
    +      readPropertiesFromOsgiResource(BrooklynVersion.class.getClassLoader(), "org.apache.brooklyn.core");
    +      // TODO there is also build-metadata.properties used in ServerResource /v1/server/version endpoint
    +      // see comments on that about folding it into this class instead
     
    -  private final String versionFromClasspath;
    -  // static useful when running from the IDE
    -  // TODO is the classpath version ever useful? should we always use the static?
    -  private final String versionFromStatic = "0.7.0-SNAPSHOT"; // BROOKLYN_VERSION
    -  private final String version;
    +      checkVersions();
    +  }
     
    -  public BrooklynVersion() {
    -    this.versionFromClasspath = readVersionPropertyFromClasspath(BrooklynVersion.class.getClassLoader());
    -    if (isValid(versionFromClasspath)) {
    -        this.version = versionFromClasspath;
    -        if (!this.version.equals(versionFromStatic)) {
    -            // should always be the same, and we can drop classpath detection ...
    -            log.warn("Version detected from classpath as "+versionFromClasspath+" (preferring that), but in code it is recorded as "+versionFromStatic);
    -        }
    -    } else {
    -        this.version = versionFromStatic;
    -    }
    +  public void checkVersions() {
    +      String mvnVersion = getVersionFromMavenProperties();
    +      if (mvnVersion!=null && !VERSION_FROM_STATIC.equals(mvnVersion)) {
    +          throw new IllegalStateException("Version error: maven "+mvnVersion+" / code "+VERSION_FROM_STATIC);
    +      }
    +      
    +      String osgiVersion = versionProperties.getProperty(OSGI_VERSION_PROPERTY_NAME);
    +      // TODO does the OSGi version include other slightly differ gubbins/style ?
    +      if (osgiVersion!=null && !VERSION_FROM_STATIC.equals(osgiVersion)) {
    +          throw new IllegalStateException("Version error: osgi "+osgiVersion+" / code "+VERSION_FROM_STATIC);
    +      }
       }
    -  
    +
    +  /** Returns version as inferred from classpath/osgi, if possible, or 0.0.0-SNAPSHOT.
    +   * See also {@link #getVersionFromMavenProperties()} and {@link #getVersionFromOsgiManifest()}.
    +   * @deprecated since 0.7.0, in favour of the more specific methods (and does anyone need that default value?)
    +   */
    +  @Deprecated
       public String getVersionFromClasspath() {
    -    return versionFromClasspath;
    +      String v = getVersionFromMavenProperties();
    +      if (Strings.isNonBlank(v)) return v;
    +      v = getVersionFromOsgiManifest();
    +      if (Strings.isNonBlank(v)) return v;
    +      return "0.0.0-SNAPSHOT";
       }
       
    -  public String getVersion() {
    -    return version;
    +  @Nullable
    +  public String getVersionFromMavenProperties() {
    +      return versionProperties.getProperty(MVN_VERSION_PROPERTY_NAME);
    +  }
    +
    +  @Nullable
    +  public String getVersionFromOsgiManifest() {
    +      return versionProperties.getProperty(OSGI_VERSION_PROPERTY_NAME);
       }
       
    -  public String getVersionFromStatic() {
    -    return versionFromStatic;
    +  @Nullable
    +  /** SHA1 of the last commit to brooklyn at the time this build was made.
    +   * For SNAPSHOT builds of course there may have been further non-committed changes. */
    +  public String getSha1FromOsgiManifest() {
    +      return versionProperties.getProperty(OSGI_SHA1_PROPERTY_NAME);
       }
    -
    -  public boolean isSnapshot() {
    -      return (getVersion().indexOf("-SNAPSHOT")>=0);
    +  
    +  public String getVersion() {
    +    return VERSION_FROM_STATIC;
       }
       
    -  private static boolean isValid(String v) {
    -    if (v==null) return false;
    -    if (v.equals("0.0.0") || v.equals("0.0")) return false;
    -    if (v.startsWith("0.0.0-") || v.startsWith("0.0-")) return false;
    -    return true;
    +  public boolean isSnapshot() {
    +      return (getVersion().indexOf("-SNAPSHOT")>=0);
       }
    -
    -  private String readVersionPropertyFromClasspath(ClassLoader resourceLoader) {
    -    Properties versionProperties = new Properties();
    +    
    +  private void readPropertiesFromMavenResource(ClassLoader resourceLoader) {
         try {
    -      InputStream versionStream = resourceLoader.getResourceAsStream(VERSION_RESOURCE_FILE);
    -      if (versionStream==null) return null;
    +      InputStream versionStream = resourceLoader.getResourceAsStream(MVN_VERSION_RESOURCE_FILE);
    +      if (versionStream==null) {
    +          if (isDevelopmentEnvironment()) {
    +              // allowed for dev env
    +              log.trace("No maven resource file "+MVN_VERSION_RESOURCE_FILE+" available");
    +          } else {
    +              log.warn("No maven resource file "+MVN_VERSION_RESOURCE_FILE+" available");
    +          }
    +          return;
    +      }
           versionProperties.load(checkNotNull(versionStream));
    -    } catch (IOException exception) {
    -      throw new IllegalStateException(format("Unable to load version resource file '%s'", VERSION_RESOURCE_FILE), exception);
    +    } catch (IOException e) {
    +      log.warn("Error reading maven resource file "+MVN_VERSION_RESOURCE_FILE+": "+e, e);
         }
    -    return checkNotNull(versionProperties.getProperty(VERSION_PROPERTY_NAME), VERSION_PROPERTY_NAME);
    +  }
    +
    +  /** reads properties from brooklyn-core's manifest */
    +  private void readPropertiesFromOsgiResource(ClassLoader resourceLoader, String symbolicName) {
    +      Enumeration<URL> paths;
    +      try {
    +          paths = BrooklynVersion.class.getClassLoader().getResources(MANIFEST_PATH);
    +      } catch (IOException e) {
    +          // shouldn't happen
    +          throw Exceptions.propagate(e);
    +      }
    +      while (paths.hasMoreElements()) {
    +          URL u = paths.nextElement();
    +          try {
    +              ManifestHelper mh = Osgis.ManifestHelper.forManifest(u.openStream());
    +              if (BROOKLYN_CORE_SYMBOLIC_NAME.equals(mh.getSymbolicName())) {
    +                  Attributes attrs = mh.getManifest().getMainAttributes();
    +                  for (Object key: attrs.keySet()) {
    +                      // key is an Attribute.Name; toString converts to string
    +                      versionProperties.put(key.toString(), attrs.getValue(key.toString()));
    +                  }
    +                  return;
    +              }
    +          } catch (Exception e) {
    +              Exceptions.propagateIfFatal(e);
    +              log.warn("Error reading OSGi manifest from "+u+" when determining version properties: "+e, e);
    +          }
    +      }
    +      if (isDevelopmentEnvironment()) {
    +          // allowed for dev env
    +          log.trace("No OSGi manifest available to determine version properties");
    +      } else {
    +          log.warn("No OSGi manifest available to determine version properties");
    +      }
    +  }
    +
    +  /** 
    +   * Returns whether this is a Brooklyn dev environment,
    +   * specifically core/target/classes/ is on the classpath for the org.apache.brooklyn.core project.
    +   * <p>
    +   * In a packaged or library build of Brooklyn (normal usage) this should return false,
    +   * and all OSGi components should be available.
    +   */
    +  public static boolean isDevelopmentEnvironment() {
    +      Boolean isDevEnv = IS_DEV_ENV.get();
    +      if (isDevEnv!=null) return isDevEnv;
    +      synchronized (IS_DEV_ENV) {
    +          isDevEnv = computeIsDevelopmentEnvironment();
    +          IS_DEV_ENV.set(isDevEnv);
    +          return isDevEnv;
    +      }
    +  }
    +  
    +  private static boolean computeIsDevelopmentEnvironment() {
    --- End diff --
    
    good spot, i'll clean this up.  (i hadn't meant to merge this yet... another fat-finger.)


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-brooklyn pull request: CLI better default, port/https co...

Posted by ahgittin <gi...@git.apache.org>.
Github user ahgittin commented on the pull request:

    https://github.com/apache/incubator-brooklyn/pull/478#issuecomment-73034995
  
    @aledsage @grkvlt  could you cast your eye over this please?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-brooklyn pull request: CLI better default, port/https co...

Posted by neykov <gi...@git.apache.org>.
Github user neykov commented on a diff in the pull request:

    https://github.com/apache/incubator-brooklyn/pull/478#discussion_r24330952
  
    --- Diff: core/src/main/java/brooklyn/BrooklynVersion.java ---
    @@ -19,80 +19,218 @@
     package brooklyn;
     
     import static com.google.common.base.Preconditions.checkNotNull;
    -import static java.lang.String.format;
     
     import java.io.IOException;
     import java.io.InputStream;
    +import java.net.URL;
    +import java.util.Enumeration;
     import java.util.Properties;
    +import java.util.concurrent.atomic.AtomicReference;
    +import java.util.jar.Attributes;
    +
    +import javax.annotation.Nullable;
     
     import org.slf4j.Logger;
     import org.slf4j.LoggerFactory;
     
    +import brooklyn.util.exceptions.Exceptions;
    +import brooklyn.util.osgi.Osgis;
    +import brooklyn.util.osgi.Osgis.ManifestHelper;
    +import brooklyn.util.text.Strings;
    +
    +/**
    + * Wraps the version of Brooklyn.
    + * <p>
    + * Also retrieves the SHA-1 from any OSGi bundle, and checks that the maven and osgi versions match.
    + */
     public class BrooklynVersion {
     
       private static final Logger log = LoggerFactory.getLogger(BrooklynVersion.class);
       
    -  private static final String VERSION_RESOURCE_FILE = "META-INF/maven/io.brooklyn/brooklyn-core/pom.properties";
    -  private static final String VERSION_PROPERTY_NAME = "version";
    +  private static final String MVN_VERSION_RESOURCE_FILE = "META-INF/maven/org.apache.brooklyn/brooklyn-core/pom.properties";
    +  private static final String MANIFEST_PATH = "META-INF/MANIFEST.MF";
    +  private static final String BROOKLYN_CORE_SYMBOLIC_NAME = "org.apache.brooklyn.core";
    +  
    +  private static final String MVN_VERSION_PROPERTY_NAME = "version";
    +  private static final String OSGI_VERSION_PROPERTY_NAME = Attributes.Name.IMPLEMENTATION_VERSION.toString();
    +  private static final String OSGI_SHA1_PROPERTY_NAME = "Implementation-SHA-1";
    +
     
    +  private final static String VERSION_FROM_STATIC = "0.7.0-SNAPSHOT"; // BROOKLYN_VERSION
    +  private static final AtomicReference<Boolean> IS_DEV_ENV = new AtomicReference<Boolean>();
    +  
       public static final BrooklynVersion INSTANCE = new BrooklynVersion();
    +  
    +  private final Properties versionProperties = new Properties();
    +  
    +  public BrooklynVersion() {
    +      // we read the maven pom metadata and osgi metadata and make sure it's sensible
    +      // everything is put into a single map for now (good enough, but should be cleaned up)
    +      readPropertiesFromMavenResource(BrooklynVersion.class.getClassLoader());
    +      readPropertiesFromOsgiResource(BrooklynVersion.class.getClassLoader(), "org.apache.brooklyn.core");
    +      // TODO there is also build-metadata.properties used in ServerResource /v1/server/version endpoint
    +      // see comments on that about folding it into this class instead
     
    -  private final String versionFromClasspath;
    -  // static useful when running from the IDE
    -  // TODO is the classpath version ever useful? should we always use the static?
    -  private final String versionFromStatic = "0.7.0-SNAPSHOT"; // BROOKLYN_VERSION
    -  private final String version;
    +      checkVersions();
    +  }
     
    -  public BrooklynVersion() {
    -    this.versionFromClasspath = readVersionPropertyFromClasspath(BrooklynVersion.class.getClassLoader());
    -    if (isValid(versionFromClasspath)) {
    -        this.version = versionFromClasspath;
    -        if (!this.version.equals(versionFromStatic)) {
    -            // should always be the same, and we can drop classpath detection ...
    -            log.warn("Version detected from classpath as "+versionFromClasspath+" (preferring that), but in code it is recorded as "+versionFromStatic);
    -        }
    -    } else {
    -        this.version = versionFromStatic;
    -    }
    +  public void checkVersions() {
    +      String mvnVersion = getVersionFromMavenProperties();
    +      if (mvnVersion!=null && !VERSION_FROM_STATIC.equals(mvnVersion)) {
    +          throw new IllegalStateException("Version error: maven "+mvnVersion+" / code "+VERSION_FROM_STATIC);
    +      }
    +      
    +      String osgiVersion = versionProperties.getProperty(OSGI_VERSION_PROPERTY_NAME);
    +      // TODO does the OSGi version include other slightly differ gubbins/style ?
    +      if (osgiVersion!=null && !VERSION_FROM_STATIC.equals(osgiVersion)) {
    +          throw new IllegalStateException("Version error: osgi "+osgiVersion+" / code "+VERSION_FROM_STATIC);
    +      }
       }
    -  
    +
    +  /** Returns version as inferred from classpath/osgi, if possible, or 0.0.0-SNAPSHOT.
    +   * See also {@link #getVersionFromMavenProperties()} and {@link #getVersionFromOsgiManifest()}.
    +   * @deprecated since 0.7.0, in favour of the more specific methods (and does anyone need that default value?)
    +   */
    +  @Deprecated
       public String getVersionFromClasspath() {
    -    return versionFromClasspath;
    +      String v = getVersionFromMavenProperties();
    +      if (Strings.isNonBlank(v)) return v;
    +      v = getVersionFromOsgiManifest();
    +      if (Strings.isNonBlank(v)) return v;
    +      return "0.0.0-SNAPSHOT";
       }
       
    -  public String getVersion() {
    -    return version;
    +  @Nullable
    +  public String getVersionFromMavenProperties() {
    +      return versionProperties.getProperty(MVN_VERSION_PROPERTY_NAME);
    +  }
    +
    +  @Nullable
    +  public String getVersionFromOsgiManifest() {
    +      return versionProperties.getProperty(OSGI_VERSION_PROPERTY_NAME);
       }
       
    -  public String getVersionFromStatic() {
    -    return versionFromStatic;
    +  @Nullable
    +  /** SHA1 of the last commit to brooklyn at the time this build was made.
    +   * For SNAPSHOT builds of course there may have been further non-committed changes. */
    +  public String getSha1FromOsgiManifest() {
    +      return versionProperties.getProperty(OSGI_SHA1_PROPERTY_NAME);
       }
    -
    -  public boolean isSnapshot() {
    -      return (getVersion().indexOf("-SNAPSHOT")>=0);
    +  
    +  public String getVersion() {
    +    return VERSION_FROM_STATIC;
       }
       
    -  private static boolean isValid(String v) {
    -    if (v==null) return false;
    -    if (v.equals("0.0.0") || v.equals("0.0")) return false;
    -    if (v.startsWith("0.0.0-") || v.startsWith("0.0-")) return false;
    -    return true;
    +  public boolean isSnapshot() {
    +      return (getVersion().indexOf("-SNAPSHOT")>=0);
       }
    -
    -  private String readVersionPropertyFromClasspath(ClassLoader resourceLoader) {
    -    Properties versionProperties = new Properties();
    +    
    +  private void readPropertiesFromMavenResource(ClassLoader resourceLoader) {
         try {
    -      InputStream versionStream = resourceLoader.getResourceAsStream(VERSION_RESOURCE_FILE);
    -      if (versionStream==null) return null;
    +      InputStream versionStream = resourceLoader.getResourceAsStream(MVN_VERSION_RESOURCE_FILE);
    --- End diff --
    
    Stream not closed (I see it was like this, bug this is a good time to fix it).


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-brooklyn pull request: CLI better default, port/https co...

Posted by ahgittin <gi...@git.apache.org>.
GitHub user ahgittin reopened a pull request:

    https://github.com/apache/incubator-brooklyn/pull/478

    CLI better default, port/https control, and version reporting

    * adds config key for web-console port
    * adds CLI for https
    * port defaults to 8443+ if no port specified and using https
    * default if no CLI options is to show help and an error
    * some https test fixes
    * discovers and reports Brooklyn OSGi metadata in log, including git SHA1 (and `brooklyn info`)

You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/ahgittin/incubator-brooklyn cli-ports-and-more

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/incubator-brooklyn/pull/478.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #478
    
----
commit d423522cacf97c3fefa0df3c641718a49f9103a5
Author: Alex Heneveld <al...@cloudsoftcorp.com>
Date:   2015-01-27T17:47:41Z

    CLI better default, port/https control, and version reporting
    
    * adds config key for web-console port
    * adds CLI for https
    * port defaults to 8443+ if no port specified and using https
    * default if no CLI options is to show help and an error
    * some https test fixes
    * discovers and reports Brooklyn OSGi metadata in log, including git SHA1 (and `brooklyn info`)

----


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-brooklyn pull request: CLI better default, port/https co...

Posted by neykov <gi...@git.apache.org>.
Github user neykov commented on a diff in the pull request:

    https://github.com/apache/incubator-brooklyn/pull/478#discussion_r24330979
  
    --- Diff: core/src/main/java/brooklyn/BrooklynVersion.java ---
    @@ -19,80 +19,218 @@
     package brooklyn;
     
     import static com.google.common.base.Preconditions.checkNotNull;
    -import static java.lang.String.format;
     
     import java.io.IOException;
     import java.io.InputStream;
    +import java.net.URL;
    +import java.util.Enumeration;
     import java.util.Properties;
    +import java.util.concurrent.atomic.AtomicReference;
    +import java.util.jar.Attributes;
    +
    +import javax.annotation.Nullable;
     
     import org.slf4j.Logger;
     import org.slf4j.LoggerFactory;
     
    +import brooklyn.util.exceptions.Exceptions;
    +import brooklyn.util.osgi.Osgis;
    +import brooklyn.util.osgi.Osgis.ManifestHelper;
    +import brooklyn.util.text.Strings;
    +
    +/**
    + * Wraps the version of Brooklyn.
    + * <p>
    + * Also retrieves the SHA-1 from any OSGi bundle, and checks that the maven and osgi versions match.
    + */
     public class BrooklynVersion {
     
       private static final Logger log = LoggerFactory.getLogger(BrooklynVersion.class);
       
    -  private static final String VERSION_RESOURCE_FILE = "META-INF/maven/io.brooklyn/brooklyn-core/pom.properties";
    -  private static final String VERSION_PROPERTY_NAME = "version";
    +  private static final String MVN_VERSION_RESOURCE_FILE = "META-INF/maven/org.apache.brooklyn/brooklyn-core/pom.properties";
    +  private static final String MANIFEST_PATH = "META-INF/MANIFEST.MF";
    +  private static final String BROOKLYN_CORE_SYMBOLIC_NAME = "org.apache.brooklyn.core";
    +  
    +  private static final String MVN_VERSION_PROPERTY_NAME = "version";
    +  private static final String OSGI_VERSION_PROPERTY_NAME = Attributes.Name.IMPLEMENTATION_VERSION.toString();
    +  private static final String OSGI_SHA1_PROPERTY_NAME = "Implementation-SHA-1";
    +
     
    +  private final static String VERSION_FROM_STATIC = "0.7.0-SNAPSHOT"; // BROOKLYN_VERSION
    +  private static final AtomicReference<Boolean> IS_DEV_ENV = new AtomicReference<Boolean>();
    +  
       public static final BrooklynVersion INSTANCE = new BrooklynVersion();
    +  
    +  private final Properties versionProperties = new Properties();
    +  
    +  public BrooklynVersion() {
    +      // we read the maven pom metadata and osgi metadata and make sure it's sensible
    +      // everything is put into a single map for now (good enough, but should be cleaned up)
    +      readPropertiesFromMavenResource(BrooklynVersion.class.getClassLoader());
    +      readPropertiesFromOsgiResource(BrooklynVersion.class.getClassLoader(), "org.apache.brooklyn.core");
    +      // TODO there is also build-metadata.properties used in ServerResource /v1/server/version endpoint
    +      // see comments on that about folding it into this class instead
     
    -  private final String versionFromClasspath;
    -  // static useful when running from the IDE
    -  // TODO is the classpath version ever useful? should we always use the static?
    -  private final String versionFromStatic = "0.7.0-SNAPSHOT"; // BROOKLYN_VERSION
    -  private final String version;
    +      checkVersions();
    +  }
     
    -  public BrooklynVersion() {
    -    this.versionFromClasspath = readVersionPropertyFromClasspath(BrooklynVersion.class.getClassLoader());
    -    if (isValid(versionFromClasspath)) {
    -        this.version = versionFromClasspath;
    -        if (!this.version.equals(versionFromStatic)) {
    -            // should always be the same, and we can drop classpath detection ...
    -            log.warn("Version detected from classpath as "+versionFromClasspath+" (preferring that), but in code it is recorded as "+versionFromStatic);
    -        }
    -    } else {
    -        this.version = versionFromStatic;
    -    }
    +  public void checkVersions() {
    +      String mvnVersion = getVersionFromMavenProperties();
    +      if (mvnVersion!=null && !VERSION_FROM_STATIC.equals(mvnVersion)) {
    +          throw new IllegalStateException("Version error: maven "+mvnVersion+" / code "+VERSION_FROM_STATIC);
    +      }
    +      
    +      String osgiVersion = versionProperties.getProperty(OSGI_VERSION_PROPERTY_NAME);
    +      // TODO does the OSGi version include other slightly differ gubbins/style ?
    +      if (osgiVersion!=null && !VERSION_FROM_STATIC.equals(osgiVersion)) {
    +          throw new IllegalStateException("Version error: osgi "+osgiVersion+" / code "+VERSION_FROM_STATIC);
    +      }
       }
    -  
    +
    +  /** Returns version as inferred from classpath/osgi, if possible, or 0.0.0-SNAPSHOT.
    +   * See also {@link #getVersionFromMavenProperties()} and {@link #getVersionFromOsgiManifest()}.
    +   * @deprecated since 0.7.0, in favour of the more specific methods (and does anyone need that default value?)
    +   */
    +  @Deprecated
       public String getVersionFromClasspath() {
    -    return versionFromClasspath;
    +      String v = getVersionFromMavenProperties();
    +      if (Strings.isNonBlank(v)) return v;
    +      v = getVersionFromOsgiManifest();
    +      if (Strings.isNonBlank(v)) return v;
    +      return "0.0.0-SNAPSHOT";
       }
       
    -  public String getVersion() {
    -    return version;
    +  @Nullable
    +  public String getVersionFromMavenProperties() {
    +      return versionProperties.getProperty(MVN_VERSION_PROPERTY_NAME);
    +  }
    +
    +  @Nullable
    +  public String getVersionFromOsgiManifest() {
    +      return versionProperties.getProperty(OSGI_VERSION_PROPERTY_NAME);
       }
       
    -  public String getVersionFromStatic() {
    -    return versionFromStatic;
    +  @Nullable
    +  /** SHA1 of the last commit to brooklyn at the time this build was made.
    +   * For SNAPSHOT builds of course there may have been further non-committed changes. */
    +  public String getSha1FromOsgiManifest() {
    +      return versionProperties.getProperty(OSGI_SHA1_PROPERTY_NAME);
       }
    -
    -  public boolean isSnapshot() {
    -      return (getVersion().indexOf("-SNAPSHOT")>=0);
    +  
    +  public String getVersion() {
    +    return VERSION_FROM_STATIC;
       }
       
    -  private static boolean isValid(String v) {
    -    if (v==null) return false;
    -    if (v.equals("0.0.0") || v.equals("0.0")) return false;
    -    if (v.startsWith("0.0.0-") || v.startsWith("0.0-")) return false;
    -    return true;
    +  public boolean isSnapshot() {
    +      return (getVersion().indexOf("-SNAPSHOT")>=0);
       }
    -
    -  private String readVersionPropertyFromClasspath(ClassLoader resourceLoader) {
    -    Properties versionProperties = new Properties();
    +    
    +  private void readPropertiesFromMavenResource(ClassLoader resourceLoader) {
         try {
    -      InputStream versionStream = resourceLoader.getResourceAsStream(VERSION_RESOURCE_FILE);
    -      if (versionStream==null) return null;
    +      InputStream versionStream = resourceLoader.getResourceAsStream(MVN_VERSION_RESOURCE_FILE);
    +      if (versionStream==null) {
    +          if (isDevelopmentEnvironment()) {
    +              // allowed for dev env
    +              log.trace("No maven resource file "+MVN_VERSION_RESOURCE_FILE+" available");
    +          } else {
    +              log.warn("No maven resource file "+MVN_VERSION_RESOURCE_FILE+" available");
    +          }
    +          return;
    +      }
           versionProperties.load(checkNotNull(versionStream));
    -    } catch (IOException exception) {
    -      throw new IllegalStateException(format("Unable to load version resource file '%s'", VERSION_RESOURCE_FILE), exception);
    +    } catch (IOException e) {
    +      log.warn("Error reading maven resource file "+MVN_VERSION_RESOURCE_FILE+": "+e, e);
         }
    -    return checkNotNull(versionProperties.getProperty(VERSION_PROPERTY_NAME), VERSION_PROPERTY_NAME);
    +  }
    +
    +  /** reads properties from brooklyn-core's manifest */
    +  private void readPropertiesFromOsgiResource(ClassLoader resourceLoader, String symbolicName) {
    +      Enumeration<URL> paths;
    +      try {
    +          paths = BrooklynVersion.class.getClassLoader().getResources(MANIFEST_PATH);
    +      } catch (IOException e) {
    +          // shouldn't happen
    +          throw Exceptions.propagate(e);
    +      }
    +      while (paths.hasMoreElements()) {
    +          URL u = paths.nextElement();
    +          try {
    +              ManifestHelper mh = Osgis.ManifestHelper.forManifest(u.openStream());
    +              if (BROOKLYN_CORE_SYMBOLIC_NAME.equals(mh.getSymbolicName())) {
    +                  Attributes attrs = mh.getManifest().getMainAttributes();
    +                  for (Object key: attrs.keySet()) {
    +                      // key is an Attribute.Name; toString converts to string
    +                      versionProperties.put(key.toString(), attrs.getValue(key.toString()));
    +                  }
    +                  return;
    +              }
    +          } catch (Exception e) {
    +              Exceptions.propagateIfFatal(e);
    +              log.warn("Error reading OSGi manifest from "+u+" when determining version properties: "+e, e);
    +          }
    +      }
    +      if (isDevelopmentEnvironment()) {
    +          // allowed for dev env
    +          log.trace("No OSGi manifest available to determine version properties");
    +      } else {
    +          log.warn("No OSGi manifest available to determine version properties");
    +      }
    +  }
    +
    +  /** 
    +   * Returns whether this is a Brooklyn dev environment,
    +   * specifically core/target/classes/ is on the classpath for the org.apache.brooklyn.core project.
    +   * <p>
    +   * In a packaged or library build of Brooklyn (normal usage) this should return false,
    +   * and all OSGi components should be available.
    +   */
    +  public static boolean isDevelopmentEnvironment() {
    +      Boolean isDevEnv = IS_DEV_ENV.get();
    +      if (isDevEnv!=null) return isDevEnv;
    +      synchronized (IS_DEV_ENV) {
    +          isDevEnv = computeIsDevelopmentEnvironment();
    +          IS_DEV_ENV.set(isDevEnv);
    +          return isDevEnv;
    +      }
    +  }
    +  
    +  private static boolean computeIsDevelopmentEnvironment() {
    +      Enumeration<URL> paths;
    +      try {
    +          paths = BrooklynVersion.class.getClassLoader().getResources(MANIFEST_PATH);
    +      } catch (IOException e) {
    +          // shouldn't happen
    +          throw Exceptions.propagate(e);
    +      }
    +      while (paths.hasMoreElements()) {
    +          URL u = paths.nextElement();
    +          if (u.getPath().endsWith("core/target/classes/META-INF/MANIFEST.MF")) {
    +              try {
    +                  ManifestHelper mh = Osgis.ManifestHelper.forManifest(u.openStream());
    --- End diff --
    
    Unclosed stream


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-brooklyn pull request: CLI better default, port/https co...

Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:

    https://github.com/apache/incubator-brooklyn/pull/478


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-brooklyn pull request: CLI better default, port/https co...

Posted by ahgittin <gi...@git.apache.org>.
Github user ahgittin closed the pull request at:

    https://github.com/apache/incubator-brooklyn/pull/478


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-brooklyn pull request: CLI better default, port/https co...

Posted by neykov <gi...@git.apache.org>.
Github user neykov commented on a diff in the pull request:

    https://github.com/apache/incubator-brooklyn/pull/478#discussion_r24330235
  
    --- Diff: core/src/main/java/brooklyn/BrooklynVersion.java ---
    @@ -19,80 +19,218 @@
     package brooklyn;
     
     import static com.google.common.base.Preconditions.checkNotNull;
    -import static java.lang.String.format;
     
     import java.io.IOException;
     import java.io.InputStream;
    +import java.net.URL;
    +import java.util.Enumeration;
     import java.util.Properties;
    +import java.util.concurrent.atomic.AtomicReference;
    +import java.util.jar.Attributes;
    +
    +import javax.annotation.Nullable;
     
     import org.slf4j.Logger;
     import org.slf4j.LoggerFactory;
     
    +import brooklyn.util.exceptions.Exceptions;
    +import brooklyn.util.osgi.Osgis;
    +import brooklyn.util.osgi.Osgis.ManifestHelper;
    +import brooklyn.util.text.Strings;
    +
    +/**
    + * Wraps the version of Brooklyn.
    + * <p>
    + * Also retrieves the SHA-1 from any OSGi bundle, and checks that the maven and osgi versions match.
    + */
     public class BrooklynVersion {
     
       private static final Logger log = LoggerFactory.getLogger(BrooklynVersion.class);
       
    -  private static final String VERSION_RESOURCE_FILE = "META-INF/maven/io.brooklyn/brooklyn-core/pom.properties";
    -  private static final String VERSION_PROPERTY_NAME = "version";
    +  private static final String MVN_VERSION_RESOURCE_FILE = "META-INF/maven/org.apache.brooklyn/brooklyn-core/pom.properties";
    +  private static final String MANIFEST_PATH = "META-INF/MANIFEST.MF";
    +  private static final String BROOKLYN_CORE_SYMBOLIC_NAME = "org.apache.brooklyn.core";
    +  
    +  private static final String MVN_VERSION_PROPERTY_NAME = "version";
    +  private static final String OSGI_VERSION_PROPERTY_NAME = Attributes.Name.IMPLEMENTATION_VERSION.toString();
    +  private static final String OSGI_SHA1_PROPERTY_NAME = "Implementation-SHA-1";
    +
     
    +  private final static String VERSION_FROM_STATIC = "0.7.0-SNAPSHOT"; // BROOKLYN_VERSION
    +  private static final AtomicReference<Boolean> IS_DEV_ENV = new AtomicReference<Boolean>();
    +  
       public static final BrooklynVersion INSTANCE = new BrooklynVersion();
    +  
    +  private final Properties versionProperties = new Properties();
    +  
    +  public BrooklynVersion() {
    +      // we read the maven pom metadata and osgi metadata and make sure it's sensible
    +      // everything is put into a single map for now (good enough, but should be cleaned up)
    +      readPropertiesFromMavenResource(BrooklynVersion.class.getClassLoader());
    +      readPropertiesFromOsgiResource(BrooklynVersion.class.getClassLoader(), "org.apache.brooklyn.core");
    +      // TODO there is also build-metadata.properties used in ServerResource /v1/server/version endpoint
    +      // see comments on that about folding it into this class instead
     
    -  private final String versionFromClasspath;
    -  // static useful when running from the IDE
    -  // TODO is the classpath version ever useful? should we always use the static?
    -  private final String versionFromStatic = "0.7.0-SNAPSHOT"; // BROOKLYN_VERSION
    -  private final String version;
    +      checkVersions();
    +  }
     
    -  public BrooklynVersion() {
    -    this.versionFromClasspath = readVersionPropertyFromClasspath(BrooklynVersion.class.getClassLoader());
    -    if (isValid(versionFromClasspath)) {
    -        this.version = versionFromClasspath;
    -        if (!this.version.equals(versionFromStatic)) {
    -            // should always be the same, and we can drop classpath detection ...
    -            log.warn("Version detected from classpath as "+versionFromClasspath+" (preferring that), but in code it is recorded as "+versionFromStatic);
    -        }
    -    } else {
    -        this.version = versionFromStatic;
    -    }
    +  public void checkVersions() {
    +      String mvnVersion = getVersionFromMavenProperties();
    +      if (mvnVersion!=null && !VERSION_FROM_STATIC.equals(mvnVersion)) {
    +          throw new IllegalStateException("Version error: maven "+mvnVersion+" / code "+VERSION_FROM_STATIC);
    +      }
    +      
    +      String osgiVersion = versionProperties.getProperty(OSGI_VERSION_PROPERTY_NAME);
    +      // TODO does the OSGi version include other slightly differ gubbins/style ?
    +      if (osgiVersion!=null && !VERSION_FROM_STATIC.equals(osgiVersion)) {
    +          throw new IllegalStateException("Version error: osgi "+osgiVersion+" / code "+VERSION_FROM_STATIC);
    +      }
       }
    -  
    +
    +  /** Returns version as inferred from classpath/osgi, if possible, or 0.0.0-SNAPSHOT.
    +   * See also {@link #getVersionFromMavenProperties()} and {@link #getVersionFromOsgiManifest()}.
    +   * @deprecated since 0.7.0, in favour of the more specific methods (and does anyone need that default value?)
    +   */
    +  @Deprecated
       public String getVersionFromClasspath() {
    -    return versionFromClasspath;
    +      String v = getVersionFromMavenProperties();
    +      if (Strings.isNonBlank(v)) return v;
    +      v = getVersionFromOsgiManifest();
    +      if (Strings.isNonBlank(v)) return v;
    +      return "0.0.0-SNAPSHOT";
       }
       
    -  public String getVersion() {
    -    return version;
    +  @Nullable
    +  public String getVersionFromMavenProperties() {
    +      return versionProperties.getProperty(MVN_VERSION_PROPERTY_NAME);
    +  }
    +
    +  @Nullable
    +  public String getVersionFromOsgiManifest() {
    +      return versionProperties.getProperty(OSGI_VERSION_PROPERTY_NAME);
       }
       
    -  public String getVersionFromStatic() {
    -    return versionFromStatic;
    +  @Nullable
    +  /** SHA1 of the last commit to brooklyn at the time this build was made.
    +   * For SNAPSHOT builds of course there may have been further non-committed changes. */
    +  public String getSha1FromOsgiManifest() {
    +      return versionProperties.getProperty(OSGI_SHA1_PROPERTY_NAME);
       }
    -
    -  public boolean isSnapshot() {
    -      return (getVersion().indexOf("-SNAPSHOT")>=0);
    +  
    +  public String getVersion() {
    +    return VERSION_FROM_STATIC;
       }
       
    -  private static boolean isValid(String v) {
    -    if (v==null) return false;
    -    if (v.equals("0.0.0") || v.equals("0.0")) return false;
    -    if (v.startsWith("0.0.0-") || v.startsWith("0.0-")) return false;
    -    return true;
    +  public boolean isSnapshot() {
    +      return (getVersion().indexOf("-SNAPSHOT")>=0);
       }
    -
    -  private String readVersionPropertyFromClasspath(ClassLoader resourceLoader) {
    -    Properties versionProperties = new Properties();
    +    
    +  private void readPropertiesFromMavenResource(ClassLoader resourceLoader) {
         try {
    -      InputStream versionStream = resourceLoader.getResourceAsStream(VERSION_RESOURCE_FILE);
    -      if (versionStream==null) return null;
    +      InputStream versionStream = resourceLoader.getResourceAsStream(MVN_VERSION_RESOURCE_FILE);
    +      if (versionStream==null) {
    +          if (isDevelopmentEnvironment()) {
    +              // allowed for dev env
    +              log.trace("No maven resource file "+MVN_VERSION_RESOURCE_FILE+" available");
    +          } else {
    +              log.warn("No maven resource file "+MVN_VERSION_RESOURCE_FILE+" available");
    +          }
    +          return;
    +      }
           versionProperties.load(checkNotNull(versionStream));
    -    } catch (IOException exception) {
    -      throw new IllegalStateException(format("Unable to load version resource file '%s'", VERSION_RESOURCE_FILE), exception);
    +    } catch (IOException e) {
    +      log.warn("Error reading maven resource file "+MVN_VERSION_RESOURCE_FILE+": "+e, e);
         }
    -    return checkNotNull(versionProperties.getProperty(VERSION_PROPERTY_NAME), VERSION_PROPERTY_NAME);
    +  }
    +
    +  /** reads properties from brooklyn-core's manifest */
    +  private void readPropertiesFromOsgiResource(ClassLoader resourceLoader, String symbolicName) {
    +      Enumeration<URL> paths;
    +      try {
    +          paths = BrooklynVersion.class.getClassLoader().getResources(MANIFEST_PATH);
    +      } catch (IOException e) {
    +          // shouldn't happen
    +          throw Exceptions.propagate(e);
    +      }
    +      while (paths.hasMoreElements()) {
    +          URL u = paths.nextElement();
    +          try {
    +              ManifestHelper mh = Osgis.ManifestHelper.forManifest(u.openStream());
    --- End diff --
    
    Close the stream - locks the .jar on Windows, leaks file handles on Linux.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-brooklyn pull request: CLI better default, port/https co...

Posted by neykov <gi...@git.apache.org>.
Github user neykov commented on a diff in the pull request:

    https://github.com/apache/incubator-brooklyn/pull/478#discussion_r24331580
  
    --- Diff: usage/launcher/src/main/java/brooklyn/launcher/BrooklynWebServer.java ---
    @@ -212,6 +212,9 @@ public void setSecurityFilter(Class<BrooklynPropertiesSecurityFilter> filterClaz
         }
     
         public BrooklynWebServer setPort(Object port) {
    +        if (port==null) {
    +            this.requestedPort = null;
    +        }
    --- End diff --
    
    Not really needed, `coerce` below` will set `requestedPort` to null anyway.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-brooklyn pull request: CLI better default, port/https co...

Posted by ahgittin <gi...@git.apache.org>.
Github user ahgittin commented on a diff in the pull request:

    https://github.com/apache/incubator-brooklyn/pull/478#discussion_r24348520
  
    --- Diff: usage/launcher/src/main/java/brooklyn/launcher/BrooklynWebServer.java ---
    @@ -338,7 +346,10 @@ public synchronized void start() throws Exception {
             if (actualPort == -1){
                 PortRange portRange = requestedPort;
                 if (portRange==null) {
    -                portRange = getHttpsEnabled()? httpsPort : httpPort;
    +                portRange = managementContext.getConfig().getConfig(BrooklynWebConfig.WEB_CONSOLE_PORT);
    +            }
    +            if (portRange==null) {
    +                portRange = getHttpsEnabled() ? httpsPort : httpPort;
    --- End diff --
    
    good point - i will add this to the release notes.  pretty sure this new default behaviour is the right thing to do...


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-brooklyn pull request: CLI better default, port/https co...

Posted by neykov <gi...@git.apache.org>.
Github user neykov commented on a diff in the pull request:

    https://github.com/apache/incubator-brooklyn/pull/478#discussion_r24332021
  
    --- Diff: usage/launcher/src/main/java/brooklyn/launcher/BrooklynWebServer.java ---
    @@ -338,7 +346,10 @@ public synchronized void start() throws Exception {
             if (actualPort == -1){
                 PortRange portRange = requestedPort;
                 if (portRange==null) {
    -                portRange = getHttpsEnabled()? httpsPort : httpPort;
    +                portRange = managementContext.getConfig().getConfig(BrooklynWebConfig.WEB_CONSOLE_PORT);
    +            }
    +            if (portRange==null) {
    +                portRange = getHttpsEnabled() ? httpsPort : httpPort;
    --- End diff --
    
    Backwards compatibility warning: if `brooklyn.webconsole.security.https.required=true` was used previously without explicit port, the server would still start on 8081.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-brooklyn pull request: CLI better default, port/https co...

Posted by ahgittin <gi...@git.apache.org>.
Github user ahgittin commented on the pull request:

    https://github.com/apache/incubator-brooklyn/pull/478#issuecomment-71739932
  
    failure due to:
    
    ```
    2015-01-27 18:03:32,625 INFO  TESTNG FAILED: "Surefire test" - brooklyn.launcher.BrooklynWebServerTest.verifyHttps() finished in 7 ms
    java.lang.IllegalArgumentException: Unable to access URL brooklyn.webconsole.security.keystore.url: /home/jenkins/jenkins-slave/workspace/incubator-brooklyn-pull-requests%402/usage/launcher/target/test-classes/server.ks
    	at brooklyn.util.ResourceUtils.checkUrlExists(ResourceUtils.java:487)
    	at brooklyn.launcher.BrooklynWebServer.start(BrooklynWebServer.java:398)
    	at brooklyn.launcher.BrooklynWebServerTest.verifyHttps(BrooklynWebServerTest.java:100)
    Caused by: java.lang.RuntimeException: Error getting resource '/home/jenkins/jenkins-slave/workspace/incubator-brooklyn-pull-requests%402/usage/launcher/target/test-classes/server.ks' for brooklyn.launcher.BrooklynWebServer@19289a7: java.io.IOException: '/home/jenkins/jenkins-slave/workspace/incubator-brooklyn-pull-requests%402/usage/launcher/target/test-classes/server.ks' not found on classpath or filesystem
    	at brooklyn.util.ResourceUtils.getResourceFromUrl(ResourceUtils.java:290)
    ```
    
    i don't think this was introduced here and can't see why it is happening; the file is in `src/test/resources`.
    
    closing an re-opening to see if that helps...


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-brooklyn pull request: CLI better default, port/https co...

Posted by neykov <gi...@git.apache.org>.
Github user neykov commented on a diff in the pull request:

    https://github.com/apache/incubator-brooklyn/pull/478#discussion_r24330002
  
    --- Diff: core/src/main/java/brooklyn/BrooklynVersion.java ---
    @@ -19,80 +19,218 @@
     package brooklyn;
     
     import static com.google.common.base.Preconditions.checkNotNull;
    -import static java.lang.String.format;
     
     import java.io.IOException;
     import java.io.InputStream;
    +import java.net.URL;
    +import java.util.Enumeration;
     import java.util.Properties;
    +import java.util.concurrent.atomic.AtomicReference;
    +import java.util.jar.Attributes;
    +
    +import javax.annotation.Nullable;
     
     import org.slf4j.Logger;
     import org.slf4j.LoggerFactory;
     
    +import brooklyn.util.exceptions.Exceptions;
    +import brooklyn.util.osgi.Osgis;
    +import brooklyn.util.osgi.Osgis.ManifestHelper;
    +import brooklyn.util.text.Strings;
    +
    +/**
    + * Wraps the version of Brooklyn.
    + * <p>
    + * Also retrieves the SHA-1 from any OSGi bundle, and checks that the maven and osgi versions match.
    + */
     public class BrooklynVersion {
     
       private static final Logger log = LoggerFactory.getLogger(BrooklynVersion.class);
       
    -  private static final String VERSION_RESOURCE_FILE = "META-INF/maven/io.brooklyn/brooklyn-core/pom.properties";
    -  private static final String VERSION_PROPERTY_NAME = "version";
    +  private static final String MVN_VERSION_RESOURCE_FILE = "META-INF/maven/org.apache.brooklyn/brooklyn-core/pom.properties";
    +  private static final String MANIFEST_PATH = "META-INF/MANIFEST.MF";
    +  private static final String BROOKLYN_CORE_SYMBOLIC_NAME = "org.apache.brooklyn.core";
    +  
    +  private static final String MVN_VERSION_PROPERTY_NAME = "version";
    +  private static final String OSGI_VERSION_PROPERTY_NAME = Attributes.Name.IMPLEMENTATION_VERSION.toString();
    +  private static final String OSGI_SHA1_PROPERTY_NAME = "Implementation-SHA-1";
    +
     
    +  private final static String VERSION_FROM_STATIC = "0.7.0-SNAPSHOT"; // BROOKLYN_VERSION
    +  private static final AtomicReference<Boolean> IS_DEV_ENV = new AtomicReference<Boolean>();
    +  
       public static final BrooklynVersion INSTANCE = new BrooklynVersion();
    +  
    +  private final Properties versionProperties = new Properties();
    +  
    +  public BrooklynVersion() {
    +      // we read the maven pom metadata and osgi metadata and make sure it's sensible
    +      // everything is put into a single map for now (good enough, but should be cleaned up)
    +      readPropertiesFromMavenResource(BrooklynVersion.class.getClassLoader());
    +      readPropertiesFromOsgiResource(BrooklynVersion.class.getClassLoader(), "org.apache.brooklyn.core");
    +      // TODO there is also build-metadata.properties used in ServerResource /v1/server/version endpoint
    +      // see comments on that about folding it into this class instead
     
    -  private final String versionFromClasspath;
    -  // static useful when running from the IDE
    -  // TODO is the classpath version ever useful? should we always use the static?
    -  private final String versionFromStatic = "0.7.0-SNAPSHOT"; // BROOKLYN_VERSION
    -  private final String version;
    +      checkVersions();
    +  }
     
    -  public BrooklynVersion() {
    -    this.versionFromClasspath = readVersionPropertyFromClasspath(BrooklynVersion.class.getClassLoader());
    -    if (isValid(versionFromClasspath)) {
    -        this.version = versionFromClasspath;
    -        if (!this.version.equals(versionFromStatic)) {
    -            // should always be the same, and we can drop classpath detection ...
    -            log.warn("Version detected from classpath as "+versionFromClasspath+" (preferring that), but in code it is recorded as "+versionFromStatic);
    -        }
    -    } else {
    -        this.version = versionFromStatic;
    -    }
    +  public void checkVersions() {
    +      String mvnVersion = getVersionFromMavenProperties();
    +      if (mvnVersion!=null && !VERSION_FROM_STATIC.equals(mvnVersion)) {
    +          throw new IllegalStateException("Version error: maven "+mvnVersion+" / code "+VERSION_FROM_STATIC);
    +      }
    +      
    +      String osgiVersion = versionProperties.getProperty(OSGI_VERSION_PROPERTY_NAME);
    +      // TODO does the OSGi version include other slightly differ gubbins/style ?
    +      if (osgiVersion!=null && !VERSION_FROM_STATIC.equals(osgiVersion)) {
    +          throw new IllegalStateException("Version error: osgi "+osgiVersion+" / code "+VERSION_FROM_STATIC);
    +      }
       }
    -  
    +
    +  /** Returns version as inferred from classpath/osgi, if possible, or 0.0.0-SNAPSHOT.
    +   * See also {@link #getVersionFromMavenProperties()} and {@link #getVersionFromOsgiManifest()}.
    +   * @deprecated since 0.7.0, in favour of the more specific methods (and does anyone need that default value?)
    +   */
    +  @Deprecated
       public String getVersionFromClasspath() {
    -    return versionFromClasspath;
    +      String v = getVersionFromMavenProperties();
    +      if (Strings.isNonBlank(v)) return v;
    +      v = getVersionFromOsgiManifest();
    +      if (Strings.isNonBlank(v)) return v;
    +      return "0.0.0-SNAPSHOT";
       }
       
    -  public String getVersion() {
    -    return version;
    +  @Nullable
    +  public String getVersionFromMavenProperties() {
    +      return versionProperties.getProperty(MVN_VERSION_PROPERTY_NAME);
    +  }
    +
    +  @Nullable
    +  public String getVersionFromOsgiManifest() {
    +      return versionProperties.getProperty(OSGI_VERSION_PROPERTY_NAME);
       }
       
    -  public String getVersionFromStatic() {
    -    return versionFromStatic;
    +  @Nullable
    +  /** SHA1 of the last commit to brooklyn at the time this build was made.
    +   * For SNAPSHOT builds of course there may have been further non-committed changes. */
    +  public String getSha1FromOsgiManifest() {
    +      return versionProperties.getProperty(OSGI_SHA1_PROPERTY_NAME);
       }
    -
    -  public boolean isSnapshot() {
    -      return (getVersion().indexOf("-SNAPSHOT")>=0);
    +  
    +  public String getVersion() {
    +    return VERSION_FROM_STATIC;
       }
       
    -  private static boolean isValid(String v) {
    -    if (v==null) return false;
    -    if (v.equals("0.0.0") || v.equals("0.0")) return false;
    -    if (v.startsWith("0.0.0-") || v.startsWith("0.0-")) return false;
    -    return true;
    +  public boolean isSnapshot() {
    +      return (getVersion().indexOf("-SNAPSHOT")>=0);
       }
    -
    -  private String readVersionPropertyFromClasspath(ClassLoader resourceLoader) {
    -    Properties versionProperties = new Properties();
    +    
    +  private void readPropertiesFromMavenResource(ClassLoader resourceLoader) {
         try {
    -      InputStream versionStream = resourceLoader.getResourceAsStream(VERSION_RESOURCE_FILE);
    -      if (versionStream==null) return null;
    +      InputStream versionStream = resourceLoader.getResourceAsStream(MVN_VERSION_RESOURCE_FILE);
    +      if (versionStream==null) {
    +          if (isDevelopmentEnvironment()) {
    +              // allowed for dev env
    +              log.trace("No maven resource file "+MVN_VERSION_RESOURCE_FILE+" available");
    +          } else {
    +              log.warn("No maven resource file "+MVN_VERSION_RESOURCE_FILE+" available");
    +          }
    +          return;
    +      }
           versionProperties.load(checkNotNull(versionStream));
    -    } catch (IOException exception) {
    -      throw new IllegalStateException(format("Unable to load version resource file '%s'", VERSION_RESOURCE_FILE), exception);
    +    } catch (IOException e) {
    +      log.warn("Error reading maven resource file "+MVN_VERSION_RESOURCE_FILE+": "+e, e);
         }
    -    return checkNotNull(versionProperties.getProperty(VERSION_PROPERTY_NAME), VERSION_PROPERTY_NAME);
    +  }
    +
    +  /** reads properties from brooklyn-core's manifest */
    +  private void readPropertiesFromOsgiResource(ClassLoader resourceLoader, String symbolicName) {
    +      Enumeration<URL> paths;
    +      try {
    +          paths = BrooklynVersion.class.getClassLoader().getResources(MANIFEST_PATH);
    +      } catch (IOException e) {
    +          // shouldn't happen
    +          throw Exceptions.propagate(e);
    +      }
    +      while (paths.hasMoreElements()) {
    +          URL u = paths.nextElement();
    +          try {
    +              ManifestHelper mh = Osgis.ManifestHelper.forManifest(u.openStream());
    +              if (BROOKLYN_CORE_SYMBOLIC_NAME.equals(mh.getSymbolicName())) {
    +                  Attributes attrs = mh.getManifest().getMainAttributes();
    +                  for (Object key: attrs.keySet()) {
    +                      // key is an Attribute.Name; toString converts to string
    +                      versionProperties.put(key.toString(), attrs.getValue(key.toString()));
    +                  }
    +                  return;
    +              }
    +          } catch (Exception e) {
    +              Exceptions.propagateIfFatal(e);
    +              log.warn("Error reading OSGi manifest from "+u+" when determining version properties: "+e, e);
    +          }
    +      }
    +      if (isDevelopmentEnvironment()) {
    +          // allowed for dev env
    +          log.trace("No OSGi manifest available to determine version properties");
    +      } else {
    +          log.warn("No OSGi manifest available to determine version properties");
    +      }
    +  }
    +
    +  /** 
    +   * Returns whether this is a Brooklyn dev environment,
    +   * specifically core/target/classes/ is on the classpath for the org.apache.brooklyn.core project.
    +   * <p>
    +   * In a packaged or library build of Brooklyn (normal usage) this should return false,
    +   * and all OSGi components should be available.
    +   */
    +  public static boolean isDevelopmentEnvironment() {
    +      Boolean isDevEnv = IS_DEV_ENV.get();
    +      if (isDevEnv!=null) return isDevEnv;
    +      synchronized (IS_DEV_ENV) {
    +          isDevEnv = computeIsDevelopmentEnvironment();
    +          IS_DEV_ENV.set(isDevEnv);
    +          return isDevEnv;
    +      }
    +  }
    +  
    +  private static boolean computeIsDevelopmentEnvironment() {
    --- End diff --
    
    Re-use in brooklyn.launcher.config.BrooklynDevelopmentModes.computeAutodectectedDevelopmentMode().


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---