You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@hc.apache.org by GitBox <gi...@apache.org> on 2021/01/28 17:02:22 UTC
[GitHub] [httpcomponents-core] tony-- opened a new pull request #251: limit scope of org.conscrypt dependency to test
tony-- opened a new pull request #251:
URL: https://github.com/apache/httpcomponents-core/pull/251
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org
[GitHub] [httpcomponents-core] michael-o commented on pull request #251: limit scope of org.conscrypt dependency to test
Posted by GitBox <gi...@apache.org>.
michael-o commented on pull request #251:
URL: https://github.com/apache/httpcomponents-core/pull/251#issuecomment-769286952
Interesting, is it really only used in tests?
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org
[GitHub] [httpcomponents-core] ok2c commented on pull request #251: limit scope of org.conscrypt dependency to test
Posted by GitBox <gi...@apache.org>.
ok2c commented on pull request #251:
URL: https://github.com/apache/httpcomponents-core/pull/251#issuecomment-769626131
@tony-- You are absolutely right. However in the 4.4.x branch the scope of `conscrypt-openjdk-uber` has been defined as `test`
https://github.com/apache/httpcomponents-core/blob/4.4.x/httpcore-nio/pom.xml#L52
My main point still stands. I do not think we should be defining the scope of dependencies in dependency management section at all.
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org
[GitHub] [httpcomponents-core] tony-- commented on pull request #251: limit scope of org.conscrypt dependency to test
Posted by GitBox <gi...@apache.org>.
tony-- commented on pull request #251:
URL: https://github.com/apache/httpcomponents-core/pull/251#issuecomment-769289950
@michael-o it was only added in 4.4.13, as you can see in the diff here:
https://github.com/apache/httpcomponents-core/compare/rel/v4.4.12...rel/v4.4.13
In the component it is actually used in, the scope is [correctly defined](https://github.com/apache/httpcomponents-core/compare/rel/v4.4.12...rel/v4.4.13#diff-552f7b836a536f89239e8a4c8e8b7e562fbfdbe20eafa9fc52936f5000a514c4R52).
I also checked out 4.4.13 to submit my PR and searched all the files. Here are the results:
Searching 680 files for "conscrypt" (regex, case sensitive)
/home/tony/development/httpcomponents-core/httpcore-nio/pom.xml:
50: <groupId>org.conscrypt</groupId>
51: <artifactId>conscrypt-openjdk-uber</artifactId>
/home/tony/development/httpcomponents-core/httpcore-nio/src/test/java/org/apache/http/nio/integration/TestJSSEProviderIntegration.java:
64: import org.conscrypt.Conscrypt;
/home/tony/development/httpcomponents-core/pom.xml:
79: <conscrypt.version>2.2.1</conscrypt.version>
90: <groupId>org.conscrypt</groupId>
91: <artifactId>conscrypt-openjdk-uber</artifactId>
92: <version>${conscrypt.version}</version>
8 matches across 3 files
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org
[GitHub] [httpcomponents-core] tony-- commented on pull request #251: limit scope of org.conscrypt dependency to test
Posted by GitBox <gi...@apache.org>.
tony-- commented on pull request #251:
URL: https://github.com/apache/httpcomponents-core/pull/251#issuecomment-769937813
@ok2c thanks for responding. I don't understand why it has to be in the root pomfile at all, in that case, but I guess it's my fault for not understanding the maven dependency resolution better. I suppose I need to read some docs to learn more.
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org
[GitHub] [httpcomponents-core] ok2c commented on pull request #251: limit scope of org.conscrypt dependency to test
Posted by GitBox <gi...@apache.org>.
ok2c commented on pull request #251:
URL: https://github.com/apache/httpcomponents-core/pull/251#issuecomment-769298853
@tony-- Please note that `conscrypt-openjdk-uber` is being used as compile scope dependency in the `h2` module albeit optional.
I do not think we should be defining the scope of dependencies in dependency management section at all.
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org
[GitHub] [httpcomponents-core] tony-- commented on pull request #251: limit scope of org.conscrypt dependency to test
Posted by GitBox <gi...@apache.org>.
tony-- commented on pull request #251:
URL: https://github.com/apache/httpcomponents-core/pull/251#issuecomment-769959624
Reading [the Maven Dependency Management docs](https://maven.apache.org/guides/introduction/introduction-to-dependency-mechanism.html#Dependency_Management) didn't really help me, but [this SO answer makes it really clear](https://stackoverflow.com/a/37280943/273628). I'll stop with the sharing-learning spam now :)
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org
[GitHub] [httpcomponents-core] ok2c commented on pull request #251: limit scope of org.conscrypt dependency to test
Posted by GitBox <gi...@apache.org>.
ok2c commented on pull request #251:
URL: https://github.com/apache/httpcomponents-core/pull/251#issuecomment-769936368
> he reason I opened the PR is because org.conscrypt is going to be pulled in for anyone consuming httpcomponents-core 4.4.x
@tony-- No, it will not. The scope of `conscrypt-openjdk-uber` is correctly defined in `httpcore-nio` as `test`. There are no other modules dependent on it. The `dependencyManagement` section should have no scope definitions. Junit and Mockito have `test` scope defined but it is a mistake.
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org
[GitHub] [httpcomponents-core] ok2c closed pull request #251: limit scope of org.conscrypt dependency to test
Posted by GitBox <gi...@apache.org>.
ok2c closed pull request #251:
URL: https://github.com/apache/httpcomponents-core/pull/251
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org
[GitHub] [httpcomponents-core] ok2c commented on pull request #251: limit scope of org.conscrypt dependency to test
Posted by GitBox <gi...@apache.org>.
ok2c commented on pull request #251:
URL: https://github.com/apache/httpcomponents-core/pull/251#issuecomment-769940824
@tony-- It is sort of a good practice to define all project dependencies in the project POM dependency management section. The scope of the same dependency however may vary. At some point of this some other module might need the same artifact but with a different scope. This is precisely what has happened with `conscrypt-openjdk-uber` in the 5.x branch. The 5.x `httpcore-h2` now depends on it with `compile` scope, thought the dependency is optional at runtime.
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org
[GitHub] [httpcomponents-core] tony-- commented on pull request #251: limit scope of org.conscrypt dependency to test
Posted by GitBox <gi...@apache.org>.
tony-- commented on pull request #251:
URL: https://github.com/apache/httpcomponents-core/pull/251#issuecomment-769953228
Thanks for your patience. I read some of the maven docs pages and then checked the dependency tree for httpcore:
```
tony@tony-tower-bionic:~/development/httpcomponents-core$ mvn dependency:tree | grep [INFO]
[INFO] Scanning for projects...
[INFO] ------------------------------------------------------------------------
[INFO] Reactor Build Order:
[INFO]
[INFO] Apache HttpComponents Core [pom]
[INFO] Apache HttpCore [jar]
[INFO] Apache HttpCore NIO [jar]
[INFO] Apache HttpCore OSGi bundle [bundle]
[INFO] Apache HttpCore Benchmarking Tool [jar]
[INFO]
[INFO] -----------< org.apache.httpcomponents:httpcomponents-core >------------
[INFO] Building Apache HttpComponents Core 4.4.15-SNAPSHOT [1/5]
[INFO] --------------------------------[ pom ]---------------------------------
[INFO]
[INFO] --- maven-dependency-plugin:3.1.1:tree (default-cli) @ httpcomponents-core ---
[INFO] org.apache.httpcomponents:httpcomponents-core:pom:4.4.15-SNAPSHOT
[INFO]
[INFO] -----------------< org.apache.httpcomponents:httpcore >-----------------
[INFO] Building Apache HttpCore 4.4.15-SNAPSHOT [2/5]
[INFO] --------------------------------[ jar ]---------------------------------
[INFO]
[INFO] --- maven-dependency-plugin:3.1.1:tree (default-cli) @ httpcore ---
[INFO] org.apache.httpcomponents:httpcore:jar:4.4.15-SNAPSHOT
[INFO] +- junit:junit:jar:4.12:test
[INFO] | \- org.hamcrest:hamcrest-core:jar:1.3:test
[INFO] +- org.mockito:mockito-core:jar:1.10.19:test
[INFO] | \- org.objenesis:objenesis:jar:2.1:test
[INFO] \- commons-logging:commons-logging:jar:1.2:test
[INFO]
[INFO] ---------------< org.apache.httpcomponents:httpcore-nio >---------------
[INFO] Building Apache HttpCore NIO 4.4.15-SNAPSHOT [3/5]
[INFO] --------------------------------[ jar ]---------------------------------
[INFO]
[INFO] --- maven-dependency-plugin:3.1.1:tree (default-cli) @ httpcore-nio ---
[INFO] org.apache.httpcomponents:httpcore-nio:jar:4.4.15-SNAPSHOT
[INFO] +- org.apache.httpcomponents:httpcore:jar:4.4.15-SNAPSHOT:compile
[INFO] +- org.conscrypt:conscrypt-openjdk-uber:jar:2.2.1:test
[INFO] +- junit:junit:jar:4.12:test
[INFO] | \- org.hamcrest:hamcrest-core:jar:1.3:test
[INFO] +- org.mockito:mockito-core:jar:1.10.19:test
[INFO] | \- org.objenesis:objenesis:jar:2.1:test
[INFO] \- commons-logging:commons-logging:jar:1.2:test
[INFO]
[INFO] --------------< org.apache.httpcomponents:httpcore-osgi >---------------
[INFO] Building Apache HttpCore OSGi bundle 4.4.15-SNAPSHOT [4/5]
[INFO] -------------------------------[ bundle ]-------------------------------
[INFO]
[INFO] --- maven-dependency-plugin:3.1.1:tree (default-cli) @ httpcore-osgi ---
[INFO] org.apache.httpcomponents:httpcore-osgi:bundle:4.4.15-SNAPSHOT
[INFO] +- org.apache.httpcomponents:httpcore:jar:4.4.15-SNAPSHOT:compile
[INFO] \- org.apache.httpcomponents:httpcore-nio:jar:4.4.15-SNAPSHOT:compile
[INFO]
[INFO] ---------------< org.apache.httpcomponents:httpcore-ab >----------------
[INFO] Building Apache HttpCore Benchmarking Tool 4.4.15-SNAPSHOT [5/5]
[INFO] --------------------------------[ jar ]---------------------------------
[INFO]
[INFO] --- maven-dependency-plugin:3.1.1:tree (default-cli) @ httpcore-ab ---
[INFO] org.apache.httpcomponents:httpcore-ab:jar:4.4.15-SNAPSHOT
[INFO] +- org.apache.httpcomponents:httpcore:jar:4.4.15-SNAPSHOT:compile
[INFO] +- commons-cli:commons-cli:jar:1.2:compile
[INFO] \- junit:junit:jar:4.12:test
[INFO] \- org.hamcrest:hamcrest-core:jar:1.3:test
[INFO] ------------------------------------------------------------------------
[INFO] Reactor Summary for Apache HttpComponents Core 4.4.15-SNAPSHOT:
[INFO]
[INFO] Apache HttpComponents Core ......................... SUCCESS [ 0.255 s]
[INFO] Apache HttpCore .................................... SUCCESS [ 0.015 s]
[INFO] Apache HttpCore NIO ................................ SUCCESS [ 0.011 s]
[INFO] Apache HttpCore OSGi bundle ........................ SUCCESS [ 0.132 s]
[INFO] Apache HttpCore Benchmarking Tool .................. SUCCESS [ 0.005 s]
[INFO] ------------------------------------------------------------------------
[INFO] BUILD SUCCESS
[INFO] ------------------------------------------------------------------------
[INFO] Total time: 1.027 s
[INFO] Finished at: 2021-01-29T09:42:55-08:00
[INFO] ------------------------------------------------------------------------
```
So I see now that org.conscrypt will not be pulled in unconditionally, as I was imagining.
I still don't understand how the dependencyManagement section gets resolved here, but my original concern is resolved and the rest is just gaps in my understanding. Again, thank you.
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org
[GitHub] [httpcomponents-core] tony-- commented on pull request #251: limit scope of org.conscrypt dependency to test
Posted by GitBox <gi...@apache.org>.
tony-- commented on pull request #251:
URL: https://github.com/apache/httpcomponents-core/pull/251#issuecomment-769933711
@ok2c I'm sorry, I just don't understand your position here. All of the other dependencies in this file have the scope marked as test, so your argument that we should not define scope in the dependency management section is really confusing for me. The reason I opened the PR is because org.conscrypt is going to be pulled in for anyone consuming httpcomponents-core 4.4.x for no reason that I can see. Would it be better to just remove the dependency from the root pomfile? If you prefer that, I can open a new PR to do that instead...
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org
[GitHub] [httpcomponents-core] tony-- commented on pull request #251: limit scope of org.conscrypt dependency to test
Posted by GitBox <gi...@apache.org>.
tony-- commented on pull request #251:
URL: https://github.com/apache/httpcomponents-core/pull/251#issuecomment-769399635
> Please note that conscrypt-openjdk-uber is being used as compile scope dependency in the h2 module
@ok2c correct me if I'm wrong, but this is not true on the 4.x branch, is it?
My PR targets the 4.x 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
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org