You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@shiro.apache.org by GitBox <gi...@apache.org> on 2021/05/04 15:38:36 UTC
[GitHub] [shiro] bdemers opened a new pull request #297: 1.8 build issues
bdemers opened a new pull request #297:
URL: https://github.com/apache/shiro/pull/297
Fix version and build issues on 1.8 branch
--
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
[GitHub] [shiro] bdemers commented on a change in pull request #297: 1.8 build issues
Posted by GitBox <gi...@apache.org>.
bdemers commented on a change in pull request #297:
URL: https://github.com/apache/shiro/pull/297#discussion_r626674564
##########
File path: pom.xml
##########
@@ -110,7 +110,7 @@
<!-- Test 3rd-party dependencies: -->
<easymock.version>4.1</easymock.version>
<gmaven.version>1.8.0</gmaven.version>
- <groovy.version>2.5.8</groovy.version>
+ <groovy.version>2.5.14</groovy.version>
Review comment:
newer groovy uses an ASM version that supports Java 14
--
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
[GitHub] [shiro] bdemers commented on a change in pull request #297: 1.8 build issues
Posted by GitBox <gi...@apache.org>.
bdemers commented on a change in pull request #297:
URL: https://github.com/apache/shiro/pull/297#discussion_r626677427
##########
File path: pom.xml
##########
@@ -1533,6 +1533,7 @@
<plugin>
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-invoker-plugin</artifactId>
+ <version>3.2.2</version>
Review comment:
starting with this version `streamLogsOnFailures` has been added (to help with logging output for CI usage)
--
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
[GitHub] [shiro] bdemers commented on a change in pull request #297: 1.8 build issues
Posted by GitBox <gi...@apache.org>.
bdemers commented on a change in pull request #297:
URL: https://github.com/apache/shiro/pull/297#discussion_r626673840
##########
File path: Jenkinsfile
##########
@@ -72,14 +72,15 @@ pipeline {
stage('Build') {
steps {
echo 'Building'
- sh 'mvn -U -B -e clean install -DskipTests apache-rat:check'
+ sh 'mvn -U -B -e apache-rat:check'
+ sh 'mvn -U -B -e ${MVN_LOCAL_REPO_OPT} clean install -DskipTests'
}
}
stage('Tests') {
steps {
echo 'Running tests'
- sh 'mvn verify -Prun-its'
+ sh 'mvn verify -Prun-its ${MVN_LOCAL_REPO_OPT} -Dinvoker.streamLogsOnFailures=true'
Review comment:
Should we stick the `invoker.streamLogsOnFailures=true` in the pom?
```xml
<streamLogsOnFailures>true</streamLogsOnFailures>
```
--
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
[GitHub] [shiro] bmarwell commented on a change in pull request #297: 1.8 build issues
Posted by GitBox <gi...@apache.org>.
bmarwell commented on a change in pull request #297:
URL: https://github.com/apache/shiro/pull/297#discussion_r626699100
##########
File path: Jenkinsfile
##########
@@ -72,14 +72,15 @@ pipeline {
stage('Build') {
steps {
echo 'Building'
- sh 'mvn -U -B -e clean install -DskipTests apache-rat:check'
+ sh 'mvn -U -B -e apache-rat:check'
+ sh 'mvn -U -B -e ${MVN_LOCAL_REPO_OPT} clean install -DskipTests'
}
}
stage('Tests') {
steps {
echo 'Running tests'
- sh 'mvn verify -Prun-its'
+ sh 'mvn verify -Prun-its ${MVN_LOCAL_REPO_OPT} -Dinvoker.streamLogsOnFailures=true'
Review comment:
Yes, because you don't want them on your PC, probably. You have build.log files there. That's what I find much more comfortable than scrolling through a console or even copy lots from there.
--
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
[GitHub] [shiro] bdemers commented on a change in pull request #297: 1.8 build issues
Posted by GitBox <gi...@apache.org>.
bdemers commented on a change in pull request #297:
URL: https://github.com/apache/shiro/pull/297#discussion_r626676955
##########
File path: Jenkinsfile-jdk11
##########
@@ -72,14 +72,15 @@ pipeline {
stage('Build') {
steps {
echo 'Building'
- sh 'mvn -U -B -e clean install -DskipTests apache-rat:check'
+ sh 'mvn -U -B -e apache-rat:check'
Review comment:
A bit of a cheat, execute "rat" using the global repo (shared with all builds) `~/.m2/repository`
Other maven executions use `.repository` in the clean checkout directory.
Alternatively, I can configure the rat plugin to ignore `.repository` and add it to .gitignore?
--
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
[GitHub] [shiro] bdemers commented on a change in pull request #297: 1.8 build issues
Posted by GitBox <gi...@apache.org>.
bdemers commented on a change in pull request #297:
URL: https://github.com/apache/shiro/pull/297#discussion_r627519117
##########
File path: Jenkinsfile-jdk11
##########
@@ -72,14 +72,15 @@ pipeline {
stage('Build') {
steps {
echo 'Building'
- sh 'mvn -U -B -e clean install -DskipTests apache-rat:check'
+ sh 'mvn -U -B -e apache-rat:check'
Review comment:
oh, this is already resolved on `main` with a matrix build 😄
--
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
[GitHub] [shiro] bdemers commented on a change in pull request #297: 1.8 build issues
Posted by GitBox <gi...@apache.org>.
bdemers commented on a change in pull request #297:
URL: https://github.com/apache/shiro/pull/297#discussion_r627466016
##########
File path: Jenkinsfile-jdk11
##########
@@ -72,14 +72,15 @@ pipeline {
stage('Build') {
steps {
echo 'Building'
- sh 'mvn -U -B -e clean install -DskipTests apache-rat:check'
+ sh 'mvn -U -B -e apache-rat:check'
Review comment:
+1 for this, but I'm going to move it in its own stage for now, and try to remove the duplication between these files on `main`.
Hopefully something like this in each Jenkins file:
```groovy
const jdkVersion = 'jdk_8_latest'
const deploy = false
const rat = false
const supportedBranch = (env.BRANCH_NAME == "1.7.x" || env.BRANCH_NAME == "1.8.x" || env.BRANCH_NAME == "main") // we have a lot of this in the individual files
load('Jenkinsfile-common')
}
```
I'm not sure that will work (given the scope of those globals 🤮), but I'll poke around with it
##########
File path: Jenkinsfile-jdk11
##########
@@ -72,14 +72,15 @@ pipeline {
stage('Build') {
steps {
echo 'Building'
- sh 'mvn -U -B -e clean install -DskipTests apache-rat:check'
+ sh 'mvn -U -B -e apache-rat:check'
Review comment:
+1 for this, but I'm going to move it in its own stage for now, and try to remove the duplication between these files on `main`.
Hopefully something like this in each Jenkins file:
```groovy
const jdkVersion = 'jdk_8_latest'
const deploy = false
const rat = false
const supportedBranch = (env.BRANCH_NAME == "1.7.x" || env.BRANCH_NAME == "1.8.x" || env.BRANCH_NAME == "main") // we have a lot of this in the individual files
load('Jenkinsfile-common')
```
I'm not sure that will work (given the scope of those globals 🤮), but I'll poke around with it
--
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
[GitHub] [shiro] bdemers commented on a change in pull request #297: 1.8 build issues
Posted by GitBox <gi...@apache.org>.
bdemers commented on a change in pull request #297:
URL: https://github.com/apache/shiro/pull/297#discussion_r626674074
##########
File path: crypto/cipher/src/main/java/org/apache/shiro/crypto/AesCipherService.java
##########
@@ -89,12 +89,16 @@
* <b>**</b>Since {@code GCM} is a stream cipher, padding is implemented in the operation mode and an external padding scheme
* cannot be used in conjunction with {@code GCM}. In fact, {@code AES/GCM/PKCS5Padding} is just an alias in most JVM for
* {@code AES/GCM/NoPadding}.
+ * <p/>
+ * <b>NOTE:</b> As of Java 14, setting a streaming padding for the above example will throw a NoSuchAlgorithmException
+ * @see <a href="https://www.oracle.com/java/technologies/javase/14-relnote-issues.html#JDK-8180392">JDK-8180392</a>
Review comment:
I'll merge this line back to master
##########
File path: crypto/cipher/src/main/java/org/apache/shiro/crypto/AesCipherService.java
##########
@@ -89,12 +89,16 @@
* <b>**</b>Since {@code GCM} is a stream cipher, padding is implemented in the operation mode and an external padding scheme
* cannot be used in conjunction with {@code GCM}. In fact, {@code AES/GCM/PKCS5Padding} is just an alias in most JVM for
* {@code AES/GCM/NoPadding}.
+ * <p/>
+ * <b>NOTE:</b> As of Java 14, setting a streaming padding for the above example will throw a NoSuchAlgorithmException
+ * @see <a href="https://www.oracle.com/java/technologies/javase/14-relnote-issues.html#JDK-8180392">JDK-8180392</a>
Review comment:
I'll merge this line back to main
--
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
[GitHub] [shiro] bmarwell commented on a change in pull request #297: 1.8 build issues
Posted by GitBox <gi...@apache.org>.
bmarwell commented on a change in pull request #297:
URL: https://github.com/apache/shiro/pull/297#discussion_r626700793
##########
File path: Jenkinsfile-jdk11
##########
@@ -72,14 +72,15 @@ pipeline {
stage('Build') {
steps {
echo 'Building'
- sh 'mvn -U -B -e clean install -DskipTests apache-rat:check'
+ sh 'mvn -U -B -e apache-rat:check'
Review comment:
Sigh. That's why maven has a per-workspace repo. That shouldn't be needed and became a little better through the matrix build on main.
Or just do the rat check on jdk11 only.
--
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
[GitHub] [shiro] bdemers commented on a change in pull request #297: 1.8 build issues
Posted by GitBox <gi...@apache.org>.
bdemers commented on a change in pull request #297:
URL: https://github.com/apache/shiro/pull/297#discussion_r626674922
##########
File path: web/src/it/projects/MultipleServiceLoader/pom.xml
##########
@@ -38,7 +38,7 @@
<dependency>
<groupId>org.apache.shiro</groupId>
<artifactId>shiro-web</artifactId>
- <version>1.7.1-SNAPSHOT</version>
+ <version>@project.version@</version>
Review comment:
I'll double-check `main` to make sure this is also in place
--
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
[GitHub] [shiro] bdemers commented on a change in pull request #297: 1.8 build issues
Posted by GitBox <gi...@apache.org>.
bdemers commented on a change in pull request #297:
URL: https://github.com/apache/shiro/pull/297#discussion_r626761959
##########
File path: Jenkinsfile-jdk11
##########
@@ -72,14 +72,15 @@ pipeline {
stage('Build') {
steps {
echo 'Building'
- sh 'mvn -U -B -e clean install -DskipTests apache-rat:check'
+ sh 'mvn -U -B -e apache-rat:check'
+ sh 'mvn -U -B -e ${MVN_LOCAL_REPO_OPT} clean install -DskipTests'
Review comment:
just verify would be fine, but I'm not sure why the build and test were broken into different stages originally?
Maybe the stages should be:
* apache-rat:check
* dependency:resolve dependency:plugin-resolve
* verify -Prun-its
--
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
[GitHub] [shiro] bmarwell commented on a change in pull request #297: 1.8 build issues
Posted by GitBox <gi...@apache.org>.
bmarwell commented on a change in pull request #297:
URL: https://github.com/apache/shiro/pull/297#discussion_r626701474
##########
File path: pom.xml
##########
@@ -1533,6 +1533,7 @@
<plugin>
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-invoker-plugin</artifactId>
+ <version>3.2.2</version>
Review comment:
Good option!
##########
File path: Jenkinsfile-jdk11
##########
@@ -72,14 +72,15 @@ pipeline {
stage('Build') {
steps {
echo 'Building'
- sh 'mvn -U -B -e clean install -DskipTests apache-rat:check'
+ sh 'mvn -U -B -e apache-rat:check'
+ sh 'mvn -U -B -e ${MVN_LOCAL_REPO_OPT} clean install -DskipTests'
Review comment:
Is install really needed?
--
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
[GitHub] [shiro] bdemers merged pull request #297: 1.8 build issues
Posted by GitBox <gi...@apache.org>.
bdemers merged pull request #297:
URL: https://github.com/apache/shiro/pull/297
--
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