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 2023/01/05 18:30:49 UTC
[GitHub] [maven-resolver] cstamas opened a new pull request, #234: [MRESOLVER-313] Wrong FS permissions on installed artifacts.
cstamas opened a new pull request, #234:
URL: https://github.com/apache/maven-resolver/pull/234
Collocated temp file should NOT use Files.createTempFile as it uses 0600 perms and not relies implicitly on umask.
---
https://issues.apache.org/jira/browse/MRESOLVER-313
--
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 #234: [MRESOLVER-313] Wrong FS permissions on cached artifacts.
Posted by GitBox <gi...@apache.org>.
cstamas merged PR #234:
URL: https://github.com/apache/maven-resolver/pull/234
--
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 #234: [MRESOLVER-313] Wrong FS permissions on installed artifacts.
Posted by GitBox <gi...@apache.org>.
cstamas commented on code in PR #234:
URL: https://github.com/apache/maven-resolver/pull/234#discussion_r1063280658
##########
maven-resolver-util/src/main/java/org/eclipse/aether/util/FileUtils.java:
##########
@@ -85,17 +92,23 @@ public void close() throws IOException
}
/**
- * Creates a {@link TempFile} for given file. It will be in same directory where given file is, and will reuse its
- * name for generated name. Returned instance should be handled in try-with-resource construct and created temp
- * file once ready can be moved to passed in {@code file} parameter place.
+ * Creates a {@link CollocatedTempFile} instance for given file without backing file. The path will be located in
+ * same directory where given file is, and will reuse its name for generated (randomized) name. Returned instance
+ * should be handled in try-with-resource and created temp path is removed (if exists) when returned instance is
+ * closed. The {@link CollocatedTempFile#move()} makes possible to atomically replace passed in file with the
+ * processed content written into a file backing the {@link CollocatedTempFile} instance.
* <p>
* The {@code file} nor it's parent directories have to exist. The parent directories are created if needed.
+ * <p>
+ * This method uses {@link Path#resolve(String)} to create the temporary file path in passed in file parent
+ * directory, but it does NOT create backing file on file system.
*/
public static CollocatedTempFile newTempFile( Path file ) throws IOException
{
Path parent = requireNonNull( file.getParent(), "file must have parent" );
Files.createDirectories( parent );
- Path tempFile = Files.createTempFile( parent, file.getFileName().toString(), "tmp" );
+ Path tempFile = parent.resolve(
+ file.getFileName().toString() + Long.toUnsignedString( RANDOM.nextLong() ) + ".tmp" );
Review Comment:
Because out of the box Maven does NOT provides inter process sync context, just local to Java. Hence I wanted to retain all the semantics this method had from before (latest wins), while keep all the promises as well (no partial files)..
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
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 #234: [MRESOLVER-313] Wrong FS permissions on cached artifacts.
Posted by GitBox <gi...@apache.org>.
michael-o commented on PR #234:
URL: https://github.com/apache/maven-resolver/pull/234#issuecomment-1373621017
> @michael-o please look at last commit (and it's message)
Makes sense, expect for the typo on the commit: its final place
--
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 #234: [MRESOLVER-313] Wrong FS permissions on cached artifacts.
Posted by GitBox <gi...@apache.org>.
gnodet commented on code in PR #234:
URL: https://github.com/apache/maven-resolver/pull/234#discussion_r1063451155
##########
maven-resolver-util/src/main/java/org/eclipse/aether/util/FileUtils.java:
##########
@@ -85,17 +92,23 @@ public void close() throws IOException
}
/**
- * Creates a {@link TempFile} for given file. It will be in same directory where given file is, and will reuse its
- * name for generated name. Returned instance should be handled in try-with-resource construct and created temp
- * file once ready can be moved to passed in {@code file} parameter place.
+ * Creates a {@link CollocatedTempFile} instance for given file without backing file. The path will be located in
+ * same directory where given file is, and will reuse its name for generated (randomized) name. Returned instance
+ * should be handled in try-with-resource and created temp path is removed (if exists) when returned instance is
+ * closed. The {@link CollocatedTempFile#move()} makes possible to atomically replace passed in file with the
+ * processed content written into a file backing the {@link CollocatedTempFile} instance.
* <p>
* The {@code file} nor it's parent directories have to exist. The parent directories are created if needed.
+ * <p>
+ * This method uses {@link Path#resolve(String)} to create the temporary file path in passed in file parent
+ * directory, but it does NOT create backing file on file system.
*/
public static CollocatedTempFile newTempFile( Path file ) throws IOException
{
Path parent = requireNonNull( file.getParent(), "file must have parent" );
Files.createDirectories( parent );
- Path tempFile = Files.createTempFile( parent, file.getFileName().toString(), "tmp" );
+ Path tempFile = parent.resolve(
+ file.getFileName().toString() + Long.toUnsignedString( RANDOM.nextLong() ) + ".tmp" );
Review Comment:
Use `ThreadLocalRandom.current().nextLong()` instead
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
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 #234: [MRESOLVER-313] Wrong FS permissions on cached artifacts.
Posted by GitBox <gi...@apache.org>.
cstamas commented on code in PR #234:
URL: https://github.com/apache/maven-resolver/pull/234#discussion_r1064409456
##########
maven-resolver-util/src/main/java/org/eclipse/aether/util/FileUtils.java:
##########
@@ -85,17 +92,23 @@ public void close() throws IOException
}
/**
- * Creates a {@link TempFile} for given file. It will be in same directory where given file is, and will reuse its
- * name for generated name. Returned instance should be handled in try-with-resource construct and created temp
- * file once ready can be moved to passed in {@code file} parameter place.
+ * Creates a {@link CollocatedTempFile} instance for given file without backing file. The path will be located in
+ * same directory where given file is, and will reuse its name for generated (randomized) name. Returned instance
+ * should be handled in try-with-resource and created temp path is removed (if exists) when returned instance is
+ * closed. The {@link CollocatedTempFile#move()} makes possible to atomically replace passed in file with the
+ * processed content written into a file backing the {@link CollocatedTempFile} instance.
* <p>
* The {@code file} nor it's parent directories have to exist. The parent directories are created if needed.
+ * <p>
+ * This method uses {@link Path#resolve(String)} to create the temporary file path in passed in file parent
+ * directory, but it does NOT create backing file on file system.
*/
public static CollocatedTempFile newTempFile( Path file ) throws IOException
{
Path parent = requireNonNull( file.getParent(), "file must have parent" );
Files.createDirectories( parent );
- Path tempFile = Files.createTempFile( parent, file.getFileName().toString(), "tmp" );
+ Path tempFile = parent.resolve(
+ file.getFileName().toString() + Long.toUnsignedString( RANDOM.nextLong() ) + ".tmp" );
Review Comment:
done
--
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 #234: [MRESOLVER-313] Wrong FS permissions on cached artifacts.
Posted by GitBox <gi...@apache.org>.
gnodet commented on code in PR #234:
URL: https://github.com/apache/maven-resolver/pull/234#discussion_r1063452977
##########
maven-resolver-util/src/main/java/org/eclipse/aether/util/FileUtils.java:
##########
@@ -85,17 +92,23 @@ public void close() throws IOException
}
/**
- * Creates a {@link TempFile} for given file. It will be in same directory where given file is, and will reuse its
- * name for generated name. Returned instance should be handled in try-with-resource construct and created temp
- * file once ready can be moved to passed in {@code file} parameter place.
+ * Creates a {@link CollocatedTempFile} instance for given file without backing file. The path will be located in
+ * same directory where given file is, and will reuse its name for generated (randomized) name. Returned instance
+ * should be handled in try-with-resource and created temp path is removed (if exists) when returned instance is
+ * closed. The {@link CollocatedTempFile#move()} makes possible to atomically replace passed in file with the
+ * processed content written into a file backing the {@link CollocatedTempFile} instance.
* <p>
* The {@code file} nor it's parent directories have to exist. The parent directories are created if needed.
+ * <p>
+ * This method uses {@link Path#resolve(String)} to create the temporary file path in passed in file parent
+ * directory, but it does NOT create backing file on file system.
*/
public static CollocatedTempFile newTempFile( Path file ) throws IOException
{
Path parent = requireNonNull( file.getParent(), "file must have parent" );
Files.createDirectories( parent );
- Path tempFile = Files.createTempFile( parent, file.getFileName().toString(), "tmp" );
+ Path tempFile = parent.resolve(
+ file.getFileName() + "." + Long.toUnsignedString( RANDOM.nextLong() ) + ".tmp" );
Review Comment:
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 pull request #234: [MRESOLVER-313] Wrong FS permissions on installed artifacts.
Posted by GitBox <gi...@apache.org>.
cstamas commented on PR #234:
URL: https://github.com/apache/maven-resolver/pull/234#issuecomment-1373426642
@michael-o please look at last commit (and it's 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] michael-o commented on a diff in pull request #234: [MRESOLVER-313] Wrong FS permissions on installed artifacts.
Posted by GitBox <gi...@apache.org>.
michael-o commented on code in PR #234:
URL: https://github.com/apache/maven-resolver/pull/234#discussion_r1063228624
##########
maven-resolver-util/src/main/java/org/eclipse/aether/util/FileUtils.java:
##########
@@ -85,17 +92,23 @@ public void close() throws IOException
}
/**
- * Creates a {@link TempFile} for given file. It will be in same directory where given file is, and will reuse its
- * name for generated name. Returned instance should be handled in try-with-resource construct and created temp
- * file once ready can be moved to passed in {@code file} parameter place.
+ * Creates a {@link CollocatedTempFile} instance for given file without backing file. The path will be located in
+ * same directory where given file is, and will reuse its name for generated (randomized) name. Returned instance
+ * should be handled in try-with-resource and created temp path is removed (if exists) when returned instance is
+ * closed. The {@link CollocatedTempFile#move()} makes possible to atomically replace passed in file with the
+ * processed content written into a file backing the {@link CollocatedTempFile} instance.
* <p>
* The {@code file} nor it's parent directories have to exist. The parent directories are created if needed.
+ * <p>
+ * This method uses {@link Path#resolve(String)} to create the temporary file path in passed in file parent
+ * directory, but it does NOT create backing file on file system.
*/
public static CollocatedTempFile newTempFile( Path file ) throws IOException
{
Path parent = requireNonNull( file.getParent(), "file must have parent" );
Files.createDirectories( parent );
- Path tempFile = Files.createTempFile( parent, file.getFileName().toString(), "tmp" );
+ Path tempFile = parent.resolve(
+ file.getFileName().toString() + Long.toUnsignedString( RANDOM.nextLong() ) + ".tmp" );
Review Comment:
Why is the random part necessary at all? All downloads happen in a sync context. E.g., Firefox create a non-random temp file during download and then moves to the target as well.
--
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 #234: [MRESOLVER-313] Wrong FS permissions on installed artifacts.
Posted by GitBox <gi...@apache.org>.
michael-o commented on code in PR #234:
URL: https://github.com/apache/maven-resolver/pull/234#discussion_r1063283610
##########
maven-resolver-util/src/main/java/org/eclipse/aether/util/FileUtils.java:
##########
@@ -85,17 +92,23 @@ public void close() throws IOException
}
/**
- * Creates a {@link TempFile} for given file. It will be in same directory where given file is, and will reuse its
- * name for generated name. Returned instance should be handled in try-with-resource construct and created temp
- * file once ready can be moved to passed in {@code file} parameter place.
+ * Creates a {@link CollocatedTempFile} instance for given file without backing file. The path will be located in
+ * same directory where given file is, and will reuse its name for generated (randomized) name. Returned instance
+ * should be handled in try-with-resource and created temp path is removed (if exists) when returned instance is
+ * closed. The {@link CollocatedTempFile#move()} makes possible to atomically replace passed in file with the
+ * processed content written into a file backing the {@link CollocatedTempFile} instance.
* <p>
* The {@code file} nor it's parent directories have to exist. The parent directories are created if needed.
+ * <p>
+ * This method uses {@link Path#resolve(String)} to create the temporary file path in passed in file parent
+ * directory, but it does NOT create backing file on file system.
*/
public static CollocatedTempFile newTempFile( Path file ) throws IOException
{
Path parent = requireNonNull( file.getParent(), "file must have parent" );
Files.createDirectories( parent );
- Path tempFile = Files.createTempFile( parent, file.getFileName().toString(), "tmp" );
+ Path tempFile = parent.resolve(
+ file.getFileName().toString() + Long.toUnsignedString( RANDOM.nextLong() ) + ".tmp" );
Review Comment:
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 diff in pull request #234: [MRESOLVER-313] Wrong FS permissions on installed artifacts.
Posted by GitBox <gi...@apache.org>.
cstamas commented on code in PR #234:
URL: https://github.com/apache/maven-resolver/pull/234#discussion_r1063280658
##########
maven-resolver-util/src/main/java/org/eclipse/aether/util/FileUtils.java:
##########
@@ -85,17 +92,23 @@ public void close() throws IOException
}
/**
- * Creates a {@link TempFile} for given file. It will be in same directory where given file is, and will reuse its
- * name for generated name. Returned instance should be handled in try-with-resource construct and created temp
- * file once ready can be moved to passed in {@code file} parameter place.
+ * Creates a {@link CollocatedTempFile} instance for given file without backing file. The path will be located in
+ * same directory where given file is, and will reuse its name for generated (randomized) name. Returned instance
+ * should be handled in try-with-resource and created temp path is removed (if exists) when returned instance is
+ * closed. The {@link CollocatedTempFile#move()} makes possible to atomically replace passed in file with the
+ * processed content written into a file backing the {@link CollocatedTempFile} instance.
* <p>
* The {@code file} nor it's parent directories have to exist. The parent directories are created if needed.
+ * <p>
+ * This method uses {@link Path#resolve(String)} to create the temporary file path in passed in file parent
+ * directory, but it does NOT create backing file on file system.
*/
public static CollocatedTempFile newTempFile( Path file ) throws IOException
{
Path parent = requireNonNull( file.getParent(), "file must have parent" );
Files.createDirectories( parent );
- Path tempFile = Files.createTempFile( parent, file.getFileName().toString(), "tmp" );
+ Path tempFile = parent.resolve(
+ file.getFileName().toString() + Long.toUnsignedString( RANDOM.nextLong() ) + ".tmp" );
Review Comment:
Because out of the box Maven does NOT provides inter process sync context, just local to JVM. Hence I wanted to retain all the semantics this method had from before (latest wins), while keep all the promises as well (no partial files)..
Anyway, the random name does not matter at all, if you use sync context, does not breaks anything, as functionality remains the same.
--
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 #234: [MRESOLVER-313] Wrong FS permissions on installed artifacts.
Posted by GitBox <gi...@apache.org>.
cstamas commented on code in PR #234:
URL: https://github.com/apache/maven-resolver/pull/234#discussion_r1063289876
##########
maven-resolver-util/src/main/java/org/eclipse/aether/util/FileUtils.java:
##########
@@ -85,17 +92,23 @@ public void close() throws IOException
}
/**
- * Creates a {@link TempFile} for given file. It will be in same directory where given file is, and will reuse its
- * name for generated name. Returned instance should be handled in try-with-resource construct and created temp
- * file once ready can be moved to passed in {@code file} parameter place.
+ * Creates a {@link CollocatedTempFile} instance for given file without backing file. The path will be located in
+ * same directory where given file is, and will reuse its name for generated (randomized) name. Returned instance
+ * should be handled in try-with-resource and created temp path is removed (if exists) when returned instance is
+ * closed. The {@link CollocatedTempFile#move()} makes possible to atomically replace passed in file with the
+ * processed content written into a file backing the {@link CollocatedTempFile} instance.
* <p>
* The {@code file} nor it's parent directories have to exist. The parent directories are created if needed.
+ * <p>
+ * This method uses {@link Path#resolve(String)} to create the temporary file path in passed in file parent
+ * directory, but it does NOT create backing file on file system.
*/
public static CollocatedTempFile newTempFile( Path file ) throws IOException
{
Path parent = requireNonNull( file.getParent(), "file must have parent" );
Files.createDirectories( parent );
- Path tempFile = Files.createTempFile( parent, file.getFileName().toString(), "tmp" );
+ Path tempFile = parent.resolve(
+ file.getFileName().toString() + Long.toUnsignedString( RANDOM.nextLong() ) + ".tmp" );
Review Comment:
done
--
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 #234: [MRESOLVER-313] Wrong FS permissions on installed artifacts.
Posted by GitBox <gi...@apache.org>.
cstamas commented on code in PR #234:
URL: https://github.com/apache/maven-resolver/pull/234#discussion_r1063280658
##########
maven-resolver-util/src/main/java/org/eclipse/aether/util/FileUtils.java:
##########
@@ -85,17 +92,23 @@ public void close() throws IOException
}
/**
- * Creates a {@link TempFile} for given file. It will be in same directory where given file is, and will reuse its
- * name for generated name. Returned instance should be handled in try-with-resource construct and created temp
- * file once ready can be moved to passed in {@code file} parameter place.
+ * Creates a {@link CollocatedTempFile} instance for given file without backing file. The path will be located in
+ * same directory where given file is, and will reuse its name for generated (randomized) name. Returned instance
+ * should be handled in try-with-resource and created temp path is removed (if exists) when returned instance is
+ * closed. The {@link CollocatedTempFile#move()} makes possible to atomically replace passed in file with the
+ * processed content written into a file backing the {@link CollocatedTempFile} instance.
* <p>
* The {@code file} nor it's parent directories have to exist. The parent directories are created if needed.
+ * <p>
+ * This method uses {@link Path#resolve(String)} to create the temporary file path in passed in file parent
+ * directory, but it does NOT create backing file on file system.
*/
public static CollocatedTempFile newTempFile( Path file ) throws IOException
{
Path parent = requireNonNull( file.getParent(), "file must have parent" );
Files.createDirectories( parent );
- Path tempFile = Files.createTempFile( parent, file.getFileName().toString(), "tmp" );
+ Path tempFile = parent.resolve(
+ file.getFileName().toString() + Long.toUnsignedString( RANDOM.nextLong() ) + ".tmp" );
Review Comment:
Because out of the box Maven does NOT provides inter process sync context, just local to JVM. Hence I wanted to retain all the semantics this method had from before (latest wins), while keep all the promises as well (no partial files)..
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
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 #234: [MRESOLVER-313] Wrong FS permissions on installed artifacts.
Posted by GitBox <gi...@apache.org>.
michael-o commented on code in PR #234:
URL: https://github.com/apache/maven-resolver/pull/234#discussion_r1063286536
##########
maven-resolver-util/src/main/java/org/eclipse/aether/util/FileUtils.java:
##########
@@ -85,17 +92,23 @@ public void close() throws IOException
}
/**
- * Creates a {@link TempFile} for given file. It will be in same directory where given file is, and will reuse its
- * name for generated name. Returned instance should be handled in try-with-resource construct and created temp
- * file once ready can be moved to passed in {@code file} parameter place.
+ * Creates a {@link CollocatedTempFile} instance for given file without backing file. The path will be located in
+ * same directory where given file is, and will reuse its name for generated (randomized) name. Returned instance
+ * should be handled in try-with-resource and created temp path is removed (if exists) when returned instance is
+ * closed. The {@link CollocatedTempFile#move()} makes possible to atomically replace passed in file with the
+ * processed content written into a file backing the {@link CollocatedTempFile} instance.
* <p>
* The {@code file} nor it's parent directories have to exist. The parent directories are created if needed.
+ * <p>
+ * This method uses {@link Path#resolve(String)} to create the temporary file path in passed in file parent
+ * directory, but it does NOT create backing file on file system.
*/
public static CollocatedTempFile newTempFile( Path file ) throws IOException
{
Path parent = requireNonNull( file.getParent(), "file must have parent" );
Files.createDirectories( parent );
- Path tempFile = Files.createTempFile( parent, file.getFileName().toString(), "tmp" );
+ Path tempFile = parent.resolve(
+ file.getFileName().toString() + Long.toUnsignedString( RANDOM.nextLong() ) + ".tmp" );
Review Comment:
ToString I'd redundant now. Add a dot between filename and long.
--
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