You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@guacamole.apache.org by GitBox <gi...@apache.org> on 2021/04/01 01:45:23 UTC

[GitHub] [guacamole-client] mike-jumper opened a new pull request #600: GUACAMOLE-1298: Correct mismatched dependencies from migration to Jersey 2.x.

mike-jumper opened a new pull request #600:
URL: https://github.com/apache/guacamole-client/pull/600


   Since migrating to Jersey 2.x, a mismatch between Jackson versions has resulted in some Jackson annotations being ignored. This is due to a difference in package name between the Jackson versions. From the perspective of the newer Jackson, the classes with the older annotations are simply unannotated.
   
   This change updates the annotations where needed, and removes use of those annotations in extensions that should not make any assumptions regarding the JSON serialization implementation.
   
   But wait! What about:
   
   ![thats-a-lot-of-changes](https://user-images.githubusercontent.com/4632905/113231809-e4499a80-9250-11eb-9c18-db56a9e42337.png)
   
   With the core issue here being a version mismatch for a common dependency, part of the resolution had to be updating dependencies to use the same common versions. This resulted in a refactor of all `pom.xml` files to leverage a common, shared set of dependencies.
   
   But wait! That still doesn't quite account for:
   
   ![thats-a-lot-of-changes](https://user-images.githubusercontent.com/4632905/113231809-e4499a80-9250-11eb-9c18-db56a9e42337.png)
   
   With a great many dependencies having to be updated, the various referenced licenses needed to be updated, as well. This includes the numerous copies of the "guice-3.0" dependency and license. Rather than manually update each of these, I instead:
   
   1. Created a script and build process for automatically generating `LICENSE` and `NOTICE` based on actual runtime dependencies as reported by Maven.
   2. Removed duplicated references to the Apache License, which we really only need one of.
   3. Added licenses for dependencies that were missing license documentation.
   4. Added/updated `NOTICE` files where information was missing.
   
   From here on, the amount of duplicated `pom.xml` content is reduced, updating license information should be much easier, and the build will outright fail if we are missing license info.
   
   It also should fix the issue reported on the mailing list regarding TOTP no longer presenting a QR code.


-- 
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] [guacamole-client] necouchman commented on a change in pull request #600: GUACAMOLE-1298: Correct mismatched dependencies from migration to Jersey 2.x.

Posted by GitBox <gi...@apache.org>.
necouchman commented on a change in pull request #600:
URL: https://github.com/apache/guacamole-client/pull/600#discussion_r605883738



##########
File path: extensions/guacamole-auth-duo/src/main/java/org/apache/guacamole/auth/duo/form/DuoSignedResponseField.java
##########
@@ -81,8 +80,7 @@ public DuoSignedResponseField(String apiHost, String signedRequest) {
      * @return
      *     The hostname of the DuoWeb API endpoint.
      */
-    @JsonProperty("apiHost")
-    public String getAPIHost() {
+    public String getApiHost() {

Review comment:
       I see this changed, here, to a more strict CamelCase, along with a couple of others like it in other places in the code, but I don't see that any of the calls to this method get changed. Am I missing something?

##########
File path: extensions/.ratignore
##########
@@ -0,0 +1 @@
+guacamole-auth-radius/**/*

Review comment:
       So does the RADIUS extension not get checked at all? It is excluded, here, and the RAT stuff is also removed from the RADIUS extension's pom.xml file. Does the fact that it's got (L)GPL dependencies mean that we don't do any checking on 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] [guacamole-client] necouchman commented on pull request #600: GUACAMOLE-1298: Correct mismatched dependencies from migration to Jersey 2.x.

Posted by GitBox <gi...@apache.org>.
necouchman commented on pull request #600:
URL: https://github.com/apache/guacamole-client/pull/600#issuecomment-812138241


   Ah, okay, that worked. Looks good...merge time...


-- 
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] [guacamole-client] necouchman commented on pull request #600: GUACAMOLE-1298: Correct mismatched dependencies from migration to Jersey 2.x.

Posted by GitBox <gi...@apache.org>.
necouchman commented on pull request #600:
URL: https://github.com/apache/guacamole-client/pull/600#issuecomment-812127945


   Currently seeing this failure during build:
   ```console
   [INFO] --- exec-maven-plugin:3.0.0:exec (generate-license-files) @ guacamole-auth-json ---
   Processing runtime dependencies to produce LICENSE and NOTICE. Output will be within "/home/cotyww.com/nick_couchman/guacamole/guacamole-client/extensions/guacamole-auth-json/target/licenses".
   ERROR: Missing license README in src/licenses/bundled/jaxb-api
   [ERROR] Command execution failed.
   org.apache.commons.exec.ExecuteException: Process exited with an error: 1 (Exit value: 1)
       at org.apache.commons.exec.DefaultExecutor.executeInternal (DefaultExecutor.java:404)
       at org.apache.commons.exec.DefaultExecutor.execute (DefaultExecutor.java:166)
       at org.codehaus.mojo.exec.ExecMojo.executeCommandLine (ExecMojo.java:982)
       at org.codehaus.mojo.exec.ExecMojo.executeCommandLine (ExecMojo.java:929)
       at org.codehaus.mojo.exec.ExecMojo.execute (ExecMojo.java:457)
       at org.apache.maven.plugin.DefaultBuildPluginManager.executeMojo (DefaultBuildPluginManager.java:137)
       at org.apache.maven.lifecycle.internal.MojoExecutor.execute (MojoExecutor.java:210)
       at org.apache.maven.lifecycle.internal.MojoExecutor.execute (MojoExecutor.java:156)
       at org.apache.maven.lifecycle.internal.MojoExecutor.execute (MojoExecutor.java:148)
       at org.apache.maven.lifecycle.internal.LifecycleModuleBuilder.buildProject (LifecycleModuleBuilder.java:117)
       at org.apache.maven.lifecycle.internal.LifecycleModuleBuilder.buildProject (LifecycleModuleBuilder.java:81)
       at org.apache.maven.lifecycle.internal.builder.singlethreaded.SingleThreadedBuilder.build (SingleThreadedBuilder.java:56)
       at org.apache.maven.lifecycle.internal.LifecycleStarter.execute (LifecycleStarter.java:128)
       at org.apache.maven.DefaultMaven.doExecute (DefaultMaven.java:305)
       at org.apache.maven.DefaultMaven.doExecute (DefaultMaven.java:192)
       at org.apache.maven.DefaultMaven.execute (DefaultMaven.java:105)
       at org.apache.maven.cli.MavenCli.execute (MavenCli.java:957)
       at org.apache.maven.cli.MavenCli.doMain (MavenCli.java:289)
       at org.apache.maven.cli.MavenCli.main (MavenCli.java:193)
       at sun.reflect.NativeMethodAccessorImpl.invoke0 (Native Method)
       at sun.reflect.NativeMethodAccessorImpl.invoke (NativeMethodAccessorImpl.java:62)
       at sun.reflect.DelegatingMethodAccessorImpl.invoke (DelegatingMethodAccessorImpl.java:43)
       at java.lang.reflect.Method.invoke (Method.java:498)
       at org.codehaus.plexus.classworlds.launcher.Launcher.launchEnhanced (Launcher.java:282)
       at org.codehaus.plexus.classworlds.launcher.Launcher.launch (Launcher.java:225)
       at org.codehaus.plexus.classworlds.launcher.Launcher.mainWithExitCode (Launcher.java:406)
       at org.codehaus.plexus.classworlds.launcher.Launcher.main (Launcher.java:347)
   ```


-- 
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] [guacamole-client] mike-jumper commented on a change in pull request #600: GUACAMOLE-1298: Correct mismatched dependencies from migration to Jersey 2.x.

Posted by GitBox <gi...@apache.org>.
mike-jumper commented on a change in pull request #600:
URL: https://github.com/apache/guacamole-client/pull/600#discussion_r605886675



##########
File path: extensions/guacamole-auth-duo/src/main/java/org/apache/guacamole/auth/duo/form/DuoSignedResponseField.java
##########
@@ -81,8 +80,7 @@ public DuoSignedResponseField(String apiHost, String signedRequest) {
      * @return
      *     The hostname of the DuoWeb API endpoint.
      */
-    @JsonProperty("apiHost")
-    public String getAPIHost() {
+    public String getApiHost() {

Review comment:
       Nope - there aren't any such calls. This method serves only to expose data for serialization into JSON by Jackson. Renaming the function in this case allows the JSON property to be `apiHost` without requiring a Jackson-specific annotation to force that name.




-- 
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] [guacamole-client] mike-jumper edited a comment on pull request #600: GUACAMOLE-1298: Correct mismatched dependencies from migration to Jersey 2.x.

Posted by GitBox <gi...@apache.org>.
mike-jumper edited a comment on pull request #600:
URL: https://github.com/apache/guacamole-client/pull/600#issuecomment-812136110


   Can you do a `git clean -xfd .` from the main `guacamole-client` directory to make sure there aren't any stale directories hanging around?
   
   There shouldn't be a `src/licenses/bundled/` within `guacamole-auth-json` any longer, and the dependency on `jaxb-api` has been removed.


-- 
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] [guacamole-client] mike-jumper commented on pull request #600: GUACAMOLE-1298: Correct mismatched dependencies from migration to Jersey 2.x.

Posted by GitBox <gi...@apache.org>.
mike-jumper commented on pull request #600:
URL: https://github.com/apache/guacamole-client/pull/600#issuecomment-812136110


   Can you do a `git clean -xfd .` from the main `guacamole-client` directory to make sure there aren't any stale directories handing around?
   
   There shouldn't be a `src/licenses/bundled/` within `guacamole-auth-json` any longer, and the dependency on `jaxb-api` has been removed.


-- 
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] [guacamole-client] mike-jumper commented on a change in pull request #600: GUACAMOLE-1298: Correct mismatched dependencies from migration to Jersey 2.x.

Posted by GitBox <gi...@apache.org>.
mike-jumper commented on a change in pull request #600:
URL: https://github.com/apache/guacamole-client/pull/600#discussion_r605887739



##########
File path: extensions/.ratignore
##########
@@ -0,0 +1 @@
+guacamole-auth-radius/**/*

Review comment:
       It's checked when it's built by the RAT plugin (it will automatically run when the build is triggered for all projects now, including guacamole-auth-radius).
   
   The issue here is that guacamole-auth-radius is not part of the build unless explicitly included with `-Plgpl-extensions`. The RAT plugin will helpfully exclude modules from the check, but that exclusion won't occur for guacamole-auth-radius if it's not part of the current build. This forces that module to be ignored regardless of whether it's part of the 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] [guacamole-client] necouchman commented on pull request #600: GUACAMOLE-1298: Correct mismatched dependencies from migration to Jersey 2.x.

Posted by GitBox <gi...@apache.org>.
necouchman commented on pull request #600:
URL: https://github.com/apache/guacamole-client/pull/600#issuecomment-811889870


   That's a lot of changes.
   
   ;-)


-- 
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] [guacamole-client] necouchman merged pull request #600: GUACAMOLE-1298: Correct mismatched dependencies from migration to Jersey 2.x.

Posted by GitBox <gi...@apache.org>.
necouchman merged pull request #600:
URL: https://github.com/apache/guacamole-client/pull/600


   


-- 
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] [guacamole-client] necouchman edited a comment on pull request #600: GUACAMOLE-1298: Correct mismatched dependencies from migration to Jersey 2.x.

Posted by GitBox <gi...@apache.org>.
necouchman edited a comment on pull request #600:
URL: https://github.com/apache/guacamole-client/pull/600#issuecomment-812127945


   Currently seeing this failure during build:
   ```console
   [INFO] --- exec-maven-plugin:3.0.0:exec (generate-license-files) @ guacamole-auth-json ---
   Processing runtime dependencies to produce LICENSE and NOTICE. Output will be within "/home/nick/guacamole/guacamole-client/extensions/guacamole-auth-json/target/licenses".
   ERROR: Missing license README in src/licenses/bundled/jaxb-api
   [ERROR] Command execution failed.
   org.apache.commons.exec.ExecuteException: Process exited with an error: 1 (Exit value: 1)
       at org.apache.commons.exec.DefaultExecutor.executeInternal (DefaultExecutor.java:404)
       at org.apache.commons.exec.DefaultExecutor.execute (DefaultExecutor.java:166)
       at org.codehaus.mojo.exec.ExecMojo.executeCommandLine (ExecMojo.java:982)
       at org.codehaus.mojo.exec.ExecMojo.executeCommandLine (ExecMojo.java:929)
       at org.codehaus.mojo.exec.ExecMojo.execute (ExecMojo.java:457)
       at org.apache.maven.plugin.DefaultBuildPluginManager.executeMojo (DefaultBuildPluginManager.java:137)
       at org.apache.maven.lifecycle.internal.MojoExecutor.execute (MojoExecutor.java:210)
       at org.apache.maven.lifecycle.internal.MojoExecutor.execute (MojoExecutor.java:156)
       at org.apache.maven.lifecycle.internal.MojoExecutor.execute (MojoExecutor.java:148)
       at org.apache.maven.lifecycle.internal.LifecycleModuleBuilder.buildProject (LifecycleModuleBuilder.java:117)
       at org.apache.maven.lifecycle.internal.LifecycleModuleBuilder.buildProject (LifecycleModuleBuilder.java:81)
       at org.apache.maven.lifecycle.internal.builder.singlethreaded.SingleThreadedBuilder.build (SingleThreadedBuilder.java:56)
       at org.apache.maven.lifecycle.internal.LifecycleStarter.execute (LifecycleStarter.java:128)
       at org.apache.maven.DefaultMaven.doExecute (DefaultMaven.java:305)
       at org.apache.maven.DefaultMaven.doExecute (DefaultMaven.java:192)
       at org.apache.maven.DefaultMaven.execute (DefaultMaven.java:105)
       at org.apache.maven.cli.MavenCli.execute (MavenCli.java:957)
       at org.apache.maven.cli.MavenCli.doMain (MavenCli.java:289)
       at org.apache.maven.cli.MavenCli.main (MavenCli.java:193)
       at sun.reflect.NativeMethodAccessorImpl.invoke0 (Native Method)
       at sun.reflect.NativeMethodAccessorImpl.invoke (NativeMethodAccessorImpl.java:62)
       at sun.reflect.DelegatingMethodAccessorImpl.invoke (DelegatingMethodAccessorImpl.java:43)
       at java.lang.reflect.Method.invoke (Method.java:498)
       at org.codehaus.plexus.classworlds.launcher.Launcher.launchEnhanced (Launcher.java:282)
       at org.codehaus.plexus.classworlds.launcher.Launcher.launch (Launcher.java:225)
       at org.codehaus.plexus.classworlds.launcher.Launcher.mainWithExitCode (Launcher.java:406)
       at org.codehaus.plexus.classworlds.launcher.Launcher.main (Launcher.java:347)
   ```


-- 
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