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 2022/02/22 08:22:09 UTC
[GitHub] [maven-resolver] cstamas opened a new pull request #154: [MRESOLVER-241] Resolver checksum calculation should be driven by layout
cstamas opened a new pull request #154:
URL: https://github.com/apache/maven-resolver/pull/154
Refactor checksum calculator and validator to rely on layout
provided checksums (layout always had these), and not by the presence
(or absence) or checksum locations.
Checksum locations should drive only the checksum provision from
remote, and nothing else.
--
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-resolver] jvanzyl commented on pull request #154: [MRESOLVER-241] Resolver checksum calculation should be driven by layout
Posted by GitBox <gi...@apache.org>.
jvanzyl commented on pull request #154:
URL: https://github.com/apache/maven-resolver/pull/154#issuecomment-1065978232
If you're referring to Sigstore integration, it's up for discussion what the best naming conventions will be. I have x509, SSH, and PGP signing working. So probably want to keep `.asc` as not to interfere with existing PGP sigs, but for x509 maybe `.sig` and for SSH `.sshsig`. Not sure yet really.
--
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-resolver] michael-o commented on a change in pull request #154: [MRESOLVER-241] Resolver checksum calculation should be driven by layout
Posted by GitBox <gi...@apache.org>.
michael-o commented on a change in pull request #154:
URL: https://github.com/apache/maven-resolver/pull/154#discussion_r825358352
##########
File path: maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/checksum/DefaultArtifactChecksumFilterFactory.java
##########
@@ -0,0 +1,90 @@
+package org.eclipse.aether.internal.impl.checksum;
+
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import javax.inject.Named;
+import javax.inject.Singleton;
+
+import java.util.Arrays;
+import java.util.Set;
+import java.util.stream.Collectors;
+
+import org.eclipse.aether.RepositorySystemSession;
+import org.eclipse.aether.artifact.Artifact;
+import org.eclipse.aether.repository.RemoteRepository;
+import org.eclipse.aether.spi.connector.checksum.ArtifactChecksumFilter;
+import org.eclipse.aether.spi.connector.checksum.ArtifactChecksumFilterFactory;
+import org.eclipse.aether.util.ConfigUtils;
+
+import static java.util.Objects.requireNonNull;
+
+/**
+ * Default implementation that implements default resolver strategy: filters by known (user configured) extensions,
+ * or by default only for GPG signature.
+ *
+ * @since 1.8.0
+ */
+@Singleton
+@Named
+public class DefaultArtifactChecksumFilterFactory
+ implements ArtifactChecksumFilterFactory
+{
+ public static final String CONFIG_PROP_OMIT_CHECKSUMS_FOR_EXTENSIONS =
+ "aether.checksums.omitChecksumsForExtensions";
+
+ private static final String DEFAULT_OMIT_CHECKSUMS_FOR_EXTENSIONS = ".asc";
+
+ @Override
+ public ArtifactChecksumFilter newInstance( RepositorySystemSession session, RemoteRepository repository )
+ {
+ // ensure uniqueness of (potentially user set) extension list
+ Set<String> omitChecksumsForExtensions = Arrays.stream( ConfigUtils.getString(
+ session, DEFAULT_OMIT_CHECKSUMS_FOR_EXTENSIONS, CONFIG_PROP_OMIT_CHECKSUMS_FOR_EXTENSIONS )
+ .split( "," )
+ ).filter( s -> s != null && !s.trim().isEmpty() ).collect( Collectors.toSet() );
+
+ return new ExtensionArtifactChecksumFilter( omitChecksumsForExtensions );
+ }
+
+ private static class ExtensionArtifactChecksumFilter
+ implements ArtifactChecksumFilter
+ {
+ private final Set<String> extensionsWithoutChecksums;
+
+ private ExtensionArtifactChecksumFilter( Set<String> extensionsWithoutChecksums )
+ {
+ this.extensionsWithoutChecksums = requireNonNull( extensionsWithoutChecksums );
+ }
+
+ @Override
+ public boolean omitChecksumsFor( Artifact artifact )
+ {
+ String artifactExtension = artifact.getExtension(); // ie. pom.asc
+ for ( String extensionWithoutChecksums : extensionsWithoutChecksums )
+ {
+ if ( artifactExtension.endsWith( extensionWithoutChecksums ) )
Review comment:
I don't fully understand your second comment. Does this mean that:
* Artifact Ext `X` should not be subject for checksum (without dot)
* Subartifact Ext `X.asc` needs the dot be differentiate between artifact and subartifact?
--
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-resolver] cstamas commented on pull request #154: [MRESOLVER-241] Resolver checksum calculation should be driven by layout
Posted by GitBox <gi...@apache.org>.
cstamas commented on pull request #154:
URL: https://github.com/apache/maven-resolver/pull/154#issuecomment-1066616895
IMO, as we still test with `endsWith`, dotless and dotted is important, but for simplicity sake, we'd maybe enforce configuration to use only dotted (dot prefixed) input. OTOH, I'd not apply anything like auto-adding dots, as that may hinder our later efforts to do something else (as we see that having or not having dot as prefix does matters, but we still don't know exactly what and how to use that, so better not allow users to be lazy that may hit us back later).
Simply put: change factory to enforce leading dots, and update doco that list must have extensions with leading dots.
Rationale:
* artifact.getExtension returns dotless extension (so not starting with dot), but sub-artifacts may have dots within (jar.asc)
* enforcing that users use dot-prefixed strings means we are applying this ONLY to sub-artifacts, that again, makes sense. I don't think we want to have top level artifacts that does NOT have checksums.
--
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-resolver] cstamas commented on a change in pull request #154: [MRESOLVER-241] Resolver checksum calculation should be driven by layout
Posted by GitBox <gi...@apache.org>.
cstamas commented on a change in pull request #154:
URL: https://github.com/apache/maven-resolver/pull/154#discussion_r825357737
##########
File path: maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/checksum/DefaultArtifactChecksumFilterFactory.java
##########
@@ -0,0 +1,90 @@
+package org.eclipse.aether.internal.impl.checksum;
+
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import javax.inject.Named;
+import javax.inject.Singleton;
+
+import java.util.Arrays;
+import java.util.Set;
+import java.util.stream.Collectors;
+
+import org.eclipse.aether.RepositorySystemSession;
+import org.eclipse.aether.artifact.Artifact;
+import org.eclipse.aether.repository.RemoteRepository;
+import org.eclipse.aether.spi.connector.checksum.ArtifactChecksumFilter;
+import org.eclipse.aether.spi.connector.checksum.ArtifactChecksumFilterFactory;
+import org.eclipse.aether.util.ConfigUtils;
+
+import static java.util.Objects.requireNonNull;
+
+/**
+ * Default implementation that implements default resolver strategy: filters by known (user configured) extensions,
+ * or by default only for GPG signature.
+ *
+ * @since 1.8.0
+ */
+@Singleton
+@Named
+public class DefaultArtifactChecksumFilterFactory
+ implements ArtifactChecksumFilterFactory
+{
+ public static final String CONFIG_PROP_OMIT_CHECKSUMS_FOR_EXTENSIONS =
+ "aether.checksums.omitChecksumsForExtensions";
+
+ private static final String DEFAULT_OMIT_CHECKSUMS_FOR_EXTENSIONS = ".asc";
+
+ @Override
+ public ArtifactChecksumFilter newInstance( RepositorySystemSession session, RemoteRepository repository )
+ {
+ // ensure uniqueness of (potentially user set) extension list
+ Set<String> omitChecksumsForExtensions = Arrays.stream( ConfigUtils.getString(
+ session, DEFAULT_OMIT_CHECKSUMS_FOR_EXTENSIONS, CONFIG_PROP_OMIT_CHECKSUMS_FOR_EXTENSIONS )
+ .split( "," )
+ ).filter( s -> s != null && !s.trim().isEmpty() ).collect( Collectors.toSet() );
+
+ return new ExtensionArtifactChecksumFilter( omitChecksumsForExtensions );
+ }
+
+ private static class ExtensionArtifactChecksumFilter
+ implements ArtifactChecksumFilter
+ {
+ private final Set<String> extensionsWithoutChecksums;
+
+ private ExtensionArtifactChecksumFilter( Set<String> extensionsWithoutChecksums )
+ {
+ this.extensionsWithoutChecksums = requireNonNull( extensionsWithoutChecksums );
+ }
+
+ @Override
+ public boolean omitChecksumsFor( Artifact artifact )
+ {
+ String artifactExtension = artifact.getExtension(); // ie. pom.asc
+ for ( String extensionWithoutChecksums : extensionsWithoutChecksums )
+ {
+ if ( artifactExtension.endsWith( extensionWithoutChecksums ) )
Review comment:
See doco and default value: `.asc`, period is there. But right, if I introduce new packaging type "foo" (so extension is ".foo"), I'd be unable to make it filtered out...
--
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-resolver] michael-o commented on a change in pull request #154: [MRESOLVER-241] Resolver checksum calculation should be driven by layout
Posted by GitBox <gi...@apache.org>.
michael-o commented on a change in pull request #154:
URL: https://github.com/apache/maven-resolver/pull/154#discussion_r825355964
##########
File path: maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/Maven2RepositoryLayoutFactory.java
##########
@@ -225,32 +239,9 @@ public URI getLocation( Metadata metadata, boolean upload )
return checksumLocations;
}
- }
-
- private static class Maven2RepositoryLayoutEx
- extends Maven2RepositoryLayout
- {
-
- protected Maven2RepositoryLayoutEx( List<ChecksumAlgorithmFactory> checksumAlgorithms )
+ private boolean isChecksum( String extension )
{
- super( checksumAlgorithms );
+ return checksumAlgorithms.stream().anyMatch( a -> extension.endsWith( "." + a.getFileExtension() ) );
Review comment:
Why `endsWith()` and not `equals()`?
--
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-resolver] cstamas commented on a change in pull request #154: [MRESOLVER-241] Resolver checksum calculation should be driven by layout
Posted by GitBox <gi...@apache.org>.
cstamas commented on a change in pull request #154:
URL: https://github.com/apache/maven-resolver/pull/154#discussion_r825359263
##########
File path: maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/checksum/DefaultArtifactChecksumFilterFactory.java
##########
@@ -0,0 +1,90 @@
+package org.eclipse.aether.internal.impl.checksum;
+
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import javax.inject.Named;
+import javax.inject.Singleton;
+
+import java.util.Arrays;
+import java.util.Set;
+import java.util.stream.Collectors;
+
+import org.eclipse.aether.RepositorySystemSession;
+import org.eclipse.aether.artifact.Artifact;
+import org.eclipse.aether.repository.RemoteRepository;
+import org.eclipse.aether.spi.connector.checksum.ArtifactChecksumFilter;
+import org.eclipse.aether.spi.connector.checksum.ArtifactChecksumFilterFactory;
+import org.eclipse.aether.util.ConfigUtils;
+
+import static java.util.Objects.requireNonNull;
+
+/**
+ * Default implementation that implements default resolver strategy: filters by known (user configured) extensions,
+ * or by default only for GPG signature.
+ *
+ * @since 1.8.0
+ */
+@Singleton
+@Named
+public class DefaultArtifactChecksumFilterFactory
+ implements ArtifactChecksumFilterFactory
+{
+ public static final String CONFIG_PROP_OMIT_CHECKSUMS_FOR_EXTENSIONS =
+ "aether.checksums.omitChecksumsForExtensions";
+
+ private static final String DEFAULT_OMIT_CHECKSUMS_FOR_EXTENSIONS = ".asc";
+
+ @Override
+ public ArtifactChecksumFilter newInstance( RepositorySystemSession session, RemoteRepository repository )
+ {
+ // ensure uniqueness of (potentially user set) extension list
+ Set<String> omitChecksumsForExtensions = Arrays.stream( ConfigUtils.getString(
+ session, DEFAULT_OMIT_CHECKSUMS_FOR_EXTENSIONS, CONFIG_PROP_OMIT_CHECKSUMS_FOR_EXTENSIONS )
+ .split( "," )
+ ).filter( s -> s != null && !s.trim().isEmpty() ).collect( Collectors.toSet() );
+
+ return new ExtensionArtifactChecksumFilter( omitChecksumsForExtensions );
+ }
+
+ private static class ExtensionArtifactChecksumFilter
+ implements ArtifactChecksumFilter
+ {
+ private final Set<String> extensionsWithoutChecksums;
+
+ private ExtensionArtifactChecksumFilter( Set<String> extensionsWithoutChecksums )
+ {
+ this.extensionsWithoutChecksums = requireNonNull( extensionsWithoutChecksums );
+ }
+
+ @Override
+ public boolean omitChecksumsFor( Artifact artifact )
+ {
+ String artifactExtension = artifact.getExtension(); // ie. pom.asc
+ for ( String extensionWithoutChecksums : extensionsWithoutChecksums )
+ {
+ if ( artifactExtension.endsWith( extensionWithoutChecksums ) )
Review comment:
See https://github.com/apache/maven-resolver/pull/154#discussion_r825358528
--
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-resolver] cstamas commented on a change in pull request #154: [MRESOLVER-241] Resolver checksum calculation should be driven by layout
Posted by GitBox <gi...@apache.org>.
cstamas commented on a change in pull request #154:
URL: https://github.com/apache/maven-resolver/pull/154#discussion_r825357135
##########
File path: maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/Maven2RepositoryLayoutFactory.java
##########
@@ -206,6 +216,10 @@ public URI getLocation( Metadata metadata, boolean upload )
@Override
public List<ChecksumLocation> getChecksumLocations( Artifact artifact, boolean upload, URI location )
{
+ if ( artifactChecksumFilter.omitChecksumsFor( artifact ) || isChecksum( artifact.getExtension() ) )
Review comment:
artifact.getExtension returns values like "pom", "pom.sha1", "pom.asc" etc
--
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-resolver] michael-o commented on a change in pull request #154: [MRESOLVER-241] Resolver checksum calculation should be driven by layout
Posted by GitBox <gi...@apache.org>.
michael-o commented on a change in pull request #154:
URL: https://github.com/apache/maven-resolver/pull/154#discussion_r825356991
##########
File path: maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/checksum/DefaultArtifactChecksumFilterFactory.java
##########
@@ -0,0 +1,90 @@
+package org.eclipse.aether.internal.impl.checksum;
+
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import javax.inject.Named;
+import javax.inject.Singleton;
+
+import java.util.Arrays;
+import java.util.Set;
+import java.util.stream.Collectors;
+
+import org.eclipse.aether.RepositorySystemSession;
+import org.eclipse.aether.artifact.Artifact;
+import org.eclipse.aether.repository.RemoteRepository;
+import org.eclipse.aether.spi.connector.checksum.ArtifactChecksumFilter;
+import org.eclipse.aether.spi.connector.checksum.ArtifactChecksumFilterFactory;
+import org.eclipse.aether.util.ConfigUtils;
+
+import static java.util.Objects.requireNonNull;
+
+/**
+ * Default implementation that implements default resolver strategy: filters by known (user configured) extensions,
+ * or by default only for GPG signature.
+ *
+ * @since 1.8.0
+ */
+@Singleton
+@Named
+public class DefaultArtifactChecksumFilterFactory
+ implements ArtifactChecksumFilterFactory
+{
+ public static final String CONFIG_PROP_OMIT_CHECKSUMS_FOR_EXTENSIONS =
+ "aether.checksums.omitChecksumsForExtensions";
+
+ private static final String DEFAULT_OMIT_CHECKSUMS_FOR_EXTENSIONS = ".asc";
+
+ @Override
+ public ArtifactChecksumFilter newInstance( RepositorySystemSession session, RemoteRepository repository )
+ {
+ // ensure uniqueness of (potentially user set) extension list
+ Set<String> omitChecksumsForExtensions = Arrays.stream( ConfigUtils.getString(
+ session, DEFAULT_OMIT_CHECKSUMS_FOR_EXTENSIONS, CONFIG_PROP_OMIT_CHECKSUMS_FOR_EXTENSIONS )
+ .split( "," )
+ ).filter( s -> s != null && !s.trim().isEmpty() ).collect( Collectors.toSet() );
+
+ return new ExtensionArtifactChecksumFilter( omitChecksumsForExtensions );
+ }
+
+ private static class ExtensionArtifactChecksumFilter
+ implements ArtifactChecksumFilter
+ {
+ private final Set<String> extensionsWithoutChecksums;
+
+ private ExtensionArtifactChecksumFilter( Set<String> extensionsWithoutChecksums )
+ {
+ this.extensionsWithoutChecksums = requireNonNull( extensionsWithoutChecksums );
+ }
+
+ @Override
+ public boolean omitChecksumsFor( Artifact artifact )
+ {
+ String artifactExtension = artifact.getExtension(); // ie. pom.asc
+ for ( String extensionWithoutChecksums : extensionsWithoutChecksums )
+ {
+ if ( artifactExtension.endsWith( extensionWithoutChecksums ) )
Review comment:
The period should be added for testing here
##########
File path: maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/Maven2RepositoryLayoutFactory.java
##########
@@ -206,6 +216,10 @@ public URI getLocation( Metadata metadata, boolean upload )
@Override
public List<ChecksumLocation> getChecksumLocations( Artifact artifact, boolean upload, URI location )
{
+ if ( artifactChecksumFilter.omitChecksumsFor( artifact ) || isChecksum( artifact.getExtension() ) )
Review comment:
Does this `getExtension()` really contain a dot?
##########
File path: maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/checksum/DefaultArtifactChecksumFilterFactory.java
##########
@@ -0,0 +1,90 @@
+package org.eclipse.aether.internal.impl.checksum;
+
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import javax.inject.Named;
+import javax.inject.Singleton;
+
+import java.util.Arrays;
+import java.util.Set;
+import java.util.stream.Collectors;
+
+import org.eclipse.aether.RepositorySystemSession;
+import org.eclipse.aether.artifact.Artifact;
+import org.eclipse.aether.repository.RemoteRepository;
+import org.eclipse.aether.spi.connector.checksum.ArtifactChecksumFilter;
+import org.eclipse.aether.spi.connector.checksum.ArtifactChecksumFilterFactory;
+import org.eclipse.aether.util.ConfigUtils;
+
+import static java.util.Objects.requireNonNull;
+
+/**
+ * Default implementation that implements default resolver strategy: filters by known (user configured) extensions,
+ * or by default only for GPG signature.
+ *
+ * @since 1.8.0
+ */
+@Singleton
+@Named
+public class DefaultArtifactChecksumFilterFactory
+ implements ArtifactChecksumFilterFactory
+{
+ public static final String CONFIG_PROP_OMIT_CHECKSUMS_FOR_EXTENSIONS =
+ "aether.checksums.omitChecksumsForExtensions";
+
+ private static final String DEFAULT_OMIT_CHECKSUMS_FOR_EXTENSIONS = ".asc";
Review comment:
Since `org.eclipse.aether.artifact.Artifact.getExtension()` works without leading period, I would expect the same here.
--
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-resolver] cstamas commented on a change in pull request #154: [MRESOLVER-241] Resolver checksum calculation should be driven by layout
Posted by GitBox <gi...@apache.org>.
cstamas commented on a change in pull request #154:
URL: https://github.com/apache/maven-resolver/pull/154#discussion_r825356843
##########
File path: maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/Maven2RepositoryLayoutFactory.java
##########
@@ -225,32 +239,9 @@ public URI getLocation( Metadata metadata, boolean upload )
return checksumLocations;
}
- }
-
- private static class Maven2RepositoryLayoutEx
- extends Maven2RepositoryLayout
- {
-
- protected Maven2RepositoryLayoutEx( List<ChecksumAlgorithmFactory> checksumAlgorithms )
+ private boolean isChecksum( String extension )
{
- super( checksumAlgorithms );
+ return checksumAlgorithms.stream().anyMatch( a -> extension.endsWith( "." + a.getFileExtension() ) );
Review comment:
as extension of POM signature is `pom.asc`, also if you look at old code, it was doing same thing for same reason.
--
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-resolver] cstamas commented on a change in pull request #154: [MRESOLVER-241] Resolver checksum calculation should be driven by layout
Posted by GitBox <gi...@apache.org>.
cstamas commented on a change in pull request #154:
URL: https://github.com/apache/maven-resolver/pull/154#discussion_r825360764
##########
File path: maven-resolver-spi/src/main/java/org/eclipse/aether/spi/connector/checksum/ArtifactChecksumFilter.java
##########
@@ -0,0 +1,44 @@
+package org.eclipse.aether.spi.connector.checksum;
+
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import org.eclipse.aether.artifact.Artifact;
+
+/**
+ * Filter that is able to tell does artifact have expected checksums or not. Most notably, artifacts like
+ * different kind of signatures will not have checksums.
+ *
+ * @since 1.8.0
+ */
+public interface ArtifactChecksumFilter
Review comment:
fun fact: Central for example (but same stands for Nx2 or Artifactory) does sent checksum header for ANY payload, hence, even checksum for checksum, and resolver in this PR DOES USE IT to validate payload (these are all REMOTE_INCLUDED ones).
--
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-resolver] michael-o commented on pull request #154: [MRESOLVER-241] Resolver checksum calculation should be driven by layout
Posted by GitBox <gi...@apache.org>.
michael-o commented on pull request #154:
URL: https://github.com/apache/maven-resolver/pull/154#issuecomment-1066921491
> IMO, as we still test with `endsWith`, dotless and dotted is important, but for simplicity sake, we'd maybe enforce configuration to use only dotted (dot prefixed) input. OTOH, I'd not apply anything like auto-adding dots, as that may hinder our later efforts to do something else (as we see that having or not having dot as prefix does matters, but we still don't know exactly what and how to use that, so better not allow users to be lazy that may hit us back later).
>
> Simply put: change factory to enforce leading dots, and update doco that list must have extensions with leading dots.
>
> Rationale:
>
> * artifact.getExtension returns dotless extension (so not starting with dot), but sub-artifacts may have dots within extension (jar.asc)
>
> * enforcing that users use dot-prefixed strings means we are applying this ONLY to sub-artifacts, that again, makes sense. I don't think we want to have top level artifacts that does NOT have checksums.
>
> * still, having users adding dots, instead code auto-adding it leaves open a possibility for us, to change this later (and maybe use a list of some ant patterns or whatnot, but currently I'd see this as overkill)
I follow your reasoning...
--
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-resolver] michael-o commented on a change in pull request #154: [MRESOLVER-241] Resolver checksum calculation should be driven by layout
Posted by GitBox <gi...@apache.org>.
michael-o commented on a change in pull request #154:
URL: https://github.com/apache/maven-resolver/pull/154#discussion_r826056881
##########
File path: maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/Maven2RepositoryLayoutFactory.java
##########
@@ -128,6 +129,18 @@ public RepositoryLayout newInstance( RepositorySystemSession session, RemoteRepo
.split( "," )
).filter( s -> s != null && !s.trim().isEmpty() ).collect( Collectors.toSet() );
+ // validation: enforce that all strings in this set are having leading dot
+ if ( omitChecksumsForExtensions.stream().anyMatch( s -> !s.startsWith( "." ) ) )
+ {
+ throw new IllegalArgumentException(
+ String.format(
+ "The configuration %s contains illegal values: %s (all entries must start with '.' (dot)",
Review comment:
Unbalanced parens at end: `)`
--
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-resolver] cstamas commented on pull request #154: [MRESOLVER-241] Resolver checksum calculation should be driven by layout
Posted by GitBox <gi...@apache.org>.
cstamas commented on pull request #154:
URL: https://github.com/apache/maven-resolver/pull/154#issuecomment-1065985638
@michael-o review last commits pls, much simpler
--
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-resolver] cstamas commented on a change in pull request #154: [MRESOLVER-241] Resolver checksum calculation should be driven by layout
Posted by GitBox <gi...@apache.org>.
cstamas commented on a change in pull request #154:
URL: https://github.com/apache/maven-resolver/pull/154#discussion_r825358528
##########
File path: maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/checksum/DefaultArtifactChecksumFilterFactory.java
##########
@@ -0,0 +1,90 @@
+package org.eclipse.aether.internal.impl.checksum;
+
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import javax.inject.Named;
+import javax.inject.Singleton;
+
+import java.util.Arrays;
+import java.util.Set;
+import java.util.stream.Collectors;
+
+import org.eclipse.aether.RepositorySystemSession;
+import org.eclipse.aether.artifact.Artifact;
+import org.eclipse.aether.repository.RemoteRepository;
+import org.eclipse.aether.spi.connector.checksum.ArtifactChecksumFilter;
+import org.eclipse.aether.spi.connector.checksum.ArtifactChecksumFilterFactory;
+import org.eclipse.aether.util.ConfigUtils;
+
+import static java.util.Objects.requireNonNull;
+
+/**
+ * Default implementation that implements default resolver strategy: filters by known (user configured) extensions,
+ * or by default only for GPG signature.
+ *
+ * @since 1.8.0
+ */
+@Singleton
+@Named
+public class DefaultArtifactChecksumFilterFactory
+ implements ArtifactChecksumFilterFactory
+{
+ public static final String CONFIG_PROP_OMIT_CHECKSUMS_FOR_EXTENSIONS =
+ "aether.checksums.omitChecksumsForExtensions";
+
+ private static final String DEFAULT_OMIT_CHECKSUMS_FOR_EXTENSIONS = ".asc";
Review comment:
Having or not having leading period is important:
| input | `artifact-1.0.foo` | `artifact-1.0.jar.foo` |
|-------|------------------|---------------------|
|`.foo` | not matches | matches |
|`foo`| matches | matches |
Some extensions (like PGP signature `asc`) may be only subartifact, so extension is actually `jar.asc` or `pom.asc`. Still, if we'd want to generalize this, we may do:
* if `extensionWithoutChecksums` starts with period => check with `endsWith`
* if `extensionWithoutChecksums` not starts with period => check with `equals`
WDYT?
--
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-resolver] cstamas edited a comment on pull request #154: [MRESOLVER-241] Resolver checksum calculation should be driven by layout
Posted by GitBox <gi...@apache.org>.
cstamas edited a comment on pull request #154:
URL: https://github.com/apache/maven-resolver/pull/154#issuecomment-1066616895
IMO, as we still test with `endsWith`, dotless and dotted is important, but for simplicity sake, we'd maybe enforce configuration to use only dotted (dot prefixed) input. OTOH, I'd not apply anything like auto-adding dots, as that may hinder our later efforts to do something else (as we see that having or not having dot as prefix does matters, but we still don't know exactly what and how to use that, so better not allow users to be lazy that may hit us back later).
Simply put: change factory to enforce leading dots, and update doco that list must have extensions with leading dots.
Rationale:
* artifact.getExtension returns dotless extension (so not starting with dot), but sub-artifacts may have dots within (jar.asc)
* enforcing that users use dot-prefixed strings means we are applying this ONLY to sub-artifacts, that again, makes sense. I don't think we want to have top level artifacts that does NOT have checksums.
* still, having users adding dots, instead code auto-adding it leaves open a possibility for us, to change this later (and maybe use a list of some ant patterns or whatnot, but currently I'd see this as overkill)
--
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-resolver] michael-o commented on a change in pull request #154: [MRESOLVER-241] Resolver checksum calculation should be driven by layout
Posted by GitBox <gi...@apache.org>.
michael-o commented on a change in pull request #154:
URL: https://github.com/apache/maven-resolver/pull/154#discussion_r825430242
##########
File path: maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/Maven2RepositoryLayoutFactory.java
##########
@@ -118,9 +123,16 @@ public RepositoryLayout newInstance( RepositorySystemSession session, RemoteRepo
checksumsAlgorithms.add( checksumAlgorithmFactorySelector.select( checksumsAlgorithmName ) );
}
- return forSignature
- ? new Maven2RepositoryLayout( checksumsAlgorithms )
- : new Maven2RepositoryLayoutEx( checksumsAlgorithms );
+ // ensure uniqueness of (potentially user set) extension list
+ Set<String> omitChecksumsForExtensions = Arrays.stream( ConfigUtils.getString(
+ session, DEFAULT_OMIT_CHECKSUMS_FOR_EXTENSIONS, CONFIG_PROP_OMIT_CHECKSUMS_FOR_EXTENSIONS )
+ .split( "," )
+ ).filter( s -> s != null && !s.trim().isEmpty() ).collect( Collectors.toSet() );
Review comment:
Note: We don't apply `s != null && !s.trim().isEmpty()` to `CONFIG_PROP_CHECKSUMS_ALGORITHMS`. It should be consistent both or nothing.
--
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-resolver] slawekjaranowski commented on pull request #154: [MRESOLVER-241] Resolver checksum calculation should be driven by layout
Posted by GitBox <gi...@apache.org>.
slawekjaranowski commented on pull request #154:
URL: https://github.com/apache/maven-resolver/pull/154#issuecomment-1047978847
small request - try to align arguments in method calls like in:
https://github.com/apache/maven-site/pull/291
--
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-resolver] cstamas edited a comment on pull request #154: [MRESOLVER-241] Resolver checksum calculation should be driven by layout
Posted by GitBox <gi...@apache.org>.
cstamas edited a comment on pull request #154:
URL: https://github.com/apache/maven-resolver/pull/154#issuecomment-1066616895
IMO, as we still test with `endsWith`, dotless and dotted is important, but for simplicity sake, we'd maybe enforce configuration to use only dotted (dot prefixed) input. OTOH, I'd not apply anything like auto-adding dots, as that may hinder our later efforts to do something else (as we see that having or not having dot as prefix does matters, but we still don't know exactly what and how to use that, so better not allow users to be lazy that may hit us back later).
Simply put: change factory to enforce leading dots, and update doco that list must have extensions with leading dots.
Rationale:
* artifact.getExtension returns dotless extension (so not starting with dot), but sub-artifacts may have dots within extension (jar.asc)
* enforcing that users use dot-prefixed strings means we are applying this ONLY to sub-artifacts, that again, makes sense. I don't think we want to have top level artifacts that does NOT have checksums.
* still, having users adding dots, instead code auto-adding it leaves open a possibility for us, to change this later (and maybe use a list of some ant patterns or whatnot, but currently I'd see this as overkill)
--
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-resolver] cstamas commented on a change in pull request #154: [MRESOLVER-241] Resolver checksum calculation should be driven by layout
Posted by GitBox <gi...@apache.org>.
cstamas commented on a change in pull request #154:
URL: https://github.com/apache/maven-resolver/pull/154#discussion_r825358528
##########
File path: maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/checksum/DefaultArtifactChecksumFilterFactory.java
##########
@@ -0,0 +1,90 @@
+package org.eclipse.aether.internal.impl.checksum;
+
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import javax.inject.Named;
+import javax.inject.Singleton;
+
+import java.util.Arrays;
+import java.util.Set;
+import java.util.stream.Collectors;
+
+import org.eclipse.aether.RepositorySystemSession;
+import org.eclipse.aether.artifact.Artifact;
+import org.eclipse.aether.repository.RemoteRepository;
+import org.eclipse.aether.spi.connector.checksum.ArtifactChecksumFilter;
+import org.eclipse.aether.spi.connector.checksum.ArtifactChecksumFilterFactory;
+import org.eclipse.aether.util.ConfigUtils;
+
+import static java.util.Objects.requireNonNull;
+
+/**
+ * Default implementation that implements default resolver strategy: filters by known (user configured) extensions,
+ * or by default only for GPG signature.
+ *
+ * @since 1.8.0
+ */
+@Singleton
+@Named
+public class DefaultArtifactChecksumFilterFactory
+ implements ArtifactChecksumFilterFactory
+{
+ public static final String CONFIG_PROP_OMIT_CHECKSUMS_FOR_EXTENSIONS =
+ "aether.checksums.omitChecksumsForExtensions";
+
+ private static final String DEFAULT_OMIT_CHECKSUMS_FOR_EXTENSIONS = ".asc";
Review comment:
Having or not having leading period is important:
| inout | `artifact-1.0.foo` | `artifact-1.0.jar.foo` |
|-------|------------------|---------------------|
|`.foo` | not matches | matches |
|`foo`| metches | matches |
Some extensions (like PGP signature `asc`) may be only subartifact, so extension is actually `jar.asc` or `pom.asc`. Still, if we'd want to generalize this, we may do:
* if `extensionWithoutChecksums` starts with period => check with `endsWith`
* if `extensionWithoutChecksums` not starts with period => check with `equals`
WDYT?
--
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-resolver] cstamas commented on a change in pull request #154: [MRESOLVER-241] Resolver checksum calculation should be driven by layout
Posted by GitBox <gi...@apache.org>.
cstamas commented on a change in pull request #154:
URL: https://github.com/apache/maven-resolver/pull/154#discussion_r825361307
##########
File path: maven-resolver-spi/src/main/java/org/eclipse/aether/spi/connector/checksum/ArtifactChecksumFilter.java
##########
@@ -0,0 +1,44 @@
+package org.eclipse.aether.spi.connector.checksum;
+
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import org.eclipse.aether.artifact.Artifact;
+
+/**
+ * Filter that is able to tell does artifact have expected checksums or not. Most notably, artifacts like
+ * different kind of signatures will not have checksums.
+ *
+ * @since 1.8.0
+ */
+public interface ArtifactChecksumFilter
Review comment:
Another minor thing: am not quite ok with class name `ArtifactChecksumFilter`. This class basically tells does passed in Artifact should have checksums or not... and it does not "filter" per se...
--
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-resolver] michael-o commented on pull request #154: [MRESOLVER-241] Resolver checksum calculation should be driven by layout
Posted by GitBox <gi...@apache.org>.
michael-o commented on pull request #154:
URL: https://github.com/apache/maven-resolver/pull/154#issuecomment-1065978388
> If you're referring to Sigstore integration, it's up for discussion what the best naming conventions will be. I have x509, SSH, and PGP signing working. So probably want to keep `.asc` as not to interfere with existing PGP sigs, but for x509 maybe `.sig` and for SSH `.sshsig`. Not sure yet really.
OK, let's move this to a separate discussion when we have somthing usable.
--
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-resolver] cstamas commented on a change in pull request #154: [MRESOLVER-241] Resolver checksum calculation should be driven by layout
Posted by GitBox <gi...@apache.org>.
cstamas commented on a change in pull request #154:
URL: https://github.com/apache/maven-resolver/pull/154#discussion_r825356843
##########
File path: maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/Maven2RepositoryLayoutFactory.java
##########
@@ -225,32 +239,9 @@ public URI getLocation( Metadata metadata, boolean upload )
return checksumLocations;
}
- }
-
- private static class Maven2RepositoryLayoutEx
- extends Maven2RepositoryLayout
- {
-
- protected Maven2RepositoryLayoutEx( List<ChecksumAlgorithmFactory> checksumAlgorithms )
+ private boolean isChecksum( String extension )
{
- super( checksumAlgorithms );
+ return checksumAlgorithms.stream().anyMatch( a -> extension.endsWith( "." + a.getFileExtension() ) );
Review comment:
as extension of POM ~signature~ hash is ~`pom.asc`~ `pom.sha1`, also if you look at old code, it was doing same thing for same reason.
--
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-resolver] cstamas commented on a change in pull request #154: [MRESOLVER-241] Resolver checksum calculation should be driven by layout
Posted by GitBox <gi...@apache.org>.
cstamas commented on a change in pull request #154:
URL: https://github.com/apache/maven-resolver/pull/154#discussion_r825358956
##########
File path: maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/checksum/DefaultArtifactChecksumFilterFactory.java
##########
@@ -0,0 +1,90 @@
+package org.eclipse.aether.internal.impl.checksum;
+
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import javax.inject.Named;
+import javax.inject.Singleton;
+
+import java.util.Arrays;
+import java.util.Set;
+import java.util.stream.Collectors;
+
+import org.eclipse.aether.RepositorySystemSession;
+import org.eclipse.aether.artifact.Artifact;
+import org.eclipse.aether.repository.RemoteRepository;
+import org.eclipse.aether.spi.connector.checksum.ArtifactChecksumFilter;
+import org.eclipse.aether.spi.connector.checksum.ArtifactChecksumFilterFactory;
+import org.eclipse.aether.util.ConfigUtils;
+
+import static java.util.Objects.requireNonNull;
+
+/**
+ * Default implementation that implements default resolver strategy: filters by known (user configured) extensions,
+ * or by default only for GPG signature.
+ *
+ * @since 1.8.0
+ */
+@Singleton
+@Named
+public class DefaultArtifactChecksumFilterFactory
+ implements ArtifactChecksumFilterFactory
+{
+ public static final String CONFIG_PROP_OMIT_CHECKSUMS_FOR_EXTENSIONS =
+ "aether.checksums.omitChecksumsForExtensions";
+
+ private static final String DEFAULT_OMIT_CHECKSUMS_FOR_EXTENSIONS = ".asc";
Review comment:
But let's not complicate, let's agree that:
* this PR w/o any custom config (in this area) behaves as master resolver w/o any custom config (jn this area)
* this PR w/ `aether.checksums.omitChecksumsForExtensions=""` (empty string) behaves the same as master does with `aether.checksums.forSignatures=true`
* for start, let's document these are for sub-artifacts only (signatures) and MUST use leading period.
--
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-resolver] michael-o commented on a change in pull request #154: [MRESOLVER-241] Resolver checksum calculation should be driven by layout
Posted by GitBox <gi...@apache.org>.
michael-o commented on a change in pull request #154:
URL: https://github.com/apache/maven-resolver/pull/154#discussion_r825361384
##########
File path: maven-resolver-spi/src/main/java/org/eclipse/aether/spi/connector/checksum/ArtifactChecksumFilter.java
##########
@@ -0,0 +1,44 @@
+package org.eclipse.aether.spi.connector.checksum;
+
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import org.eclipse.aether.artifact.Artifact;
+
+/**
+ * Filter that is able to tell does artifact have expected checksums or not. Most notably, artifacts like
+ * different kind of signatures will not have checksums.
+ *
+ * @since 1.8.0
+ */
+public interface ArtifactChecksumFilter
Review comment:
`ArtifactChecksumDecider`?
--
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-resolver] cstamas commented on a change in pull request #154: [MRESOLVER-241] Resolver checksum calculation should be driven by layout
Posted by GitBox <gi...@apache.org>.
cstamas commented on a change in pull request #154:
URL: https://github.com/apache/maven-resolver/pull/154#discussion_r825358035
##########
File path: maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/checksum/DefaultArtifactChecksumFilterFactory.java
##########
@@ -0,0 +1,90 @@
+package org.eclipse.aether.internal.impl.checksum;
+
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import javax.inject.Named;
+import javax.inject.Singleton;
+
+import java.util.Arrays;
+import java.util.Set;
+import java.util.stream.Collectors;
+
+import org.eclipse.aether.RepositorySystemSession;
+import org.eclipse.aether.artifact.Artifact;
+import org.eclipse.aether.repository.RemoteRepository;
+import org.eclipse.aether.spi.connector.checksum.ArtifactChecksumFilter;
+import org.eclipse.aether.spi.connector.checksum.ArtifactChecksumFilterFactory;
+import org.eclipse.aether.util.ConfigUtils;
+
+import static java.util.Objects.requireNonNull;
+
+/**
+ * Default implementation that implements default resolver strategy: filters by known (user configured) extensions,
+ * or by default only for GPG signature.
+ *
+ * @since 1.8.0
+ */
+@Singleton
+@Named
+public class DefaultArtifactChecksumFilterFactory
+ implements ArtifactChecksumFilterFactory
+{
+ public static final String CONFIG_PROP_OMIT_CHECKSUMS_FOR_EXTENSIONS =
+ "aether.checksums.omitChecksumsForExtensions";
+
+ private static final String DEFAULT_OMIT_CHECKSUMS_FOR_EXTENSIONS = ".asc";
+
+ @Override
+ public ArtifactChecksumFilter newInstance( RepositorySystemSession session, RemoteRepository repository )
+ {
+ // ensure uniqueness of (potentially user set) extension list
+ Set<String> omitChecksumsForExtensions = Arrays.stream( ConfigUtils.getString(
+ session, DEFAULT_OMIT_CHECKSUMS_FOR_EXTENSIONS, CONFIG_PROP_OMIT_CHECKSUMS_FOR_EXTENSIONS )
+ .split( "," )
+ ).filter( s -> s != null && !s.trim().isEmpty() ).collect( Collectors.toSet() );
+
+ return new ExtensionArtifactChecksumFilter( omitChecksumsForExtensions );
+ }
+
+ private static class ExtensionArtifactChecksumFilter
+ implements ArtifactChecksumFilter
+ {
+ private final Set<String> extensionsWithoutChecksums;
+
+ private ExtensionArtifactChecksumFilter( Set<String> extensionsWithoutChecksums )
+ {
+ this.extensionsWithoutChecksums = requireNonNull( extensionsWithoutChecksums );
+ }
+
+ @Override
+ public boolean omitChecksumsFor( Artifact artifact )
+ {
+ String artifactExtension = artifact.getExtension(); // ie. pom.asc
+ for ( String extensionWithoutChecksums : extensionsWithoutChecksums )
+ {
+ if ( artifactExtension.endsWith( extensionWithoutChecksums ) )
Review comment:
I think is ok: in my case of extension foo, I'd use value ".asc,foo" (so no leading dot before foo), while for sub-artifact like signature, value to be used is `.asc` (w/ leading dot).
--
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-resolver] cstamas commented on a change in pull request #154: [MRESOLVER-241] Resolver checksum calculation should be driven by layout
Posted by GitBox <gi...@apache.org>.
cstamas commented on a change in pull request #154:
URL: https://github.com/apache/maven-resolver/pull/154#discussion_r825357214
##########
File path: maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/checksum/DefaultArtifactChecksumFilterFactory.java
##########
@@ -0,0 +1,90 @@
+package org.eclipse.aether.internal.impl.checksum;
+
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import javax.inject.Named;
+import javax.inject.Singleton;
+
+import java.util.Arrays;
+import java.util.Set;
+import java.util.stream.Collectors;
+
+import org.eclipse.aether.RepositorySystemSession;
+import org.eclipse.aether.artifact.Artifact;
+import org.eclipse.aether.repository.RemoteRepository;
+import org.eclipse.aether.spi.connector.checksum.ArtifactChecksumFilter;
+import org.eclipse.aether.spi.connector.checksum.ArtifactChecksumFilterFactory;
+import org.eclipse.aether.util.ConfigUtils;
+
+import static java.util.Objects.requireNonNull;
+
+/**
+ * Default implementation that implements default resolver strategy: filters by known (user configured) extensions,
+ * or by default only for GPG signature.
+ *
+ * @since 1.8.0
+ */
+@Singleton
+@Named
+public class DefaultArtifactChecksumFilterFactory
+ implements ArtifactChecksumFilterFactory
+{
+ public static final String CONFIG_PROP_OMIT_CHECKSUMS_FOR_EXTENSIONS =
+ "aether.checksums.omitChecksumsForExtensions";
+
+ private static final String DEFAULT_OMIT_CHECKSUMS_FOR_EXTENSIONS = ".asc";
Review comment:
The signature artifact.getExtension returns "jar.asc", and we used `endsWith(.asc)` for same reason. Basically I did not change what happens in old code, just shuffled it around.
--
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-resolver] michael-o commented on a change in pull request #154: [MRESOLVER-241] Resolver checksum calculation should be driven by layout
Posted by GitBox <gi...@apache.org>.
michael-o commented on a change in pull request #154:
URL: https://github.com/apache/maven-resolver/pull/154#discussion_r825357666
##########
File path: maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/Maven2RepositoryLayoutFactory.java
##########
@@ -225,32 +239,9 @@ public URI getLocation( Metadata metadata, boolean upload )
return checksumLocations;
}
- }
-
- private static class Maven2RepositoryLayoutEx
- extends Maven2RepositoryLayout
- {
-
- protected Maven2RepositoryLayoutEx( List<ChecksumAlgorithmFactory> checksumAlgorithms )
+ private boolean isChecksum( String extension )
{
- super( checksumAlgorithms );
+ return checksumAlgorithms.stream().anyMatch( a -> extension.endsWith( "." + a.getFileExtension() ) );
Review comment:
True, the `a` provides only `md5`, etc. and as you noticed it is `XXX.md5`. This makes sense.
--
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-resolver] cstamas commented on a change in pull request #154: [MRESOLVER-241] Resolver checksum calculation should be driven by layout
Posted by GitBox <gi...@apache.org>.
cstamas commented on a change in pull request #154:
URL: https://github.com/apache/maven-resolver/pull/154#discussion_r825360764
##########
File path: maven-resolver-spi/src/main/java/org/eclipse/aether/spi/connector/checksum/ArtifactChecksumFilter.java
##########
@@ -0,0 +1,44 @@
+package org.eclipse.aether.spi.connector.checksum;
+
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import org.eclipse.aether.artifact.Artifact;
+
+/**
+ * Filter that is able to tell does artifact have expected checksums or not. Most notably, artifacts like
+ * different kind of signatures will not have checksums.
+ *
+ * @since 1.8.0
+ */
+public interface ArtifactChecksumFilter
Review comment:
fun fact: Central for example (but same stands for Nx2 or Artifactory) does sent checksum header for ANY payload, hence, even checksum for checksum, and resolver in this PR DOES USE IT to validate payload.
--
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-resolver] michael-o commented on a change in pull request #154: [MRESOLVER-241] Resolver checksum calculation should be driven by layout
Posted by GitBox <gi...@apache.org>.
michael-o commented on a change in pull request #154:
URL: https://github.com/apache/maven-resolver/pull/154#discussion_r825358752
##########
File path: maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/checksum/DefaultArtifactChecksumFilterFactory.java
##########
@@ -0,0 +1,90 @@
+package org.eclipse.aether.internal.impl.checksum;
+
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import javax.inject.Named;
+import javax.inject.Singleton;
+
+import java.util.Arrays;
+import java.util.Set;
+import java.util.stream.Collectors;
+
+import org.eclipse.aether.RepositorySystemSession;
+import org.eclipse.aether.artifact.Artifact;
+import org.eclipse.aether.repository.RemoteRepository;
+import org.eclipse.aether.spi.connector.checksum.ArtifactChecksumFilter;
+import org.eclipse.aether.spi.connector.checksum.ArtifactChecksumFilterFactory;
+import org.eclipse.aether.util.ConfigUtils;
+
+import static java.util.Objects.requireNonNull;
+
+/**
+ * Default implementation that implements default resolver strategy: filters by known (user configured) extensions,
+ * or by default only for GPG signature.
+ *
+ * @since 1.8.0
+ */
+@Singleton
+@Named
+public class DefaultArtifactChecksumFilterFactory
+ implements ArtifactChecksumFilterFactory
+{
+ public static final String CONFIG_PROP_OMIT_CHECKSUMS_FOR_EXTENSIONS =
+ "aether.checksums.omitChecksumsForExtensions";
+
+ private static final String DEFAULT_OMIT_CHECKSUMS_FOR_EXTENSIONS = ".asc";
Review comment:
OK, then I did understand your intention correctly. Yes, you proposal makes sense and you clearly convey the difference. Please also document it in the config.md file.
--
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-resolver] cstamas commented on a change in pull request #154: [MRESOLVER-241] Resolver checksum calculation should be driven by layout
Posted by GitBox <gi...@apache.org>.
cstamas commented on a change in pull request #154:
URL: https://github.com/apache/maven-resolver/pull/154#discussion_r825360533
##########
File path: maven-resolver-spi/src/main/java/org/eclipse/aether/spi/connector/checksum/ArtifactChecksumFilter.java
##########
@@ -0,0 +1,44 @@
+package org.eclipse.aether.spi.connector.checksum;
+
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import org.eclipse.aether.artifact.Artifact;
+
+/**
+ * Filter that is able to tell does artifact have expected checksums or not. Most notably, artifacts like
+ * different kind of signatures will not have checksums.
+ *
+ * @since 1.8.0
+ */
+public interface ArtifactChecksumFilter
Review comment:
We should clarify here: this is ONLY FOR REMOTE_EXTERNAL checksums. So if this class returns `true`, on deploy checksums will NOT be calculated and deployed, BUT on retrieve, the stream checksums ARE calculated (to satisfy PROVIDED or REMOTE_INCLUDED), but REMOTE_EXTERNAL ones will NOT be fetched (and hence will not be validated).
--
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-resolver] cstamas commented on a change in pull request #154: [MRESOLVER-241] Resolver checksum calculation should be driven by layout
Posted by GitBox <gi...@apache.org>.
cstamas commented on a change in pull request #154:
URL: https://github.com/apache/maven-resolver/pull/154#discussion_r825480990
##########
File path: maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/Maven2RepositoryLayoutFactory.java
##########
@@ -118,9 +123,16 @@ public RepositoryLayout newInstance( RepositorySystemSession session, RemoteRepo
checksumsAlgorithms.add( checksumAlgorithmFactorySelector.select( checksumsAlgorithmName ) );
}
- return forSignature
- ? new Maven2RepositoryLayout( checksumsAlgorithms )
- : new Maven2RepositoryLayoutEx( checksumsAlgorithms );
+ // ensure uniqueness of (potentially user set) extension list
+ Set<String> omitChecksumsForExtensions = Arrays.stream( ConfigUtils.getString(
+ session, DEFAULT_OMIT_CHECKSUMS_FOR_EXTENSIONS, CONFIG_PROP_OMIT_CHECKSUMS_FOR_EXTENSIONS )
+ .split( "," )
+ ).filter( s -> s != null && !s.trim().isEmpty() ).collect( Collectors.toSet() );
Review comment:
applied to both
--
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-resolver] michael-o commented on pull request #154: [MRESOLVER-241] Resolver checksum calculation should be driven by layout
Posted by GitBox <gi...@apache.org>.
michael-o commented on pull request #154:
URL: https://github.com/apache/maven-resolver/pull/154#issuecomment-1066450554
> What happended to `foo`,`.asc` dot and dotless?
Only this one is left. I will put this into testing meanwhile.
--
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-resolver] michael-o commented on pull request #154: [MRESOLVER-241] Resolver checksum calculation should be driven by layout
Posted by GitBox <gi...@apache.org>.
michael-o commented on pull request #154:
URL: https://github.com/apache/maven-resolver/pull/154#issuecomment-1067683173
Going through 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.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-resolver] cstamas merged pull request #154: [MRESOLVER-241] Resolver checksum calculation should be driven by layout
Posted by GitBox <gi...@apache.org>.
cstamas merged pull request #154:
URL: https://github.com/apache/maven-resolver/pull/154
--
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-resolver] michael-o commented on a change in pull request #154: [MRESOLVER-241] Resolver checksum calculation should be driven by layout
Posted by GitBox <gi...@apache.org>.
michael-o commented on a change in pull request #154:
URL: https://github.com/apache/maven-resolver/pull/154#discussion_r825357467
##########
File path: maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/checksum/DefaultArtifactChecksumFilterFactory.java
##########
@@ -0,0 +1,90 @@
+package org.eclipse.aether.internal.impl.checksum;
+
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import javax.inject.Named;
+import javax.inject.Singleton;
+
+import java.util.Arrays;
+import java.util.Set;
+import java.util.stream.Collectors;
+
+import org.eclipse.aether.RepositorySystemSession;
+import org.eclipse.aether.artifact.Artifact;
+import org.eclipse.aether.repository.RemoteRepository;
+import org.eclipse.aether.spi.connector.checksum.ArtifactChecksumFilter;
+import org.eclipse.aether.spi.connector.checksum.ArtifactChecksumFilterFactory;
+import org.eclipse.aether.util.ConfigUtils;
+
+import static java.util.Objects.requireNonNull;
+
+/**
+ * Default implementation that implements default resolver strategy: filters by known (user configured) extensions,
+ * or by default only for GPG signature.
+ *
+ * @since 1.8.0
+ */
+@Singleton
+@Named
+public class DefaultArtifactChecksumFilterFactory
+ implements ArtifactChecksumFilterFactory
+{
+ public static final String CONFIG_PROP_OMIT_CHECKSUMS_FOR_EXTENSIONS =
+ "aether.checksums.omitChecksumsForExtensions";
+
+ private static final String DEFAULT_OMIT_CHECKSUMS_FOR_EXTENSIONS = ".asc";
Review comment:
Since you have have basically replaced the old property I would strive for a cleanup and consistency with `#getExtension()`.
--
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-resolver] michael-o commented on a change in pull request #154: [MRESOLVER-241] Resolver checksum calculation should be driven by layout
Posted by GitBox <gi...@apache.org>.
michael-o commented on a change in pull request #154:
URL: https://github.com/apache/maven-resolver/pull/154#discussion_r825359150
##########
File path: maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/checksum/DefaultArtifactChecksumFilterFactory.java
##########
@@ -0,0 +1,90 @@
+package org.eclipse.aether.internal.impl.checksum;
+
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import javax.inject.Named;
+import javax.inject.Singleton;
+
+import java.util.Arrays;
+import java.util.Set;
+import java.util.stream.Collectors;
+
+import org.eclipse.aether.RepositorySystemSession;
+import org.eclipse.aether.artifact.Artifact;
+import org.eclipse.aether.repository.RemoteRepository;
+import org.eclipse.aether.spi.connector.checksum.ArtifactChecksumFilter;
+import org.eclipse.aether.spi.connector.checksum.ArtifactChecksumFilterFactory;
+import org.eclipse.aether.util.ConfigUtils;
+
+import static java.util.Objects.requireNonNull;
+
+/**
+ * Default implementation that implements default resolver strategy: filters by known (user configured) extensions,
+ * or by default only for GPG signature.
+ *
+ * @since 1.8.0
+ */
+@Singleton
+@Named
+public class DefaultArtifactChecksumFilterFactory
+ implements ArtifactChecksumFilterFactory
+{
+ public static final String CONFIG_PROP_OMIT_CHECKSUMS_FOR_EXTENSIONS =
+ "aether.checksums.omitChecksumsForExtensions";
+
+ private static final String DEFAULT_OMIT_CHECKSUMS_FOR_EXTENSIONS = ".asc";
Review comment:
Agreed.
--
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-resolver] cstamas commented on a change in pull request #154: [MRESOLVER-241] Resolver checksum calculation should be driven by layout
Posted by GitBox <gi...@apache.org>.
cstamas commented on a change in pull request #154:
URL: https://github.com/apache/maven-resolver/pull/154#discussion_r825363398
##########
File path: maven-resolver-spi/src/main/java/org/eclipse/aether/spi/connector/checksum/ArtifactChecksumFilter.java
##########
@@ -0,0 +1,44 @@
+package org.eclipse.aether.spi.connector.checksum;
+
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import org.eclipse.aether.artifact.Artifact;
+
+/**
+ * Filter that is able to tell does artifact have expected checksums or not. Most notably, artifacts like
+ * different kind of signatures will not have checksums.
+ *
+ * @since 1.8.0
+ */
+public interface ArtifactChecksumFilter
Review comment:
hm, this needs more refactoring: I just realized, this is internal matter of Maven2RepositoryLayoutFactory. Still, IMO we do want to have it extensible, but as is in this PR, it seems misleading, as it's factory is really neglecting passed in repository completely, while all this applies to maven2 layout only. Also, I don't think a generic factory is right here, hence, I dislike to copy+paste same logic as in maven2 layout factory... Finally, I'd make this part of layout, not checksum package, maybe even just as a method on RepositoryLayout (to expose it) but it would be used internally by maven2 layout to decide...
--
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-resolver] cstamas commented on a change in pull request #154: [MRESOLVER-241] Resolver checksum calculation should be driven by layout
Posted by GitBox <gi...@apache.org>.
cstamas commented on a change in pull request #154:
URL: https://github.com/apache/maven-resolver/pull/154#discussion_r825360533
##########
File path: maven-resolver-spi/src/main/java/org/eclipse/aether/spi/connector/checksum/ArtifactChecksumFilter.java
##########
@@ -0,0 +1,44 @@
+package org.eclipse.aether.spi.connector.checksum;
+
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import org.eclipse.aether.artifact.Artifact;
+
+/**
+ * Filter that is able to tell does artifact have expected checksums or not. Most notably, artifacts like
+ * different kind of signatures will not have checksums.
+ *
+ * @since 1.8.0
+ */
+public interface ArtifactChecksumFilter
Review comment:
We should clarify here: this is ONLY FOR REMOTE_EXTERNAL checksums. So if this class returns `true`, on deploy checksums will NOT be calculated and deployed, BUT on retrieve, the response payload checksums ARE calculated (to satisfy PROVIDED or REMOTE_INCLUDED), but REMOTE_EXTERNAL ones will NOT be fetched (and hence will not be validated).
--
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-resolver] cstamas commented on a change in pull request #154: [MRESOLVER-241] Resolver checksum calculation should be driven by layout
Posted by GitBox <gi...@apache.org>.
cstamas commented on a change in pull request #154:
URL: https://github.com/apache/maven-resolver/pull/154#discussion_r825360533
##########
File path: maven-resolver-spi/src/main/java/org/eclipse/aether/spi/connector/checksum/ArtifactChecksumFilter.java
##########
@@ -0,0 +1,44 @@
+package org.eclipse.aether.spi.connector.checksum;
+
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import org.eclipse.aether.artifact.Artifact;
+
+/**
+ * Filter that is able to tell does artifact have expected checksums or not. Most notably, artifacts like
+ * different kind of signatures will not have checksums.
+ *
+ * @since 1.8.0
+ */
+public interface ArtifactChecksumFilter
Review comment:
We should clarify here: this is ONLY FOR REMOTE_EXTERNAL artifacts. So if this class returns `true`, on deploy checksums will NOT be calculated and deployed, BUT on retrieve, the stream checksums ARE calculated (to satisfy PROVIDED or REMOTE_INCLUDED), but REMOTE_EXTERNAL ones will NOT be fetched (and hence will not be validated).
--
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-resolver] michael-o commented on pull request #154: [MRESOLVER-241] Resolver checksum calculation should be driven by layout
Posted by GitBox <gi...@apache.org>.
michael-o commented on pull request #154:
URL: https://github.com/apache/maven-resolver/pull/154#issuecomment-1065977636
@jvanzyl Should we add `.sig` right away to the default list?
--
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