You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@maven.apache.org by GitBox <gi...@apache.org> on 2021/05/06 12:09:40 UTC
[GitHub] [maven-javadoc-plugin] bmarwell opened a new pull request #77: [MJAVADOC-642] do not try to read from links in offline mode.
bmarwell opened a new pull request #77:
URL: https://github.com/apache/maven-javadoc-plugin/pull/77
- output error stack traces in debug mode.
Pulled out a trimmed-lowercase-link variable for readability and added a null check which oddly occured after creating this PR.
Please note that not even the non-integration-tests can be run behind a proxy.
Following this checklist to help us incorporate your
contribution quickly and easily:
- [X] Make sure there is a [JIRA issue](https://issues.apache.org/jira/browse/MJAVADOC) filed
for the change (usually before you start working on it). Trivial changes like typos do not
require a JIRA issue. Your pull request should address just this issue, without
pulling in other changes.
- [X] Each commit in the pull request should have a meaningful subject line and body.
- [X] Format the pull request title like `[MJAVADOC-XXX] - Fixes bug in ApproximateQuantiles`,
where you replace `MJAVADOC-XXX` with the appropriate JIRA issue. Best practice
is to use the JIRA issue title in the pull request title and in the first line of the
commit message.
- [X] Write a pull request description that is detailed enough to understand what the pull request does, how, and why.
- [ ] Run `mvn clean verify -Prun-its` to make sure basic checks pass. A more thorough check will
be performed on your pull request automatically.
If your pull request is about ~20 lines of code you don't need to sign an
[Individual Contributor License Agreement](https://www.apache.org/licenses/icla.pdf) if you are unsure
please ask on the developers list.
To make clear that you license your contribution under
the [Apache License Version 2.0, January 2004](http://www.apache.org/licenses/LICENSE-2.0)
you have to acknowledge this by using the following check-box.
- [X] I hereby declare this contribution to be licenced under the [Apache License Version 2.0, January 2004](http://www.apache.org/licenses/LICENSE-2.0)
- [X] In any other case, please file an [Apache Individual Contributor License Agreement](https://www.apache.org/licenses/icla.pdf).
--
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] [maven-javadoc-plugin] bmarwell commented on a change in pull request #77: [MJAVADOC-642] do not try to read from links in offline mode.
Posted by GitBox <gi...@apache.org>.
bmarwell commented on a change in pull request #77:
URL: https://github.com/apache/maven-javadoc-plugin/pull/77#discussion_r635532238
##########
File path: src/main/java/org/apache/maven/plugins/javadoc/AbstractJavadocMojo.java
##########
@@ -6677,6 +6682,11 @@ protected boolean isValidJavadocLink( String link, boolean detecting )
elementListUri = new File( dir, ELEMENT_LIST ).toURI();
}
+ if ( isOffline && !trimmedLowercaseLink.startsWith( "file:" ) )
+ {
+ // checking of both elementlists and package lists skipped.
+ return false;
Review comment:
Well, a quick interim solution could be sth like this:
```diff
Index: src/main/java/org/apache/maven/plugins/javadoc/AbstractJavadocMojo.java
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/src/main/java/org/apache/maven/plugins/javadoc/AbstractJavadocMojo.java b/src/main/java/org/apache/maven/plugins/javadoc/AbstractJavadocMojo.java
--- a/src/main/java/org/apache/maven/plugins/javadoc/AbstractJavadocMojo.java (revision 92957646c0be81474fcbf1e84032880f2e2b350e)
+++ b/src/main/java/org/apache/maven/plugins/javadoc/AbstractJavadocMojo.java (date 1621452882333)
@@ -4368,7 +4368,7 @@
continue;
}
- if ( isOffline && !link.startsWith( "file:" ) )
+ if ( isOffline && isOnlineLink( link ))
{
continue;
}
@@ -4382,6 +4382,19 @@
}
}
+ private boolean isOnlineLink( String link ) {
+ if ( link.startsWith( "file:" ) ) {
+ return true;
+ }
+
+ if ( link.startsWith( "http" ) &&
+ (link.contains( "//localhost" ) || link.contains( "//127.0.0." ) ) ) {
+ return false;
+ }
+
+ return true;
+ }
+
/**
* Coppy all resources to the output directory
*
@@ -6726,7 +6739,7 @@
elementListUri = new File( dir, ELEMENT_LIST ).toURI();
}
- if ( isOffline && !trimmedLowercaseLink.startsWith( "file:" ) )
+ if ( isOffline && isOnlineLink( trimmedLowercaseLink ))
{
// checking of both elementlists and package lists skipped.
return false;
```
But I fear such provisional changes, as nothings holds as forever as provisionally solutions.
--
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] [maven-javadoc-plugin] rfscholte commented on pull request #77: [MJAVADOC-642] do not try to read from links in offline mode.
Posted by GitBox <gi...@apache.org>.
rfscholte commented on pull request #77:
URL: https://github.com/apache/maven-javadoc-plugin/pull/77#issuecomment-898399440
The transport layer is the central location to understand offline.
--
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.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-javadoc-plugin] rfscholte commented on pull request #77: [MJAVADOC-642] do not try to read from links in offline mode.
Posted by GitBox <gi...@apache.org>.
rfscholte commented on pull request #77:
URL: https://github.com/apache/maven-javadoc-plugin/pull/77#issuecomment-898396631
I should have said: the same implementation handling URL requests. Based of https://maven.apache.org/resolver/index.html that could still be Wagon. Based on the image we should be using the same "transport", right?
--
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.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-javadoc-plugin] rfscholte commented on pull request #77: [MJAVADOC-642] do not try to read from links in offline mode.
Posted by GitBox <gi...@apache.org>.
rfscholte commented on pull request #77:
URL: https://github.com/apache/maven-javadoc-plugin/pull/77#issuecomment-833562277
Please don't split the check for package-list and element-list. If there's no element-list you shouldn't log, as the package-list could exist. Only if oth are missing, there's a reason to log.
--
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] [maven-javadoc-plugin] bmarwell commented on a change in pull request #77: [MJAVADOC-642] do not try to read from links in offline mode.
Posted by GitBox <gi...@apache.org>.
bmarwell commented on a change in pull request #77:
URL: https://github.com/apache/maven-javadoc-plugin/pull/77#discussion_r634706343
##########
File path: src/main/java/org/apache/maven/plugins/javadoc/AbstractJavadocMojo.java
##########
@@ -1457,21 +1457,21 @@
* stylesheetfile</a>.
*/
@Parameter( property = "stylesheetfile" )
- private String stylesheetfile;
-
- /**
- * Specifies the path of an additional HTML stylesheet file relative to the {@link #javadocDirectory}
- * Example:
- * <pre>
- * <addStylesheets>
- * <resources/addstylesheet.css</addStylesheet>
- * </addStylesheets>
- * </pre>
+ private String stylesheetfile;
+
+ /**
+ * Specifies the path of an additional HTML stylesheet file relative to the {@link #javadocDirectory}
+ * Example:
+ * <pre>
+ * <addStylesheets>
+ * <resources/addstylesheet.css</addStylesheet>
+ * </addStylesheets>
+ * </pre>
* @since 3.3.0
- */
- @Parameter
- private String[] addStylesheets;
-
+ */
+ @Parameter
+ private String[] addStylesheets;
+
Review comment:
Mixed windows linebreaks converted to unix by IDE.
##########
File path: src/main/java/org/apache/maven/plugins/javadoc/AbstractJavadocMojo.java
##########
@@ -3119,34 +3119,34 @@ private void addAddStyleSheets( List<String> arguments ) throws MavenReportExcep
}
}
-
- private Optional<File> getAddStylesheet( final File javadocOutputDirectory, final String stylesheet )
- throws MavenReportException
- {
- if ( StringUtils.isEmpty( stylesheet ) )
- {
- return Optional.empty();
- }
-
- File addstylesheetfile = new File( getJavadocDirectory(), stylesheet );
- if ( addstylesheetfile.exists() )
- {
- Optional<File> stylesheetfile = getStylesheetFile( javadocOutputDirectory );
- if ( stylesheetfile.isPresent() )
- {
- if ( stylesheetfile.get().getName().equals( addstylesheetfile.getName() ) )
- {
- throw new MavenReportException( "additional stylesheet must have a different name "
- + "than stylesheetfile: " + stylesheetfile.get().getName() );
- }
- }
-
- return Optional.of( addstylesheetfile );
- }
-
- throw new MavenReportException( "additional stylesheet file does not exist: "
- + addstylesheetfile.getAbsolutePath() );
- }
+
+ private Optional<File> getAddStylesheet( final File javadocOutputDirectory, final String stylesheet )
+ throws MavenReportException
+ {
+ if ( StringUtils.isEmpty( stylesheet ) )
+ {
+ return Optional.empty();
+ }
+
+ File addstylesheetfile = new File( getJavadocDirectory(), stylesheet );
+ if ( addstylesheetfile.exists() )
+ {
+ Optional<File> stylesheetfile = getStylesheetFile( javadocOutputDirectory );
+ if ( stylesheetfile.isPresent() )
+ {
+ if ( stylesheetfile.get().getName().equals( addstylesheetfile.getName() ) )
+ {
+ throw new MavenReportException( "additional stylesheet must have a different name "
+ + "than stylesheetfile: " + stylesheetfile.get().getName() );
+ }
+ }
+
+ return Optional.of( addstylesheetfile );
+ }
+
+ throw new MavenReportException( "additional stylesheet file does not exist: "
+ + addstylesheetfile.getAbsolutePath() );
+ }
Review comment:
Mixed windows linebreaks converted to unix by IDE.
##########
File path: src/main/java/org/apache/maven/plugins/javadoc/AbstractJavadocMojo.java
##########
@@ -5643,7 +5643,7 @@ private void addStandardDocletOptions( File javadocOutputDirectory,
JavadocUtil.quotedPathArgument( stylesheetfile.get().getAbsolutePath() ) );
}
- addAddStyleSheets( arguments );
+ addAddStyleSheets( arguments );
Review comment:
Mixed windows linebreaks converted to unix by IDE.
--
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] [maven-javadoc-plugin] bmarwell commented on pull request #77: [MJAVADOC-642] do not try to read from links in offline mode.
Posted by GitBox <gi...@apache.org>.
bmarwell commented on pull request #77:
URL: https://github.com/apache/maven-javadoc-plugin/pull/77#issuecomment-846631084
Sure, go ahead. There's more to change my introducing wagon which would be out of scope in this issue.
--
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] [maven-javadoc-plugin] rfscholte commented on a change in pull request #77: [MJAVADOC-642] do not try to read from links in offline mode.
Posted by GitBox <gi...@apache.org>.
rfscholte commented on a change in pull request #77:
URL: https://github.com/apache/maven-javadoc-plugin/pull/77#discussion_r628098913
##########
File path: src/main/java/org/apache/maven/plugins/javadoc/AbstractJavadocMojo.java
##########
@@ -6677,6 +6682,11 @@ protected boolean isValidJavadocLink( String link, boolean detecting )
elementListUri = new File( dir, ELEMENT_LIST ).toURI();
}
+ if ( isOffline && !trimmedLowercaseLink.startsWith( "file:" ) )
+ {
+ // checking of both elementlists and package lists skipped.
+ return false;
Review comment:
`localhost` and `127.0.0.1` are considered safe too. Makes we wonder if there's a way to use Mavens own Wagon instance that already respects the configuration of settings.xml
--
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] [maven-javadoc-plugin] bmarwell commented on pull request #77: [MJAVADOC-642] do not try to read from links in offline mode.
Posted by GitBox <gi...@apache.org>.
bmarwell commented on pull request #77:
URL: https://github.com/apache/maven-javadoc-plugin/pull/77#issuecomment-834228721
> combine package-list/element-list message to prevent unnecesary noise in logging
Will do (revert). I thought it might have been useful for debugging (-X), but this is probably out of scope for this PR.
--
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] [maven-javadoc-plugin] rfscholte commented on pull request #77: [MJAVADOC-642] do not try to read from links in offline mode.
Posted by GitBox <gi...@apache.org>.
rfscholte commented on pull request #77:
URL: https://github.com/apache/maven-javadoc-plugin/pull/77#issuecomment-844428135
When this plugin wants to use the settings' `offline`, it should behave exactly the same as Maven uses this property, which means Wagon should be used. This will completely replace all httpclient code. I can make a new ticket which will superseed this one if you want.
--
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] [maven-javadoc-plugin] bmarwell commented on pull request #77: [MJAVADOC-642] do not try to read from links in offline mode.
Posted by GitBox <gi...@apache.org>.
bmarwell commented on pull request #77:
URL: https://github.com/apache/maven-javadoc-plugin/pull/77#issuecomment-898397946
> I should have said: the same implementation handling URL requests. Based of https://maven.apache.org/resolver/index.html that could still be Wagon. Based on the image we should be using the same "transport", right?
This PR is about LESS artefacts to transport. Why change the transport in such a PR?
--
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.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-javadoc-plugin] bmarwell commented on a change in pull request #77: [MJAVADOC-642] do not try to read from links in offline mode.
Posted by GitBox <gi...@apache.org>.
bmarwell commented on a change in pull request #77:
URL: https://github.com/apache/maven-javadoc-plugin/pull/77#discussion_r634706082
##########
File path: src/main/java/org/apache/maven/plugins/javadoc/AbstractJavadocMojo.java
##########
@@ -6677,6 +6682,11 @@ protected boolean isValidJavadocLink( String link, boolean detecting )
elementListUri = new File( dir, ELEMENT_LIST ).toURI();
}
+ if ( isOffline && !trimmedLowercaseLink.startsWith( "file:" ) )
+ {
+ // checking of both elementlists and package lists skipped.
+ return false;
Review comment:
@rfscholte do you have any explicit suggestion how to resolve this? Technically "offline" could also mean "no network", where 127.0.0.1 is a local network interface, too. But if there is a better solution, I would be willing to implement 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] [maven-javadoc-plugin] cstamas commented on pull request #77: [MJAVADOC-642] do not try to read from links in offline mode.
Posted by GitBox <gi...@apache.org>.
cstamas commented on pull request #77:
URL: https://github.com/apache/maven-javadoc-plugin/pull/77#issuecomment-898385377
my 5 cents: a big NO to use Wagon (maven2) stuff in any maven plugin, not is just an overkill, but wrong IMO as well...
Btw "it should behave exactly the same as Maven uses this property, which means Wagon should be used", does it means that the logic defining Maven behavior re offline is not in Maven but in Wagon?
--
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.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org