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/10/17 09:32:31 UTC
[GitHub] [maven-resolver] cstamas opened a new pull request, #203: Simplify and fix trusted checksum sources.
cstamas opened a new pull request, #203:
URL: https://github.com/apache/maven-resolver/pull/203
Changes:
* support class meddles less re basedir
* sparse source: use `fileProcessor` to read and write checksums (not mix fileProcessor and Files.write), better handle basedir
* summary source: clearly split "record" and non-recording, enhance Writer, add "truncateOnSave" (otherwise merge existing 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 diff in pull request #203: [MRESOLVER-279] Simplify and improve trusted checksum sources
Posted by GitBox <gi...@apache.org>.
cstamas commented on code in PR #203:
URL: https://github.com/apache/maven-resolver/pull/203#discussion_r997959530
##########
maven-resolver-util/src/main/java/org/eclipse/aether/util/FileUtils.java:
##########
@@ -0,0 +1,112 @@
+package org.eclipse.aether.util;
+
+/*
+ * 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 java.io.IOException;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.nio.file.StandardCopyOption;
+
+import static java.util.Objects.requireNonNull;
+
+/**
+ * A utility class to write files.
+ *
+ * @since TBD
+ */
+public final class FileUtils
+{
+ private FileUtils()
+ {
+ // hide constructor
+ }
+
+ /**
+ * A file writer, that accepts a {@link Path} to write some content to.
+ */
+ @FunctionalInterface
+ public interface FileWriter
+ {
+ void write( Path path ) throws IOException;
+ }
+
+ /**
+ * Writes file without backup.
+ *
+ * @param target that is the target file (must be file, the path must have parent).
+ * @param writer the writer that will accept a {@link Path} to write content to.
+ * @throws IOException if at any step IO problem occurs.
+ */
+ public static void writeFile( Path target, FileWriter writer ) throws IOException
+ {
+ writeFile( target, writer, false );
+ }
+
+ /**
+ * Writes file with backup copy (appends ".bak" extension).
+ *
+ * @param target that is the target file (must be file, the path must have parent).
+ * @param writer the writer that will accept a {@link Path} to write content to.
+ * @throws IOException if at any step IO problem occurs.
+ */
+ public static void writeFileWithBackup( Path target, FileWriter writer ) throws IOException
+ {
+ writeFile( target, writer, true );
+ }
+
+ /**
+ * Utility method to write out file to disk in "atomic" manner, with optional backups (".bak") if needed. This
+ * ensures that no other thread or process will be able to read not fully written files. Finally, this methos
+ * may create the needed parent directories, if the passed in target parents does not exist.
+ *
+ * @param target that is the target file (must be file, the path must have parent).
+ * @param writer the writer that will accept a {@link Path} to write content to.
+ * @param doBackup if {@code true}, and target file is about to be overwritten, a ".bak" file with old contents will
+ * be created/overwritten.
+ * @throws IOException if at any step IO problem occurs.
+ */
+ private static void writeFile( Path target, FileWriter writer, boolean doBackup ) throws IOException
+ {
+ requireNonNull( target, "target is null" );
+ requireNonNull( writer, "writer is null" );
+ Path parent = requireNonNull( target.getParent(), "target must be a file" );
+ Path temp = null;
+
+ Files.createDirectories( target.getParent() );
Review Comment:
fixed
--
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 diff in pull request #203: [MRESOLVER-279] Simplify and improve trusted checksum sources
Posted by GitBox <gi...@apache.org>.
michael-o commented on code in PR #203:
URL: https://github.com/apache/maven-resolver/pull/203#discussion_r1002674110
##########
maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/checksum/FileTrustedChecksumsSourceSupport.java:
##########
@@ -151,29 +138,40 @@ protected String configPropKey( String name )
}
/**
- * Returns {@code true} if session configuration contains "originAware" property set to {@code true}.
+ * Returns {@code true} if session configuration marks this instance as enabled.
+ * <p>
+ * Default value is {@code false}.
+ */
+ protected boolean isEnabled( RepositorySystemSession session )
+ {
+ return ConfigUtils.getBoolean( session, false, CONFIG_PROP_PREFIX + this.name );
+ }
+
+ /**
+ * Returns {@code true} if session configuration marks this instance as origin aware.
+ * <p>
+ * Default value is {@code true}.
*/
protected boolean isOriginAware( RepositorySystemSession session )
{
- return ConfigUtils.getBoolean( session, false, configPropKey( CONF_NAME_ORIGIN_AWARE ) );
+ return ConfigUtils.getBoolean( session, true, configPropKey( CONF_NAME_ORIGIN_AWARE ) );
Review Comment:
Does this really make sense since we don't split artifacts by repo neither?
##########
maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/checksum/SummaryFileTrustedChecksumsSource.java:
##########
@@ -27,45 +28,62 @@
import java.io.UncheckedIOException;
import java.nio.charset.StandardCharsets;
import java.nio.file.Files;
-import java.nio.file.NoSuchFileException;
import java.nio.file.Path;
import java.util.ArrayList;
-import java.util.Collections;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.Objects;
-import java.util.Set;
-import java.util.TreeSet;
import java.util.concurrent.ConcurrentHashMap;
+import java.util.concurrent.atomic.AtomicBoolean;
import org.eclipse.aether.MultiRuntimeException;
import org.eclipse.aether.RepositorySystemSession;
import org.eclipse.aether.artifact.Artifact;
+import org.eclipse.aether.internal.impl.LocalPathComposer;
import org.eclipse.aether.repository.ArtifactRepository;
import org.eclipse.aether.spi.connector.checksum.ChecksumAlgorithmFactory;
-import org.eclipse.aether.util.artifact.ArtifactIdUtils;
+import org.eclipse.aether.util.ConfigUtils;
+import org.eclipse.aether.util.FileUtils;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import static java.util.Objects.requireNonNull;
+import static java.util.stream.Collectors.toList;
/**
* Compact file {@link FileTrustedChecksumsSourceSupport} implementation that use specified directory as base
- * directory, where it expects a "summary" file named as "checksums.${checksumExt}" for each checksum algorithm, and
- * file format is artifact ID and checksum separated by space per line. The format supports comments "#" (hash) and
- * empty lines (both are ignored).
+ * directory, where it expects a "summary" file named as "checksums.${checksumExt}" for each checksum algorithm.
+ * File format is GNU Coreutils compatible: each line holds checksum followed by two spaces and artifact relative path
+ * (from local repository root, without leading "./"). This means that trusted checksums summary file can be used to
+ * validate artifacts or generate it using standard GNU tools like GNU {@code sha1sum} is (for BSD derivatives same
+ * file can be used with {@code -r} switch).
* <p>
- * The source may be configured to be "origin aware", in that case it will factor in origin repository ID as well into
- * file name (for example "checksums-central.sha1").
+ * The format supports comments "#" (hash) and empty lines for easier structuring the file content, and both are
+ * ignored. Also, their presence makes the summary file incompatible with GNU Coreutils format. On save of the
+ * summary file, the comments and empty lines are lost, and file is sorted by path names for easier diffing
+ * (2nd column in file).
* <p>
- * The checksums file once loaded are cached in session, so in-flight file changes during lifecycle of session are NOT
- * noticed.
+ * The source by default is "origin aware", and it will factor in origin repository ID as well into summary file name,
+ * for example "checksums-central.sha256".
+ * <p>
+ * Example commands for managing summary file (in examples will use repository ID "central"):
+ * <ul>
+ * <li>To create summary file: {@code find * -not -name "checksums-central.sha256" -type f -print0 |
+ * xargs -0 sha256sum | sort -k 2 > checksums-central.sha256}</li>
+ * <li>To verify artifacts using summary file: {@code sha256sum --quiet -c checksums-central.sha256}</li>
+ * </ul>
+ * <p>
+ * The checksums summary file is lazily loaded and remains cached in session, so file changes during lifecycle of the
+ * session are not picked up. This implementation can be simultaneously used to lookup and also write checksums. The
+ * written checksums will become visible only for writer session, and newly written checksums, if any, will be flushed
Review Comment:
So this is not really per physical file and NOT per GAV?
--
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 diff in pull request #203: [MRESOLVER-279] Simplify and improve trusted checksum sources
Posted by GitBox <gi...@apache.org>.
cstamas commented on code in PR #203:
URL: https://github.com/apache/maven-resolver/pull/203#discussion_r997963599
##########
maven-resolver-util/src/main/java/org/eclipse/aether/util/FileUtils.java:
##########
@@ -0,0 +1,112 @@
+package org.eclipse.aether.util;
+
+/*
+ * 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 java.io.IOException;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.nio.file.StandardCopyOption;
+
+import static java.util.Objects.requireNonNull;
+
+/**
+ * A utility class to write files.
+ *
+ * @since TBD
+ */
+public final class FileUtils
+{
+ private FileUtils()
+ {
+ // hide constructor
+ }
+
+ /**
+ * A file writer, that accepts a {@link Path} to write some content to.
+ */
+ @FunctionalInterface
+ public interface FileWriter
+ {
+ void write( Path path ) throws IOException;
+ }
+
+ /**
+ * Writes file without backup.
+ *
+ * @param target that is the target file (must be file, the path must have parent).
+ * @param writer the writer that will accept a {@link Path} to write content to.
+ * @throws IOException if at any step IO problem occurs.
+ */
+ public static void writeFile( Path target, FileWriter writer ) throws IOException
+ {
+ writeFile( target, writer, false );
+ }
+
+ /**
+ * Writes file with backup copy (appends ".bak" extension).
+ *
+ * @param target that is the target file (must be file, the path must have parent).
+ * @param writer the writer that will accept a {@link Path} to write content to.
+ * @throws IOException if at any step IO problem occurs.
+ */
+ public static void writeFileWithBackup( Path target, FileWriter writer ) throws IOException
+ {
+ writeFile( target, writer, true );
+ }
+
+ /**
+ * Utility method to write out file to disk in "atomic" manner, with optional backups (".bak") if needed. This
+ * ensures that no other thread or process will be able to read not fully written files. Finally, this methos
+ * may create the needed parent directories, if the passed in target parents does not exist.
+ *
+ * @param target that is the target file (must be file, the path must have parent).
+ * @param writer the writer that will accept a {@link Path} to write content to.
+ * @param doBackup if {@code true}, and target file is about to be overwritten, a ".bak" file with old contents will
+ * be created/overwritten.
+ * @throws IOException if at any step IO problem occurs.
+ */
+ private static void writeFile( Path target, FileWriter writer, boolean doBackup ) throws IOException
+ {
+ requireNonNull( target, "target is null" );
+ requireNonNull( writer, "writer is null" );
+ Path parent = requireNonNull( target.getParent(), "target must be a file" );
Review Comment:
https://docs.oracle.com/javase/8/docs/api/java/nio/file/Path.html#getParent--
On unix, "/foo" has a parent "/" while "/" has no parent (null is returned). Unsure what happens on ~non-OSes~ toy OSes though. Point here is to not use this method with `target="/"` but only `target="/foo"`, that target path must have parent.
--
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 diff in pull request #203: [MRESOLVER-279] Simplify and improve trusted checksum sources
Posted by GitBox <gi...@apache.org>.
cstamas commented on code in PR #203:
URL: https://github.com/apache/maven-resolver/pull/203#discussion_r997963599
##########
maven-resolver-util/src/main/java/org/eclipse/aether/util/FileUtils.java:
##########
@@ -0,0 +1,112 @@
+package org.eclipse.aether.util;
+
+/*
+ * 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 java.io.IOException;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.nio.file.StandardCopyOption;
+
+import static java.util.Objects.requireNonNull;
+
+/**
+ * A utility class to write files.
+ *
+ * @since TBD
+ */
+public final class FileUtils
+{
+ private FileUtils()
+ {
+ // hide constructor
+ }
+
+ /**
+ * A file writer, that accepts a {@link Path} to write some content to.
+ */
+ @FunctionalInterface
+ public interface FileWriter
+ {
+ void write( Path path ) throws IOException;
+ }
+
+ /**
+ * Writes file without backup.
+ *
+ * @param target that is the target file (must be file, the path must have parent).
+ * @param writer the writer that will accept a {@link Path} to write content to.
+ * @throws IOException if at any step IO problem occurs.
+ */
+ public static void writeFile( Path target, FileWriter writer ) throws IOException
+ {
+ writeFile( target, writer, false );
+ }
+
+ /**
+ * Writes file with backup copy (appends ".bak" extension).
+ *
+ * @param target that is the target file (must be file, the path must have parent).
+ * @param writer the writer that will accept a {@link Path} to write content to.
+ * @throws IOException if at any step IO problem occurs.
+ */
+ public static void writeFileWithBackup( Path target, FileWriter writer ) throws IOException
+ {
+ writeFile( target, writer, true );
+ }
+
+ /**
+ * Utility method to write out file to disk in "atomic" manner, with optional backups (".bak") if needed. This
+ * ensures that no other thread or process will be able to read not fully written files. Finally, this methos
+ * may create the needed parent directories, if the passed in target parents does not exist.
+ *
+ * @param target that is the target file (must be file, the path must have parent).
+ * @param writer the writer that will accept a {@link Path} to write content to.
+ * @param doBackup if {@code true}, and target file is about to be overwritten, a ".bak" file with old contents will
+ * be created/overwritten.
+ * @throws IOException if at any step IO problem occurs.
+ */
+ private static void writeFile( Path target, FileWriter writer, boolean doBackup ) throws IOException
+ {
+ requireNonNull( target, "target is null" );
+ requireNonNull( writer, "writer is null" );
+ Path parent = requireNonNull( target.getParent(), "target must be a file" );
Review Comment:
https://docs.oracle.com/javase/8/docs/api/java/nio/file/Path.html#getParent--
On unix, "/foo" has a parent "/" while "/" has no parent (null is returned). Unsure what happens on non-OSes though. Point here is to not use this method with `target="/"` but only `target="/foo"`, that target path must have parent.
--
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 diff in pull request #203: [MRESOLVER-279] Simplify and improve trusted checksum sources
Posted by GitBox <gi...@apache.org>.
cstamas commented on code in PR #203:
URL: https://github.com/apache/maven-resolver/pull/203#discussion_r1004080129
##########
maven-resolver-impl/src/test/java/org/eclipse/aether/internal/impl/checksum/FileTrustedChecksumsSourceTestSupport.java:
##########
@@ -51,58 +56,79 @@
private FileTrustedChecksumsSourceSupport subject;
+ private boolean checksumWritten;
+
@Before
public void before() throws Exception
{
session = TestUtils.newSession();
// populate local repository
- Path basedir = session.getLocalRepository().getBasedir().toPath()
- .resolve( FileTrustedChecksumsSourceSupport.LOCAL_REPO_PREFIX_DIR );
checksumAlgorithmFactory = new Sha1ChecksumAlgorithmFactory();
- subject = prepareSubject( basedir );
+ subject = prepareSubject();
+ checksumWritten = false;
+
+ try ( DefaultRepositorySystemSession prepareSession = new DefaultRepositorySystemSession( session ) )
+ {
+ enableSource( prepareSession );
+ try ( TrustedChecksumsSource.Writer writer = subject.getTrustedArtifactChecksumsWriter( prepareSession ) )
+ {
+ if ( writer != null )
+ {
+ HashMap<String, String> checksums = new HashMap<>();
+ checksums.put( checksumAlgorithmFactory.getName(), ARTIFACT_TRUSTED_CHECKSUM );
+ writer.addTrustedArtifactChecksums( ARTIFACT_WITH_CHECKSUM, prepareSession.getLocalRepository(),
+ Collections.singletonList( checksumAlgorithmFactory ), checksums );
+ checksumWritten = true;
+ }
+ }
+ }
}
- protected abstract FileTrustedChecksumsSourceSupport prepareSubject( Path basedir ) throws IOException;
+ protected abstract FileTrustedChecksumsSourceSupport prepareSubject();
- protected abstract void enableSource();
+ protected abstract void enableSource( DefaultRepositorySystemSession session );
Review Comment:
You did, the session passed as param is used, may make support session private to make it more obvious
--
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 diff in pull request #203: [MRESOLVER-279] Simplify and improve trusted checksum sources
Posted by GitBox <gi...@apache.org>.
michael-o commented on code in PR #203:
URL: https://github.com/apache/maven-resolver/pull/203#discussion_r1004095720
##########
maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/checksum/SummaryFileTrustedChecksumsSource.java:
##########
@@ -27,45 +28,62 @@
import java.io.UncheckedIOException;
import java.nio.charset.StandardCharsets;
import java.nio.file.Files;
-import java.nio.file.NoSuchFileException;
import java.nio.file.Path;
import java.util.ArrayList;
-import java.util.Collections;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.Objects;
-import java.util.Set;
-import java.util.TreeSet;
import java.util.concurrent.ConcurrentHashMap;
+import java.util.concurrent.atomic.AtomicBoolean;
import org.eclipse.aether.MultiRuntimeException;
import org.eclipse.aether.RepositorySystemSession;
import org.eclipse.aether.artifact.Artifact;
+import org.eclipse.aether.internal.impl.LocalPathComposer;
import org.eclipse.aether.repository.ArtifactRepository;
import org.eclipse.aether.spi.connector.checksum.ChecksumAlgorithmFactory;
-import org.eclipse.aether.util.artifact.ArtifactIdUtils;
+import org.eclipse.aether.util.ConfigUtils;
+import org.eclipse.aether.util.FileUtils;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import static java.util.Objects.requireNonNull;
+import static java.util.stream.Collectors.toList;
/**
* Compact file {@link FileTrustedChecksumsSourceSupport} implementation that use specified directory as base
- * directory, where it expects a "summary" file named as "checksums.${checksumExt}" for each checksum algorithm, and
- * file format is artifact ID and checksum separated by space per line. The format supports comments "#" (hash) and
- * empty lines (both are ignored).
+ * directory, where it expects a "summary" file named as "checksums.${checksumExt}" for each checksum algorithm.
+ * File format is GNU Coreutils compatible: each line holds checksum followed by two spaces and artifact relative path
+ * (from local repository root, without leading "./"). This means that trusted checksums summary file can be used to
+ * validate artifacts or generate it using standard GNU tools like GNU {@code sha1sum} is (for BSD derivatives same
+ * file can be used with {@code -r} switch).
* <p>
- * The source may be configured to be "origin aware", in that case it will factor in origin repository ID as well into
- * file name (for example "checksums-central.sha1").
+ * The format supports comments "#" (hash) and empty lines for easier structuring the file content, and both are
+ * ignored. Also, their presence makes the summary file incompatible with GNU Coreutils format. On save of the
+ * summary file, the comments and empty lines are lost, and file is sorted by path names for easier diffing
+ * (2nd column in file).
* <p>
- * The checksums file once loaded are cached in session, so in-flight file changes during lifecycle of session are NOT
- * noticed.
+ * The source by default is "origin aware", and it will factor in origin repository ID as well into summary file name,
+ * for example "checksums-central.sha256".
+ * <p>
+ * Example commands for managing summary file (in examples will use repository ID "central"):
+ * <ul>
+ * <li>To create summary file: {@code find * -not -name "checksums-central.sha256" -type f -print0 |
+ * xargs -0 sha256sum | sort -k 2 > checksums-central.sha256}</li>
+ * <li>To verify artifacts using summary file: {@code sha256sum --quiet -c checksums-central.sha256}</li>
+ * </ul>
+ * <p>
+ * The checksums summary file is lazily loaded and remains cached in session, so file changes during lifecycle of the
+ * session are not picked up. This implementation can be simultaneously used to lookup and also write checksums. The
+ * written checksums will become visible only for writer session, and newly written checksums, if any, will be flushed
Review Comment:
This is basically what I understood.
--
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 diff in pull request #203: [MRESOLVER-279] Simplify and improve trusted checksum sources
Posted by GitBox <gi...@apache.org>.
michael-o commented on code in PR #203:
URL: https://github.com/apache/maven-resolver/pull/203#discussion_r997978146
##########
maven-resolver-util/src/main/java/org/eclipse/aether/util/FileUtils.java:
##########
@@ -0,0 +1,112 @@
+package org.eclipse.aether.util;
+
+/*
+ * 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 java.io.IOException;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.nio.file.StandardCopyOption;
+
+import static java.util.Objects.requireNonNull;
+
+/**
+ * A utility class to write files.
+ *
+ * @since TBD
+ */
+public final class FileUtils
+{
+ private FileUtils()
+ {
+ // hide constructor
+ }
+
+ /**
+ * A file writer, that accepts a {@link Path} to write some content to.
+ */
+ @FunctionalInterface
+ public interface FileWriter
+ {
+ void write( Path path ) throws IOException;
+ }
+
+ /**
+ * Writes file without backup.
+ *
+ * @param target that is the target file (must be file, the path must have parent).
+ * @param writer the writer that will accept a {@link Path} to write content to.
+ * @throws IOException if at any step IO problem occurs.
+ */
+ public static void writeFile( Path target, FileWriter writer ) throws IOException
+ {
+ writeFile( target, writer, false );
+ }
+
+ /**
+ * Writes file with backup copy (appends ".bak" extension).
+ *
+ * @param target that is the target file (must be file, the path must have parent).
+ * @param writer the writer that will accept a {@link Path} to write content to.
+ * @throws IOException if at any step IO problem occurs.
+ */
+ public static void writeFileWithBackup( Path target, FileWriter writer ) throws IOException
+ {
+ writeFile( target, writer, true );
+ }
+
+ /**
+ * Utility method to write out file to disk in "atomic" manner, with optional backups (".bak") if needed. This
+ * ensures that no other thread or process will be able to read not fully written files. Finally, this methos
+ * may create the needed parent directories, if the passed in target parents does not exist.
+ *
+ * @param target that is the target file (must be file, the path must have parent).
+ * @param writer the writer that will accept a {@link Path} to write content to.
+ * @param doBackup if {@code true}, and target file is about to be overwritten, a ".bak" file with old contents will
+ * be created/overwritten.
+ * @throws IOException if at any step IO problem occurs.
+ */
+ private static void writeFile( Path target, FileWriter writer, boolean doBackup ) throws IOException
+ {
+ requireNonNull( target, "target is null" );
+ requireNonNull( writer, "writer is null" );
+ Path parent = requireNonNull( target.getParent(), "target must be a file" );
Review Comment:
Ah OK, now this is clear.
--
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 diff in pull request #203: [MRESOLVER-279] Simplify and improve trusted checksum sources
Posted by GitBox <gi...@apache.org>.
michael-o commented on code in PR #203:
URL: https://github.com/apache/maven-resolver/pull/203#discussion_r997948338
##########
maven-resolver-util/src/main/java/org/eclipse/aether/util/FileUtils.java:
##########
@@ -0,0 +1,112 @@
+package org.eclipse.aether.util;
+
+/*
+ * 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 java.io.IOException;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.nio.file.StandardCopyOption;
+
+import static java.util.Objects.requireNonNull;
+
+/**
+ * A utility class to write files.
+ *
+ * @since TBD
+ */
+public final class FileUtils
+{
+ private FileUtils()
+ {
+ // hide constructor
+ }
+
+ /**
+ * A file writer, that accepts a {@link Path} to write some content to.
+ */
+ @FunctionalInterface
+ public interface FileWriter
+ {
+ void write( Path path ) throws IOException;
+ }
+
+ /**
+ * Writes file without backup.
+ *
+ * @param target that is the target file (must be file, the path must have parent).
+ * @param writer the writer that will accept a {@link Path} to write content to.
+ * @throws IOException if at any step IO problem occurs.
+ */
+ public static void writeFile( Path target, FileWriter writer ) throws IOException
+ {
+ writeFile( target, writer, false );
+ }
+
+ /**
+ * Writes file with backup copy (appends ".bak" extension).
+ *
+ * @param target that is the target file (must be file, the path must have parent).
+ * @param writer the writer that will accept a {@link Path} to write content to.
+ * @throws IOException if at any step IO problem occurs.
+ */
+ public static void writeFileWithBackup( Path target, FileWriter writer ) throws IOException
+ {
+ writeFile( target, writer, true );
+ }
+
+ /**
+ * Utility method to write out file to disk in "atomic" manner, with optional backups (".bak") if needed. This
+ * ensures that no other thread or process will be able to read not fully written files. Finally, this methos
+ * may create the needed parent directories, if the passed in target parents does not exist.
+ *
+ * @param target that is the target file (must be file, the path must have parent).
+ * @param writer the writer that will accept a {@link Path} to write content to.
+ * @param doBackup if {@code true}, and target file is about to be overwritten, a ".bak" file with old contents will
+ * be created/overwritten.
+ * @throws IOException if at any step IO problem occurs.
+ */
+ private static void writeFile( Path target, FileWriter writer, boolean doBackup ) throws IOException
+ {
+ requireNonNull( target, "target is null" );
+ requireNonNull( writer, "writer is null" );
+ Path parent = requireNonNull( target.getParent(), "target must be a file" );
+ Path temp = null;
+
+ Files.createDirectories( target.getParent() );
Review Comment:
Why not use `parent` here?
##########
maven-resolver-util/src/main/java/org/eclipse/aether/util/FileUtils.java:
##########
@@ -0,0 +1,112 @@
+package org.eclipse.aether.util;
+
+/*
+ * 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 java.io.IOException;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.nio.file.StandardCopyOption;
+
+import static java.util.Objects.requireNonNull;
+
+/**
+ * A utility class to write files.
+ *
+ * @since TBD
+ */
+public final class FileUtils
+{
+ private FileUtils()
+ {
+ // hide constructor
+ }
+
+ /**
+ * A file writer, that accepts a {@link Path} to write some content to.
+ */
+ @FunctionalInterface
+ public interface FileWriter
+ {
+ void write( Path path ) throws IOException;
+ }
+
+ /**
+ * Writes file without backup.
+ *
+ * @param target that is the target file (must be file, the path must have parent).
+ * @param writer the writer that will accept a {@link Path} to write content to.
+ * @throws IOException if at any step IO problem occurs.
+ */
+ public static void writeFile( Path target, FileWriter writer ) throws IOException
+ {
+ writeFile( target, writer, false );
+ }
+
+ /**
+ * Writes file with backup copy (appends ".bak" extension).
+ *
+ * @param target that is the target file (must be file, the path must have parent).
+ * @param writer the writer that will accept a {@link Path} to write content to.
+ * @throws IOException if at any step IO problem occurs.
+ */
+ public static void writeFileWithBackup( Path target, FileWriter writer ) throws IOException
+ {
+ writeFile( target, writer, true );
+ }
+
+ /**
+ * Utility method to write out file to disk in "atomic" manner, with optional backups (".bak") if needed. This
+ * ensures that no other thread or process will be able to read not fully written files. Finally, this methos
+ * may create the needed parent directories, if the passed in target parents does not exist.
+ *
+ * @param target that is the target file (must be file, the path must have parent).
+ * @param writer the writer that will accept a {@link Path} to write content to.
+ * @param doBackup if {@code true}, and target file is about to be overwritten, a ".bak" file with old contents will
+ * be created/overwritten.
+ * @throws IOException if at any step IO problem occurs.
+ */
+ private static void writeFile( Path target, FileWriter writer, boolean doBackup ) throws IOException
+ {
+ requireNonNull( target, "target is null" );
+ requireNonNull( writer, "writer is null" );
+ Path parent = requireNonNull( target.getParent(), "target must be a file" );
Review Comment:
I don't understand this message
--
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 diff in pull request #203: [MRESOLVER-279] Simplify and improve trusted checksum sources
Posted by GitBox <gi...@apache.org>.
cstamas commented on code in PR #203:
URL: https://github.com/apache/maven-resolver/pull/203#discussion_r1004091275
##########
maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/checksum/SummaryFileTrustedChecksumsSource.java:
##########
@@ -27,45 +28,62 @@
import java.io.UncheckedIOException;
import java.nio.charset.StandardCharsets;
import java.nio.file.Files;
-import java.nio.file.NoSuchFileException;
import java.nio.file.Path;
import java.util.ArrayList;
-import java.util.Collections;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.Objects;
-import java.util.Set;
-import java.util.TreeSet;
import java.util.concurrent.ConcurrentHashMap;
+import java.util.concurrent.atomic.AtomicBoolean;
import org.eclipse.aether.MultiRuntimeException;
import org.eclipse.aether.RepositorySystemSession;
import org.eclipse.aether.artifact.Artifact;
+import org.eclipse.aether.internal.impl.LocalPathComposer;
import org.eclipse.aether.repository.ArtifactRepository;
import org.eclipse.aether.spi.connector.checksum.ChecksumAlgorithmFactory;
-import org.eclipse.aether.util.artifact.ArtifactIdUtils;
+import org.eclipse.aether.util.ConfigUtils;
+import org.eclipse.aether.util.FileUtils;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import static java.util.Objects.requireNonNull;
+import static java.util.stream.Collectors.toList;
/**
* Compact file {@link FileTrustedChecksumsSourceSupport} implementation that use specified directory as base
- * directory, where it expects a "summary" file named as "checksums.${checksumExt}" for each checksum algorithm, and
- * file format is artifact ID and checksum separated by space per line. The format supports comments "#" (hash) and
- * empty lines (both are ignored).
+ * directory, where it expects a "summary" file named as "checksums.${checksumExt}" for each checksum algorithm.
+ * File format is GNU Coreutils compatible: each line holds checksum followed by two spaces and artifact relative path
+ * (from local repository root, without leading "./"). This means that trusted checksums summary file can be used to
+ * validate artifacts or generate it using standard GNU tools like GNU {@code sha1sum} is (for BSD derivatives same
+ * file can be used with {@code -r} switch).
* <p>
- * The source may be configured to be "origin aware", in that case it will factor in origin repository ID as well into
- * file name (for example "checksums-central.sha1").
+ * The format supports comments "#" (hash) and empty lines for easier structuring the file content, and both are
+ * ignored. Also, their presence makes the summary file incompatible with GNU Coreutils format. On save of the
+ * summary file, the comments and empty lines are lost, and file is sorted by path names for easier diffing
+ * (2nd column in file).
* <p>
- * The checksums file once loaded are cached in session, so in-flight file changes during lifecycle of session are NOT
- * noticed.
+ * The source by default is "origin aware", and it will factor in origin repository ID as well into summary file name,
+ * for example "checksums-central.sha256".
+ * <p>
+ * Example commands for managing summary file (in examples will use repository ID "central"):
+ * <ul>
+ * <li>To create summary file: {@code find * -not -name "checksums-central.sha256" -type f -print0 |
+ * xargs -0 sha256sum | sort -k 2 > checksums-central.sha256}</li>
+ * <li>To verify artifacts using summary file: {@code sha256sum --quiet -c checksums-central.sha256}</li>
+ * </ul>
+ * <p>
+ * The checksums summary file is lazily loaded and remains cached in session, so file changes during lifecycle of the
+ * session are not picked up. This implementation can be simultaneously used to lookup and also write checksums. The
+ * written checksums will become visible only for writer session, and newly written checksums, if any, will be flushed
Review Comment:
Ah, yes, previously the file was "resolver specific", recognized ONLY by resolver. But then realized that if I make this file (as javadoc states) "File format is GNU Coreutils compatible", then nothing stops us from:
* generate this file using GN Coreutils commands (shasum and it's brothers) and use with resolver
* generate with resolver and use with GNU Coreutils to validate local repo
* etc, it just vastly improves interoperatbility and compatibility between "maven land" and OS in both directions
The file IS about "physical files" in a sense, that filenames are relative paths resolved from repository root. This makes it portable between workstations (and OSes), while using abs file paths would make it tied to one workstation (tied to local repo location).
--
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] gnodet commented on a diff in pull request #203: [MRESOLVER-279] Simplify and improve trusted checksum sources
Posted by GitBox <gi...@apache.org>.
gnodet commented on code in PR #203:
URL: https://github.com/apache/maven-resolver/pull/203#discussion_r1002547214
##########
maven-resolver-impl/src/test/java/org/eclipse/aether/internal/impl/checksum/FileTrustedChecksumsSourceTestSupport.java:
##########
@@ -51,58 +56,79 @@
private FileTrustedChecksumsSourceSupport subject;
+ private boolean checksumWritten;
+
@Before
public void before() throws Exception
{
session = TestUtils.newSession();
// populate local repository
- Path basedir = session.getLocalRepository().getBasedir().toPath()
- .resolve( FileTrustedChecksumsSourceSupport.LOCAL_REPO_PREFIX_DIR );
checksumAlgorithmFactory = new Sha1ChecksumAlgorithmFactory();
- subject = prepareSubject( basedir );
+ subject = prepareSubject();
+ checksumWritten = false;
+
+ try ( DefaultRepositorySystemSession prepareSession = new DefaultRepositorySystemSession( session ) )
+ {
+ enableSource( prepareSession );
+ try ( TrustedChecksumsSource.Writer writer = subject.getTrustedArtifactChecksumsWriter( prepareSession ) )
+ {
+ if ( writer != null )
+ {
+ HashMap<String, String> checksums = new HashMap<>();
+ checksums.put( checksumAlgorithmFactory.getName(), ARTIFACT_TRUSTED_CHECKSUM );
+ writer.addTrustedArtifactChecksums( ARTIFACT_WITH_CHECKSUM, prepareSession.getLocalRepository(),
+ Collections.singletonList( checksumAlgorithmFactory ), checksums );
+ checksumWritten = true;
+ }
+ }
+ }
}
- protected abstract FileTrustedChecksumsSourceSupport prepareSubject( Path basedir ) throws IOException;
+ protected abstract FileTrustedChecksumsSourceSupport prepareSubject();
- protected abstract void enableSource();
+ protected abstract void enableSource( DefaultRepositorySystemSession session );
Review Comment:
I don't see the `session` parameter used. Did I miss something ?
--
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 diff in pull request #203: [MRESOLVER-279] Simplify and improve trusted checksum sources
Posted by GitBox <gi...@apache.org>.
cstamas commented on code in PR #203:
URL: https://github.com/apache/maven-resolver/pull/203#discussion_r1004085010
##########
maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/checksum/SummaryFileTrustedChecksumsSource.java:
##########
@@ -27,45 +28,62 @@
import java.io.UncheckedIOException;
import java.nio.charset.StandardCharsets;
import java.nio.file.Files;
-import java.nio.file.NoSuchFileException;
import java.nio.file.Path;
import java.util.ArrayList;
-import java.util.Collections;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.Objects;
-import java.util.Set;
-import java.util.TreeSet;
import java.util.concurrent.ConcurrentHashMap;
+import java.util.concurrent.atomic.AtomicBoolean;
import org.eclipse.aether.MultiRuntimeException;
import org.eclipse.aether.RepositorySystemSession;
import org.eclipse.aether.artifact.Artifact;
+import org.eclipse.aether.internal.impl.LocalPathComposer;
import org.eclipse.aether.repository.ArtifactRepository;
import org.eclipse.aether.spi.connector.checksum.ChecksumAlgorithmFactory;
-import org.eclipse.aether.util.artifact.ArtifactIdUtils;
+import org.eclipse.aether.util.ConfigUtils;
+import org.eclipse.aether.util.FileUtils;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import static java.util.Objects.requireNonNull;
+import static java.util.stream.Collectors.toList;
/**
* Compact file {@link FileTrustedChecksumsSourceSupport} implementation that use specified directory as base
- * directory, where it expects a "summary" file named as "checksums.${checksumExt}" for each checksum algorithm, and
- * file format is artifact ID and checksum separated by space per line. The format supports comments "#" (hash) and
- * empty lines (both are ignored).
+ * directory, where it expects a "summary" file named as "checksums.${checksumExt}" for each checksum algorithm.
+ * File format is GNU Coreutils compatible: each line holds checksum followed by two spaces and artifact relative path
+ * (from local repository root, without leading "./"). This means that trusted checksums summary file can be used to
+ * validate artifacts or generate it using standard GNU tools like GNU {@code sha1sum} is (for BSD derivatives same
+ * file can be used with {@code -r} switch).
* <p>
- * The source may be configured to be "origin aware", in that case it will factor in origin repository ID as well into
- * file name (for example "checksums-central.sha1").
+ * The format supports comments "#" (hash) and empty lines for easier structuring the file content, and both are
+ * ignored. Also, their presence makes the summary file incompatible with GNU Coreutils format. On save of the
+ * summary file, the comments and empty lines are lost, and file is sorted by path names for easier diffing
+ * (2nd column in file).
* <p>
- * The checksums file once loaded are cached in session, so in-flight file changes during lifecycle of session are NOT
- * noticed.
+ * The source by default is "origin aware", and it will factor in origin repository ID as well into summary file name,
+ * for example "checksums-central.sha256".
+ * <p>
+ * Example commands for managing summary file (in examples will use repository ID "central"):
+ * <ul>
+ * <li>To create summary file: {@code find * -not -name "checksums-central.sha256" -type f -print0 |
+ * xargs -0 sha256sum | sort -k 2 > checksums-central.sha256}</li>
+ * <li>To verify artifacts using summary file: {@code sha256sum --quiet -c checksums-central.sha256}</li>
+ * </ul>
+ * <p>
+ * The checksums summary file is lazily loaded and remains cached in session, so file changes during lifecycle of the
+ * session are not picked up. This implementation can be simultaneously used to lookup and also write checksums. The
+ * written checksums will become visible only for writer session, and newly written checksums, if any, will be flushed
Review Comment:
As explained above, summary contains a list of file -> checksum for given algorithm (one file is one algorithm). Moreover, the default is "origin aware", so that above is really "list of files -> checksum for given algorith per repository".
Actually am unsure what you ask here, "not per file and not per gav"? What is it then?
--
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 #203: [MRESOLVER-279] Simplify and improve trusted checksum sources
Posted by GitBox <gi...@apache.org>.
cstamas merged PR #203:
URL: https://github.com/apache/maven-resolver/pull/203
--
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 diff in pull request #203: [MRESOLVER-279] Simplify and improve trusted checksum sources
Posted by GitBox <gi...@apache.org>.
cstamas commented on code in PR #203:
URL: https://github.com/apache/maven-resolver/pull/203#discussion_r1004082829
##########
maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/checksum/FileTrustedChecksumsSourceSupport.java:
##########
@@ -151,29 +138,40 @@ protected String configPropKey( String name )
}
/**
- * Returns {@code true} if session configuration contains "originAware" property set to {@code true}.
+ * Returns {@code true} if session configuration marks this instance as enabled.
+ * <p>
+ * Default value is {@code false}.
+ */
+ protected boolean isEnabled( RepositorySystemSession session )
+ {
+ return ConfigUtils.getBoolean( session, false, CONFIG_PROP_PREFIX + this.name );
+ }
+
+ /**
+ * Returns {@code true} if session configuration marks this instance as origin aware.
+ * <p>
+ * Default value is {@code true}.
*/
protected boolean isOriginAware( RepositorySystemSession session )
{
- return ConfigUtils.getBoolean( session, false, configPropKey( CONF_NAME_ORIGIN_AWARE ) );
+ return ConfigUtils.getBoolean( session, true, configPropKey( CONF_NAME_ORIGIN_AWARE ) );
Review Comment:
This has nothing to do with LRM and are they split or not. This way you just say "junit 4.13.2 from central has sha-1 8ac9e16d933b6fb43bc7f576336b8f4d7eb5ba12" and turn on "fail if missing" and you are done. Or in other words, this makes possible to maintain lists of checksums "by origin" that is IMHO simper than "in bulk", where list would be project specific, while this way, those lists are reusable across projects.
Again, this is just a default, and while I do see valid points for both (per-origin and in-bulk), it can be easily overriden,
--
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 diff in pull request #203: [MRESOLVER-279] Simplify and improve trusted checksum sources
Posted by GitBox <gi...@apache.org>.
michael-o commented on code in PR #203:
URL: https://github.com/apache/maven-resolver/pull/203#discussion_r1004085709
##########
maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/checksum/SummaryFileTrustedChecksumsSource.java:
##########
@@ -27,45 +28,62 @@
import java.io.UncheckedIOException;
import java.nio.charset.StandardCharsets;
import java.nio.file.Files;
-import java.nio.file.NoSuchFileException;
import java.nio.file.Path;
import java.util.ArrayList;
-import java.util.Collections;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.Objects;
-import java.util.Set;
-import java.util.TreeSet;
import java.util.concurrent.ConcurrentHashMap;
+import java.util.concurrent.atomic.AtomicBoolean;
import org.eclipse.aether.MultiRuntimeException;
import org.eclipse.aether.RepositorySystemSession;
import org.eclipse.aether.artifact.Artifact;
+import org.eclipse.aether.internal.impl.LocalPathComposer;
import org.eclipse.aether.repository.ArtifactRepository;
import org.eclipse.aether.spi.connector.checksum.ChecksumAlgorithmFactory;
-import org.eclipse.aether.util.artifact.ArtifactIdUtils;
+import org.eclipse.aether.util.ConfigUtils;
+import org.eclipse.aether.util.FileUtils;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import static java.util.Objects.requireNonNull;
+import static java.util.stream.Collectors.toList;
/**
* Compact file {@link FileTrustedChecksumsSourceSupport} implementation that use specified directory as base
- * directory, where it expects a "summary" file named as "checksums.${checksumExt}" for each checksum algorithm, and
- * file format is artifact ID and checksum separated by space per line. The format supports comments "#" (hash) and
- * empty lines (both are ignored).
+ * directory, where it expects a "summary" file named as "checksums.${checksumExt}" for each checksum algorithm.
+ * File format is GNU Coreutils compatible: each line holds checksum followed by two spaces and artifact relative path
+ * (from local repository root, without leading "./"). This means that trusted checksums summary file can be used to
+ * validate artifacts or generate it using standard GNU tools like GNU {@code sha1sum} is (for BSD derivatives same
+ * file can be used with {@code -r} switch).
* <p>
- * The source may be configured to be "origin aware", in that case it will factor in origin repository ID as well into
- * file name (for example "checksums-central.sha1").
+ * The format supports comments "#" (hash) and empty lines for easier structuring the file content, and both are
+ * ignored. Also, their presence makes the summary file incompatible with GNU Coreutils format. On save of the
+ * summary file, the comments and empty lines are lost, and file is sorted by path names for easier diffing
+ * (2nd column in file).
* <p>
- * The checksums file once loaded are cached in session, so in-flight file changes during lifecycle of session are NOT
- * noticed.
+ * The source by default is "origin aware", and it will factor in origin repository ID as well into summary file name,
+ * for example "checksums-central.sha256".
+ * <p>
+ * Example commands for managing summary file (in examples will use repository ID "central"):
+ * <ul>
+ * <li>To create summary file: {@code find * -not -name "checksums-central.sha256" -type f -print0 |
+ * xargs -0 sha256sum | sort -k 2 > checksums-central.sha256}</li>
+ * <li>To verify artifacts using summary file: {@code sha256sum --quiet -c checksums-central.sha256}</li>
+ * </ul>
+ * <p>
+ * The checksums summary file is lazily loaded and remains cached in session, so file changes during lifecycle of the
+ * session are not picked up. This implementation can be simultaneously used to lookup and also write checksums. The
+ * written checksums will become visible only for writer session, and newly written checksums, if any, will be flushed
Review Comment:
Well, previously it contained coordinates. Just wanted to understand the change. No more, no less.
--
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 diff in pull request #203: [MRESOLVER-279] Simplify and improve trusted checksum sources
Posted by GitBox <gi...@apache.org>.
cstamas commented on code in PR #203:
URL: https://github.com/apache/maven-resolver/pull/203#discussion_r1004108378
##########
maven-resolver-impl/src/test/java/org/eclipse/aether/internal/impl/checksum/FileTrustedChecksumsSourceTestSupport.java:
##########
@@ -51,58 +56,79 @@
private FileTrustedChecksumsSourceSupport subject;
+ private boolean checksumWritten;
+
@Before
public void before() throws Exception
{
session = TestUtils.newSession();
// populate local repository
- Path basedir = session.getLocalRepository().getBasedir().toPath()
- .resolve( FileTrustedChecksumsSourceSupport.LOCAL_REPO_PREFIX_DIR );
checksumAlgorithmFactory = new Sha1ChecksumAlgorithmFactory();
- subject = prepareSubject( basedir );
+ subject = prepareSubject();
+ checksumWritten = false;
+
+ try ( DefaultRepositorySystemSession prepareSession = new DefaultRepositorySystemSession( session ) )
+ {
+ enableSource( prepareSession );
+ try ( TrustedChecksumsSource.Writer writer = subject.getTrustedArtifactChecksumsWriter( prepareSession ) )
+ {
+ if ( writer != null )
+ {
+ HashMap<String, String> checksums = new HashMap<>();
+ checksums.put( checksumAlgorithmFactory.getName(), ARTIFACT_TRUSTED_CHECKSUM );
+ writer.addTrustedArtifactChecksums( ARTIFACT_WITH_CHECKSUM, prepareSession.getLocalRepository(),
+ Collections.singletonList( checksumAlgorithmFactory ), checksums );
+ checksumWritten = true;
+ }
+ }
+ }
}
- protected abstract FileTrustedChecksumsSourceSupport prepareSubject( Path basedir ) throws IOException;
+ protected abstract FileTrustedChecksumsSourceSupport prepareSubject();
- protected abstract void enableSource();
+ protected abstract void enableSource( DefaultRepositorySystemSession session );
Review Comment:
see c87798a898f45d1d4998b5c9146fa0f2fa06c3cd
--
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