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.

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

Upgraded JChem to look at Repository for JAR #349

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
21 changes: 11 additions & 10 deletions Dockerfile
Original file line number Diff line number Diff line change
@@ -1,23 +1,24 @@
ARG CHEMISTRY_PACKAGE=jchem
FROM maven:3-openjdk-8 as builder
FROM maven:3-openjdk-11 as builder

FROM builder as dependencies
ARG CHEMISTRY_PACKAGE
ENV CHEMISTRY_PACKAGE=${CHEMISTRY_PACKAGE}

FROM dependencies as jchem
ADD lib/jchem-16.4.25.0.jar /lib/jchem-16.4.25.0.jar
RUN mvn install:install-file -Dfile=/lib/jchem-16.4.25.0.jar -DartifactId=jchem -DgroupId=com.chemaxon -Dversion=16.4.25.0 -Dpackaging=jar -DgeneratePom=true -DcreateChecksum=true
# ADD lib/jchem-16.4.25.0.jar /lib/jchem-16.4.25.0.jar
Copy link
Collaborator

Choose a reason for hiding this comment

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

Previously we'd been assuming this jar file would be supplied despite it not being in the repo. Perhaps you could do something similar for the API keys?

# RUN mvn install:install-file -Dfile=/lib/jchem-16.4.25.0.jar -DartifactId=jchem -DgroupId=com.chemaxon -Dversion=16.4.25.0 -Dpackaging=jar -DgeneratePom=true -DcreateChecksum=true

FROM ${CHEMISTRY_PACKAGE} as compile
ADD settings.xml /root/.m2/settings.xml
Copy link
Collaborator

Choose a reason for hiding this comment

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

What's this file? I don't see it in the repo. Is this the API key?

ADD pom.xml /src/pom.xml
WORKDIR /src
RUN mvn dependency:resolve -P ${CHEMISTRY_PACKAGE}
RUN mvn dependency:resolve -P ${CHEMISTRY_PACKAGE} -X
Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't think we need the maven debug flag on permanently. Can this be removed once you've finished debugging?

ADD . /src
RUN mvn clean && \
mvn compile war:war -P ${CHEMISTRY_PACKAGE}

FROM tomcat:9.0.62-jre8-openjdk-slim-buster
mvn compile war:war -P ${CHEMISTRY_PACKAGE} -X
# RUN mvn compile war:war -P ${CHEMISTRY_PACKAGE}
FROM tomcat:9.0.62-jre11-openjdk-slim-buster

RUN apt-get update && \
apt-get install -y openssl libfontconfig libfreetype6 curl
Expand All @@ -36,10 +37,10 @@ ENV NODE_PATH /usr/lib/node_modules
RUN useradd -u 1000 -ms /bin/bash runner

# Allow certificates to be added by runner
RUN chgrp runner /usr/local/openjdk-8/lib/security/cacerts && \
chmod g+w /usr/local/openjdk-8/lib/security/cacerts && \
RUN chgrp runner /usr/local/openjdk-11/lib/security/cacerts && \
chmod g+w /usr/local/openjdk-11/lib/security/cacerts && \
mkdir -p /usr/lib/jvm/java/jre/lib/security && \
ln -s /usr/local/openjdk-8/lib/security/cacerts /usr/lib/jvm/java/jre/lib/security/cacerts
ln -s /usr/local/openjdk-11/lib/security/cacerts /usr/lib/jvm/java/jre/lib/security/cacerts

# Get acas-roo-server compiled code
COPY --chown=runner:runner --from=compile /src/target/acas*.war /usr/local/tomcat/webapps/acas.war
Expand Down
22 changes: 7 additions & 15 deletions pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ along with this program. If not, see <http://www.gnu.org/licenses/>.]]>
<description>ACAS Java persistence layer</description>
<properties>
<aspectj.version>1.9.7</aspectj.version>
<java.version>1.8</java.version>
<java.version>11</java.version>
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is a big change. Was this required for the new JChem version?

<project.build.sourceEncoding>UTF-8</project.build.sourceEncoding>
<maven.build.timestamp.format>yyyyMMdd_HHmm</maven.build.timestamp.format>
<date>${maven.build.timestamp}</date>
Expand All @@ -58,6 +58,10 @@ along with this program. If not, see <http://www.gnu.org/licenses/>.]]>
<url>https://github.com/mcneilco/acas-roo-server.git</url>
</scm>
<repositories>
<repository>
Copy link
Collaborator

Choose a reason for hiding this comment

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

Could this <repository> be moved into the jchem build profile so it isn't contacted during non-jchem builds?

<id>Chemaxon Public Repository</id>
<url>https://hub.chemaxon.com/artifactory/libs-release</url>
</repository>
<repository>
<id>spring-release</id>
<name>Spring Release</name>
Expand Down Expand Up @@ -104,20 +108,8 @@ along with this program. If not, see <http://www.gnu.org/licenses/>.]]>
<dependencies>
<dependency>
<groupId>com.chemaxon</groupId>
<artifactId>jchem</artifactId>
<version>${jchem.version}</version>
</dependency>
<!-- JCHEM 16 requires dom4j 1.3. This depdendency can be removed when we upgrade jchem to jchem > 17 -->
<dependency>
<groupId>dom4j</groupId>
<artifactId>dom4j</artifactId>
<version>1.3</version>
</dependency>
<!-- JHEM 16 requires trove4j https://mvnrepository.com/artifact/net.sf.trove4j/trove4j -->
<dependency>
<groupId>net.sf.trove4j</groupId>
<artifactId>trove4j</artifactId>
<version>3.0.3</version>
<artifactId>jchem-main</artifactId>
<version>22.12.0</version>
</dependency>
</dependencies>
<properties>
Expand Down