[GitHub] [commons-vfs] MaxKellermann commented on pull request #154: Rework SoftRefFilesCache locking
MaxKellermann commented on pull request #154: URL: https://github.com/apache/commons-vfs/pull/154#issuecomment-774619672 > Loom will have to find a solution for it, anyway. I agree with that. I read that OpenJDK link, and my first thought was: this is a bad implementation if it makes `synchronized` so special that it blocks context switches. There's no reason why it would need to, that's just an (unoptimized) implementation detail. If this Loom thing forces everybody to rewrite their code, replacing a simple thing as `synchronized` with more complicated, error-prone code, it's bad. But I can't decide what you want to support, it's not my project 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [commons-release-plugin] arturobernalg commented on pull request #34: Minor Improvement:
arturobernalg commented on pull request #34: URL: https://github.com/apache/commons-release-plugin/pull/34#issuecomment-774615003 > Hi @arturobernalg > Please rebase on master and let's see of we can get a green build. 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [commons-release-plugin] coveralls commented on pull request #34: Minor Improvement:
coveralls commented on pull request #34: URL: https://github.com/apache/commons-release-plugin/pull/34#issuecomment-774614957 [](https://coveralls.io/builds/36925741) Coverage remained the same at 74.937% when pulling **b5b0e0cf3828a29a6290b78a12d814c86f7b883e on arturobernalg:feature/minor_improvement** into **b176601de87bfd2a2a04a08868081787be11b122 on apache:master**. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [commons-vfs] ecki commented on pull request #154: Rework SoftRefFilesCache locking
ecki commented on pull request #154: URL: https://github.com/apache/commons-vfs/pull/154#issuecomment-774588145 Personally I don’t care much for this rework, synchronized blocks are much easier to read and Loom will have to find a solution for it, anyway. There are better reasons, like having finer control and try and statistics, but I don’t think this applies here. What I will think is much more important is to,actually define the concurrency model and document it This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[jira] [Updated] (IMAGING-281) Simple Exif XPTitle corrupted.
[
https://issues.apache.org/jira/browse/IMAGING-281?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Garret Wilson updated IMAGING-281:
--
Description:
I have a small input JPEG image containing _no metadata sections whatsoever_. I
use Apache Commons Imaging 1.0-alpha2 to add two simple Exif {{IFD0}}
properties using
[{{ExifRewriter().updateExifMetadataLossy()}}|https://commons.apache.org/proper/commons-imaging/apidocs/org/apache/commons/imaging/formats/jpeg/exif/ExifRewriter.html#updateExifMetadataLossy-org.apache.commons.imaging.common.bytesource.ByteSource-java.io.OutputStream-org.apache.commons.imaging.formats.tiff.write.TiffOutputSet-].
* {{XPTitle}} ({{0x9C9B}}): "Gate and Turret"
* {{Copyright}} ({{33432}}, {{0x8298}}): "Copyright © 2009 Garret Wilson"
Here is a simplified excerpt of the code:
{code:java}
TagInfoAscii EXIF_XP_TITLE_TAG_INFO = new TagInfoAscii("XPTitle", 0x9C9B, -1,
TiffDirectoryType.EXIF_DIRECTORY_IFD0); //XPTitle (0x9C9B)
TagInfoAscii EXIF_COPYRIGHT_TAG_INFO = new TagInfoAscii("Copyright", 0x8298,
-1, TiffDirectoryType.EXIF_DIRECTORY_IFD0); //Copyright (33432, 0x8298)
…
TiffOutputSet tiffOutputSet = new TiffOutputSet();
TiffOutputDirectory exifDirectory = tiffOutputSet.getOrCreateRootDirectory();
exifDirectory.add(EXIF_XP_TITLE_TAG_INFO, "Gate and Turret");
exifDirectory.add(EXIF_COPYRIGHT_TAG_INFO, "Copyright © 2009 Garret Wilson");
…
new ExifRewriter().updateExifMetadataLossy(byteSource, outputStream,
tiffOutputSet);
{code}
Using [ExifTool|https://exiftool.org/] 12.16 (via
[ExifToolGUI|https://exiftool.org/gui/) 5.16.0.0], the {{Copyright}} value is
stored correctly but the {{XPTitle}} is stored as "慇整愠摮吠牵敲t".
[Metadata++|https://www.logipole.com/metadata++-en.htm] 1.22.14 also shows the
same corrupted value.
This is disheartening, as this is nearly the most simple test case possible.
(Note that [IrfanView|https://www.irfanview.com/] 4.54 can read the {{XPTitle}}
just fine! Nevertheless ExifTool is the gold standard for image metadata
reading, and is confirmed by Metadata++. Having an image the metadata of which
cannot be read in ExifTool is a show-stopper.)
I'm will attach the test case image to this ticket.
was:
I have a small input JPEG image containing _no metadata sections whatsoever_. I
use Apache Commons Imaging 1.0-alpha2 to add two simple Exif {{IFD0}}
properties using
[{{ExifRewriter().updateExifMetadataLossy()}}|https://commons.apache.org/proper/commons-imaging/apidocs/org/apache/commons/imaging/formats/jpeg/exif/ExifRewriter.html#updateExifMetadataLossy-org.apache.commons.imaging.common.bytesource.ByteSource-java.io.OutputStream-org.apache.commons.imaging.formats.tiff.write.TiffOutputSet-].
* {{XPTitle}} ({{0x9C9B}}): "Gate and Turret"
* {{Copyright}} ({{33432}}, {{0x8298}}): "Copyright © 2009 Garret Wilson"
Here is a simplified excerpt of the code:
{code:java}
TagInfoAscii EXIF_XP_TITLE_TAG_INFO = new TagInfoAscii("XPTitle", 0x9C9B, -1,
TiffDirectoryType.EXIF_DIRECTORY_IFD0); //XPTitle (0x9C9B)
TagInfoAscii EXIF_COPYRIGHT_TAG_INFO = new TagInfoAscii("Copyright", 0x8298,
-1, TiffDirectoryType.EXIF_DIRECTORY_IFD0); //Copyright (33432, 0x8298)
…
TiffOutputSet tiffOutputSet = new TiffOutputSet();
TiffOutputDirectory exifDirectory = tiffOutputSet.getOrCreateRootDirectory();
exifDirectory.add(EXIF_XP_TITLE_TAG_INFO, "Gate and Turret");
exifDirectory.add(EXIF_COPYRIGHT_TAG_INFO, "Copyright © 2009 Garret Wilson");
…
new ExifRewriter().updateExifMetadataLossy(byteSource, outputStream,
tiffOutputSet);
{code}
Using [ExifTool|https://exiftool.org/] 12.16 (via
[ExifToolGUI|https://exiftool.org/gui/) 5.16.0.0], the {{Copyright}} value is
stored correctly but the {{XPTitle}} is stored as "慇整愠摮吠牵敲t".
[Metadata++|https://www.logipole.com/metadata++-en.htm] 1.22.14 also shows the
same corrupted value.
This is disheartening, as this is nearly the most simple test case possible.
(Note that [IrfanView|https://www.irfanview.com/] 4.54 can read the {{XPTitle}}
just fine! Nevertheless ExifTool is the gold standard for image metadata
reading, and is confirmed by Metadata++. Having an image that cannot be read in
ExifTool is a show-stopper.)
I'm will attach the test case image to this ticket.
> Simple Exif XPTitle corrupted.
> --
>
> Key: IMAGING-281
> URL: https://issues.apache.org/jira/browse/IMAGING-281
> Project: Commons Imaging
> Issue Type: Bug
>Affects Versions: 1.0-alpha2
>Reporter: Garret Wilson
>Priority: Blocker
> Attachments: gate-turret-exif-bad-title.jpg
>
>
> I have a small input JPEG image containing _no metadata sections whatsoever_.
> I use Apache Commons Imaging 1.0-alpha2 to add two simple Exif {{IFD0}}
> properties using
>
[jira] [Updated] (IMAGING-281) Simple Exif XPTitle corrupted.
[
https://issues.apache.org/jira/browse/IMAGING-281?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Garret Wilson updated IMAGING-281:
--
Description:
I have a small input JPEG image containing _no metadata sections whatsoever_. I
use Apache Commons Imaging 1.0-alpha2 to add two simple Exif {{IFD0}}
properties using
[{{ExifRewriter().updateExifMetadataLossy()}}|https://commons.apache.org/proper/commons-imaging/apidocs/org/apache/commons/imaging/formats/jpeg/exif/ExifRewriter.html#updateExifMetadataLossy-org.apache.commons.imaging.common.bytesource.ByteSource-java.io.OutputStream-org.apache.commons.imaging.formats.tiff.write.TiffOutputSet-].
* {{XPTitle}} ({{0x9C9B}}): "Gate and Turret"
* {{Copyright}} ({{33432}}, {{0x8298}}): "Copyright © 2009 Garret Wilson"
Here is a simplified excerpt of the code:
{code:java}
TagInfoAscii EXIF_XP_TITLE_TAG_INFO = new TagInfoAscii("XPTitle", 0x9C9B, -1,
TiffDirectoryType.EXIF_DIRECTORY_IFD0); //XPTitle (0x9C9B)
TagInfoAscii EXIF_COPYRIGHT_TAG_INFO = new TagInfoAscii("Copyright", 0x8298,
-1, TiffDirectoryType.EXIF_DIRECTORY_IFD0); //Copyright (33432, 0x8298)
…
TiffOutputSet tiffOutputSet = new TiffOutputSet();
TiffOutputDirectory exifDirectory = tiffOutputSet.getOrCreateRootDirectory();
exifDirectory.add(EXIF_XP_TITLE_TAG_INFO, "Gate and Turret");
exifDirectory.add(EXIF_COPYRIGHT_TAG_INFO, "Copyright © 2009 Garret Wilson");
…
new ExifRewriter().updateExifMetadataLossy(byteSource, outputStream,
tiffOutputSet);
{code}
Using [ExifTool|https://exiftool.org/] 12.16 (via
[ExifToolGUI|https://exiftool.org/gui/) 5.16.0.0], the {{Copyright}} value is
stored correctly but the {{XPTitle}} is stored as "慇整愠摮吠牵敲t".
[Metadata++|https://www.logipole.com/metadata++-en.htm] 1.22.14 also shows the
same corrupted value.
This is disheartening, as this is nearly the most simple test case possible.
(Note that [IrfanView|https://www.irfanview.com/] 4.54 can read the {{XPTitle}}
just fine! Nevertheless ExifTool is the gold standard for image metadata
reading, and is confirmed by Metadata++. Having an image that cannot be read in
ExifTool is a show-stopper.)
I'm will attach the test case image to this ticket.
was:
I have a small input JPEG image containing _no metadata sections whatsoever_. I
use Apache Commons Imaging 1.0-alpha2 to add two simple Exif {{IFD0}}
properties using
[`ExifRewriter().updateExifMetadataLossy()`|https://commons.apache.org/proper/commons-imaging/apidocs/org/apache/commons/imaging/formats/jpeg/exif/ExifRewriter.html#updateExifMetadataLossy-org.apache.commons.imaging.common.bytesource.ByteSource-java.io.OutputStream-org.apache.commons.imaging.formats.tiff.write.TiffOutputSet-].
* {{XPTitle}} ({{0x9C9B}}): "Gate and Turret"
* {{Copyright}} ({{33432}}, {{0x8298}}): "Copyright © 2009 Garret Wilson"
Here is a simplified excerpt of the code:
{code:java}
TagInfoAscii EXIF_XP_TITLE_TAG_INFO = new TagInfoAscii("XPTitle", 0x9C9B, -1,
TiffDirectoryType.EXIF_DIRECTORY_IFD0); //XPTitle (0x9C9B)
TagInfoAscii EXIF_COPYRIGHT_TAG_INFO = new TagInfoAscii("Copyright", 0x8298,
-1, TiffDirectoryType.EXIF_DIRECTORY_IFD0); //Copyright (33432, 0x8298)
…
TiffOutputSet tiffOutputSet = new TiffOutputSet();
TiffOutputDirectory exifDirectory = tiffOutputSet.getOrCreateRootDirectory();
exifDirectory.add(EXIF_XP_TITLE_TAG_INFO, "Gate and Turret");
exifDirectory.add(EXIF_COPYRIGHT_TAG_INFO, "Copyright © 2009 Garret Wilson");
…
new ExifRewriter().updateExifMetadataLossy(byteSource, outputStream,
tiffOutputSet);
{code}
Using [ExifTool|https://exiftool.org/] 12.16 (via
[ExifToolGUI|https://exiftool.org/gui/) 5.16.0.0], the {{Copyright}} value is
stored correctly but the {{XPTitle}} is stored as "慇整愠摮吠牵敲t".
[Metadata++|https://www.logipole.com/metadata++-en.htm] 1.22.14 also shows the
same corrupted value.
This is disheartening, as this is nearly the most simple test case possible.
(Note that [IrfanView|https://www.irfanview.com/] 4.54 can read the {{XPTitle}}
just fine! Nevertheless ExifTool is the gold standard for image metadata
reading, and is confirmed by Metadata++. Having an image that cannot be read in
ExifTool is a show-stopper.)
I'm will attach the test case image to this ticket.
> Simple Exif XPTitle corrupted.
> --
>
> Key: IMAGING-281
> URL: https://issues.apache.org/jira/browse/IMAGING-281
> Project: Commons Imaging
> Issue Type: Bug
>Affects Versions: 1.0-alpha2
>Reporter: Garret Wilson
>Priority: Blocker
> Attachments: gate-turret-exif-bad-title.jpg
>
>
> I have a small input JPEG image containing _no metadata sections whatsoever_.
> I use Apache Commons Imaging 1.0-alpha2 to add two simple Exif {{IFD0}}
> properties using
>
[jira] [Updated] (IMAGING-281) Simple Exif XPTitle corrupted.
[
https://issues.apache.org/jira/browse/IMAGING-281?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Garret Wilson updated IMAGING-281:
--
Attachment: gate-turret-exif-bad-title.jpg
> Simple Exif XPTitle corrupted.
> --
>
> Key: IMAGING-281
> URL: https://issues.apache.org/jira/browse/IMAGING-281
> Project: Commons Imaging
> Issue Type: Bug
>Affects Versions: 1.0-alpha2
>Reporter: Garret Wilson
>Priority: Blocker
> Attachments: gate-turret-exif-bad-title.jpg
>
>
> I have a small input JPEG image containing _no metadata sections whatsoever_.
> I use Apache Commons Imaging 1.0-alpha2 to add two simple Exif {{IFD0}}
> properties using
> [`ExifRewriter().updateExifMetadataLossy()`|https://commons.apache.org/proper/commons-imaging/apidocs/org/apache/commons/imaging/formats/jpeg/exif/ExifRewriter.html#updateExifMetadataLossy-org.apache.commons.imaging.common.bytesource.ByteSource-java.io.OutputStream-org.apache.commons.imaging.formats.tiff.write.TiffOutputSet-].
> * {{XPTitle}} ({{0x9C9B}}): "Gate and Turret"
> * {{Copyright}} ({{33432}}, {{0x8298}}): "Copyright © 2009 Garret Wilson"
> Here is a simplified excerpt of the code:
> {code:java}
> TagInfoAscii EXIF_XP_TITLE_TAG_INFO = new TagInfoAscii("XPTitle", 0x9C9B, -1,
> TiffDirectoryType.EXIF_DIRECTORY_IFD0); //XPTitle (0x9C9B)
> TagInfoAscii EXIF_COPYRIGHT_TAG_INFO = new TagInfoAscii("Copyright", 0x8298,
> -1, TiffDirectoryType.EXIF_DIRECTORY_IFD0); //Copyright (33432, 0x8298)
> …
> TiffOutputSet tiffOutputSet = new TiffOutputSet();
> TiffOutputDirectory exifDirectory = tiffOutputSet.getOrCreateRootDirectory();
> exifDirectory.add(EXIF_XP_TITLE_TAG_INFO, "Gate and Turret");
> exifDirectory.add(EXIF_COPYRIGHT_TAG_INFO, "Copyright © 2009 Garret Wilson");
> …
> new ExifRewriter().updateExifMetadataLossy(byteSource, outputStream,
> tiffOutputSet);
> {code}
> Using [ExifTool|https://exiftool.org/] 12.16 (via
> [ExifToolGUI|https://exiftool.org/gui/) 5.16.0.0], the {{Copyright}} value is
> stored correctly but the {{XPTitle}} is stored as "慇整愠摮吠牵敲t".
> [Metadata++|https://www.logipole.com/metadata++-en.htm] 1.22.14 also shows
> the same corrupted value.
> This is disheartening, as this is nearly the most simple test case possible.
> (Note that [IrfanView|https://www.irfanview.com/] 4.54 can read the
> {{XPTitle}} just fine! Nevertheless ExifTool is the gold standard for image
> metadata reading, and is confirmed by Metadata++. Having an image that cannot
> be read in ExifTool is a show-stopper.)
> I'm will attach the test case image to this ticket.
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
[jira] [Created] (IMAGING-281) Simple Exif XPTitle corrupted.
Garret Wilson created IMAGING-281:
-
Summary: Simple Exif XPTitle corrupted.
Key: IMAGING-281
URL: https://issues.apache.org/jira/browse/IMAGING-281
Project: Commons Imaging
Issue Type: Bug
Affects Versions: 1.0-alpha2
Reporter: Garret Wilson
I have a small input JPEG image containing _no metadata sections whatsoever_. I
use Apache Commons Imaging 1.0-alpha2 to add two simple Exif {{IFD0}}
properties using
[`ExifRewriter().updateExifMetadataLossy()`|https://commons.apache.org/proper/commons-imaging/apidocs/org/apache/commons/imaging/formats/jpeg/exif/ExifRewriter.html#updateExifMetadataLossy-org.apache.commons.imaging.common.bytesource.ByteSource-java.io.OutputStream-org.apache.commons.imaging.formats.tiff.write.TiffOutputSet-].
* {{XPTitle}} ({{0x9C9B}}): "Gate and Turret"
* {{Copyright}} ({{33432}}, {{0x8298}}): "Copyright © 2009 Garret Wilson"
Here is a simplified excerpt of the code:
{code:java}
TagInfoAscii EXIF_XP_TITLE_TAG_INFO = new TagInfoAscii("XPTitle", 0x9C9B, -1,
TiffDirectoryType.EXIF_DIRECTORY_IFD0); //XPTitle (0x9C9B)
TagInfoAscii EXIF_COPYRIGHT_TAG_INFO = new TagInfoAscii("Copyright", 0x8298,
-1, TiffDirectoryType.EXIF_DIRECTORY_IFD0); //Copyright (33432, 0x8298)
…
TiffOutputSet tiffOutputSet = new TiffOutputSet();
TiffOutputDirectory exifDirectory = tiffOutputSet.getOrCreateRootDirectory();
exifDirectory.add(EXIF_XP_TITLE_TAG_INFO, "Gate and Turret");
exifDirectory.add(EXIF_COPYRIGHT_TAG_INFO, "Copyright © 2009 Garret Wilson");
…
new ExifRewriter().updateExifMetadataLossy(byteSource, outputStream,
tiffOutputSet);
{code}
Using [ExifTool|https://exiftool.org/] 12.16 (via
[ExifToolGUI|https://exiftool.org/gui/) 5.16.0.0], the {{Copyright}} value is
stored correctly but the {{XPTitle}} is stored as "慇整愠摮吠牵敲t".
[Metadata++|https://www.logipole.com/metadata++-en.htm] 1.22.14 also shows the
same corrupted value.
This is disheartening, as this is nearly the most simple test case possible.
(Note that [IrfanView|https://www.irfanview.com/] 4.54 can read the {{XPTitle}}
just fine! Nevertheless ExifTool is the gold standard for image metadata
reading, and is confirmed by Metadata++. Having an image that cannot be read in
ExifTool is a show-stopper.)
I'm will attach the test case image to this ticket.
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
[GitHub] [commons-release-plugin] garydgregory commented on pull request #34: Minor Improvement:
garydgregory commented on pull request #34: URL: https://github.com/apache/commons-release-plugin/pull/34#issuecomment-774552566 Hi @arturobernalg Please rebase on master and let's see of we can get a green build. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[jira] [Commented] (IO-719) PathUtils.copyDirectory() does not work when source and dest belong to different filesystems
[
https://issues.apache.org/jira/browse/IO-719?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17280293#comment-17280293
]
Gary D. Gregory commented on IO-719:
Hi [~ashcheglov]
Thank you for you report.
A PR on GitHub with a test would be most welcome.
> PathUtils.copyDirectory() does not work when source and dest belong to
> different filesystems
>
>
> Key: IO-719
> URL: https://issues.apache.org/jira/browse/IO-719
> Project: Commons IO
> Issue Type: Bug
> Components: Utilities
>Affects Versions: 2.8.0
> Environment: Java 8, Linux
>Reporter: Andrew Shcheglov
>Priority: Major
>
> This code:
> {code:java}
> @Test
> public void pathUtilsCopyDirFailure() throws IOException {
> FileSystem fs1 = Jimfs.newFileSystem();
> Path srcDir = fs1.getPath("srcDir");
> // Create files/dirs in srcDir
> Path targetDir = Paths.get("targetDir");
> PathUtils.copyDirectory(srcDir, targetDir);
> }
> {code}
> throws exception:
> {noformat}
> java.nio.file.ProviderMismatchException
> at java.base/sun.nio.fs.UnixPath.toUnixPath(UnixPath.java:198)
> at java.base/sun.nio.fs.UnixPath.resolve(UnixPath.java:410)
> at java.base/sun.nio.fs.UnixPath.resolve(UnixPath.java:43)
> at
> org.apache.commons.io.file.CopyDirectoryVisitor.preVisitDirectory(CopyDirectoryVisitor.java:130)
> at
> org.apache.commons.io.file.CopyDirectoryVisitor.preVisitDirectory(CopyDirectoryVisitor.java:36)
> at java.base/java.nio.file.Files.walkFileTree(Files.java:2731)
> at java.base/java.nio.file.Files.walkFileTree(Files.java:2796)
> at
> org.apache.commons.io.file.PathUtils.visitFileTree(PathUtils.java:687)
> at
> org.apache.commons.io.file.PathUtils.copyDirectory(PathUtils.java:196)
> {noformat}
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
[jira] [Commented] (IO-719) PathUtils.copyDirectory() does not work when source and dest belong to different filesystems
[
https://issues.apache.org/jira/browse/IO-719?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17280257#comment-17280257
]
Andrew Shcheglov commented on IO-719:
-
Possible solution: stringize relative path inside"resolve()" in
"CopyDirectoryVizitor.preVizitDirectory()" and
"CopyDirectoryVizitor.vizitFile()", like here:
{code:java}
@Override
public FileVisitResult preVisitDirectory(final Path directory, final
BasicFileAttributes attributes)
throws IOException {
final Path newTargetDir =
targetDirectory.resolve(sourceDirectory.relativize(directory).toString());
if (Files.notExists(newTargetDir)) {
Files.createDirectory(newTargetDir);
}
return super.preVisitDirectory(directory, attributes);
}
@Override
public FileVisitResult visitFile(final Path sourceFile, final
BasicFileAttributes attributes) throws IOException {
final Path targetFile =
targetDirectory.resolve(sourceDirectory.relativize(sourceFile).toString());
copy(sourceFile, targetFile);
return super.visitFile(targetFile, attributes);
}
{code}
> PathUtils.copyDirectory() does not work when source and dest belong to
> different filesystems
>
>
> Key: IO-719
> URL: https://issues.apache.org/jira/browse/IO-719
> Project: Commons IO
> Issue Type: Bug
> Components: Utilities
>Affects Versions: 2.8.0
> Environment: Java 8, Linux
>Reporter: Andrew Shcheglov
>Priority: Major
>
> This code:
> {code:java}
> @Test
> public void pathUtilsCopyDirFailure() throws IOException {
> FileSystem fs1 = Jimfs.newFileSystem();
> Path srcDir = fs1.getPath("srcDir");
> // Create files/dirs in srcDir
> Path targetDir = Paths.get("targetDir");
> PathUtils.copyDirectory(srcDir, targetDir);
> }
> {code}
> throws exception:
> {noformat}
> java.nio.file.ProviderMismatchException
> at java.base/sun.nio.fs.UnixPath.toUnixPath(UnixPath.java:198)
> at java.base/sun.nio.fs.UnixPath.resolve(UnixPath.java:410)
> at java.base/sun.nio.fs.UnixPath.resolve(UnixPath.java:43)
> at
> org.apache.commons.io.file.CopyDirectoryVisitor.preVisitDirectory(CopyDirectoryVisitor.java:130)
> at
> org.apache.commons.io.file.CopyDirectoryVisitor.preVisitDirectory(CopyDirectoryVisitor.java:36)
> at java.base/java.nio.file.Files.walkFileTree(Files.java:2731)
> at java.base/java.nio.file.Files.walkFileTree(Files.java:2796)
> at
> org.apache.commons.io.file.PathUtils.visitFileTree(PathUtils.java:687)
> at
> org.apache.commons.io.file.PathUtils.copyDirectory(PathUtils.java:196)
> {noformat}
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
[jira] [Created] (IO-719) PathUtils.copyDirectory() does not work when source and dest belong to different filesystems
Andrew Shcheglov created IO-719:
---
Summary: PathUtils.copyDirectory() does not work when source and
dest belong to different filesystems
Key: IO-719
URL: https://issues.apache.org/jira/browse/IO-719
Project: Commons IO
Issue Type: Bug
Components: Utilities
Affects Versions: 2.8.0
Environment: Java 8, Linux
Reporter: Andrew Shcheglov
This code:
{code:java}
@Test
public void pathUtilsCopyDirFailure() throws IOException {
FileSystem fs1 = Jimfs.newFileSystem();
Path srcDir = fs1.getPath("srcDir");
// Create files/dirs in srcDir
Path targetDir = Paths.get("targetDir");
PathUtils.copyDirectory(srcDir, targetDir);
}
{code}
throws exception:
{noformat}
java.nio.file.ProviderMismatchException
at java.base/sun.nio.fs.UnixPath.toUnixPath(UnixPath.java:198)
at java.base/sun.nio.fs.UnixPath.resolve(UnixPath.java:410)
at java.base/sun.nio.fs.UnixPath.resolve(UnixPath.java:43)
at
org.apache.commons.io.file.CopyDirectoryVisitor.preVisitDirectory(CopyDirectoryVisitor.java:130)
at
org.apache.commons.io.file.CopyDirectoryVisitor.preVisitDirectory(CopyDirectoryVisitor.java:36)
at java.base/java.nio.file.Files.walkFileTree(Files.java:2731)
at java.base/java.nio.file.Files.walkFileTree(Files.java:2796)
at
org.apache.commons.io.file.PathUtils.visitFileTree(PathUtils.java:687)
at
org.apache.commons.io.file.PathUtils.copyDirectory(PathUtils.java:196)
{noformat}
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
[GitHub] [commons-codec] YYTVicky commented on a change in pull request #44: Update DigestUtils.java
YYTVicky commented on a change in pull request #44:
URL: https://github.com/apache/commons-codec/pull/44#discussion_r571465718
##
File path: src/main/java/org/apache/commons/codec/digest/DigestUtils.java
##
@@ -192,6 +192,7 @@ public static MessageDigest getDigest(final String
algorithm) {
*/
public static MessageDigest getDigest(final String algorithm, final
MessageDigest defaultMessageDigest) {
try {
+/** potential insecure algorithm (MD2, MD5, SHA1, SHA256) called
here */
Review comment:
Hi @garydgregory , Thanks a lot for your kind reply, we are a security
research team at Virginia Tech. We are doing an empirical study about the
usefulness of the existing security vulnerability detection tools. May we
follow up on several questions:
**Is the bug report helpful?**
**will you prefer to fix the reported vulnerability**
**Are there any types of bugs/security vulnerabilities you want the
detection tools to pay more attention to?**
Appreciate any feedback from your side!
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [commons-release-plugin] garydgregory commented on pull request #34: Minor Improvement:
garydgregory commented on pull request #34: URL: https://github.com/apache/commons-release-plugin/pull/34#issuecomment-774502507 Hi @arturobernalg Please rebase on master to see if this fixes the build. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [commons-build-plugin] garydgregory merged pull request #18: Bump maven-antrun-plugin from 1.8 to 3.0.0
garydgregory merged pull request #18: URL: https://github.com/apache/commons-build-plugin/pull/18 This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [commons-build-plugin] garydgregory merged pull request #21: Bump actions/checkout from v2.3.3 to v2.3.4
garydgregory merged pull request #21: URL: https://github.com/apache/commons-build-plugin/pull/21 This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [commons-build-plugin] dependabot[bot] commented on pull request #23: Bump spotbugs-maven-plugin from 4.1.3 to 4.2.0
dependabot[bot] commented on pull request #23: URL: https://github.com/apache/commons-build-plugin/pull/23#issuecomment-774500597 OK, I won't notify you again about this release, but will get in touch when a new version is available. If you change your mind, just re-open this PR and I'll resolve any conflicts on it. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [commons-build-plugin] garydgregory closed pull request #23: Bump spotbugs-maven-plugin from 4.1.3 to 4.2.0
garydgregory closed pull request #23: URL: https://github.com/apache/commons-build-plugin/pull/23 This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [commons-build-plugin] garydgregory merged pull request #24: Bump spotbugs from 4.1.3 to 4.2.1
garydgregory merged pull request #24: URL: https://github.com/apache/commons-build-plugin/pull/24 This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[jira] [Closed] (NET-694) apache commons net 关于 DiscardUDPClient
[
https://issues.apache.org/jira/browse/NET-694?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Gary D. Gregory closed NET-694.
---
Resolution: Invalid
No in reply in ~60 days, closing.
> apache commons net 关于 DiscardUDPClient
> --
>
> Key: NET-694
> URL: https://issues.apache.org/jira/browse/NET-694
> Project: Commons Net
> Issue Type: Improvement
>Affects Versions: 3.6
>Reporter: wqd
>Priority: Major
>
> There is "java.lang.IllegalArgumentException: illegal length" when i use
> DiscardUDPClient to send.
>
> {code:java}
> java.lang.IllegalArgumentException: illegal length
> at java.net.DatagramPacket.setLength(DatagramPacket.java:378)
> at
> org.apache.commons.net.discard.DiscardUDPClient.send(DiscardUDPClient.java:68)
> {code}
>
> See the source
> "https://github.com/apache/commons-net/blob/master/src/main/java/org/apache/commons/net/discard/DiscardUDPClient.java;.
> {code:java}
> public DiscardUDPClient(){
> sendPacket = new DatagramPacket(new byte[0], 0);
> }
> public void send(final byte[] data, final int length, final InetAddress host,
> final int port)
> throws IOException
> {
> sendPacket.setData(data);
> sendPacket.setLength(length);
> sendPacket.setAddress(host);
> sendPacket.setPort(port);
> _socket_.send(sendPacket);
> }
> {code}
> and "java.net.DatagramPacket".
> {code:java}
> public synchronized void setData(byte[] buf) {
> if (buf == null) {
> throw new NullPointerException("null packet buffer");
> }
> this.buf = buf;
> this.offset = 0;
> this.length = buf.length;
> this.bufLength = buf.length;
> }
> public synchronized void setLength(int length) {
> if ((length + offset) > buf.length || length < 0 ||
> (length + offset) < 0) {
> throw new IllegalArgumentException("illegal length");
> }
> this.length = length;
> this.bufLength = this.length;
> }
> {code}
> I think in the case of multithreading, the "DiscardUDPClient.send" method is
> not Atomicity.
> I think that's why my problem came up.
>
> why not add synchronized for "DiscardUDPClient.send"?
>
> Forgive me for my nonstandard English
>
>
>
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
[GitHub] [commons-net] garydgregory merged pull request #72: Minor Improvements
garydgregory merged pull request #72: URL: https://github.com/apache/commons-net/pull/72 This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [commons-vfs] efge commented on pull request #154: Rework SoftRefFilesCache locking
efge commented on pull request #154: URL: https://github.com/apache/commons-vfs/pull/154#issuecomment-774486986 > > Given that the Loom project will likely have issues with `synchronized` in its first iterations > > What kind of problems are caused by `synchronized`? I'm not a Java expert, I'm curious, I thought that was the first-class synchronization feature that always "just works", and you only need stuff like `ReentrantLock` when you need special features. It is today, but the ongoing Loom project aims at adding new lightweight thread scheduling features for which the low-level monitor that `synchronized` takes is harder to interact with (again: in the first iterations, it'll eventually work ok). See https://wiki.openjdk.java.net/display/loom/Main for more, search for "synchronized". Therefore the Java libraries that aim to be Loom-friendly try to currently avoid `synchronized` blocks. Here's an example from the PostgreSQL JDBC driver: https://github.com/pgjdbc/pgjdbc/issues/1951 This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[jira] [Work logged] (LANG-1641) Over Stack Issue
[
https://issues.apache.org/jira/browse/LANG-1641?focusedWorklogId=549058=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-549058
]
ASF GitHub Bot logged work on LANG-1641:
Author: ASF GitHub Bot
Created on: 06/Feb/21 12:33
Start Date: 06/Feb/21 12:33
Worklog Time Spent: 10m
Work Description: coveralls edited a comment on pull request #703:
URL: https://github.com/apache/commons-lang/pull/703#issuecomment-770428498
[](https://coveralls.io/builds/36919471)
Coverage increased (+0.001%) to 94.983% when pulling
**9cabf7483af3215a1b80a33aa07af38965e8ea44 on mdbuck77:LANG_1641** into
**3e27e51770124866c530ed321f21368ea07e7740 on apache:master**.
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Issue Time Tracking
---
Worklog Id: (was: 549058)
Time Spent: 50m (was: 40m)
> Over Stack Issue
>
>
> Key: LANG-1641
> URL: https://issues.apache.org/jira/browse/LANG-1641
> Project: Commons Lang
> Issue Type: Bug
> Components: lang.time.*
>Affects Versions: 3.7
>Reporter: Xia Yun
>Priority: Major
> Time Spent: 50m
> Remaining Estimate: 0h
>
> {code:java}
> //this is the demo
> package com.wcs233;
> import java.util.Date;
> import java.util.TimeZone;
> import org.apache.commons.lang3.time.DateFormatUtils;
> import org.apache.commons.lang3.time.FastTimeZone;
> public class GmtTimeZoneIssue {
> public static void main(String[] args) {
> final String timeZoneString = "GMT+7:00";
> final String ISO8601_DATE_FORMAT = "-MM-dd'T'HH:mm:ssZZ";
> for (int i = 0; i < 100; i++) {
> Date date = new Date();
> TimeZone timeZone = FastTimeZone.getTimeZone(timeZoneString);
> String dateString = DateFormatUtils.format(date,
> ISO8601_DATE_FORMAT, timeZone);
> System.out.println(dateString);
> }
> }
> }{code}
> when running the code above , there is over stack risk
> 1. When invoking FastTimeZone.getTimeZone(timeZoneString), a new GmtTimeZone
> will be created
> {code:java}
> GmtTimeZone(final boolean negate, final int hours, final int minutes) {
> if (hours >= HOURS_PER_DAY) {
> throw new IllegalArgumentException(hours + " hours out of range");
> }
> if (minutes >= MINUTES_PER_HOUR) {
> throw new IllegalArgumentException(minutes + " minutes out of
> range");
> }
> final int milliseconds = (minutes + (hours * MINUTES_PER_HOUR)) *
> MILLISECONDS_PER_MINUTE;
> offset = negate ? -milliseconds : milliseconds;
> zoneId = twoDigits(
> twoDigits(new StringBuilder(9).append("GMT").append(negate ? '-'
> : '+'), hours)
> .append(':'), minutes).toString();
> }
> {code}
> the every GmtTimeZone instance , the zoneIds instance are different , even
> through the values are the same.
> 2. When invoking DateFormatUtils.format(), there is a ConCurrentHashMap in
> FormatCache with key is MultipartKey. when invoking
> FormatCache.getInstance(), first get from cInstanceCache, if value is null,
> putIfAbsent, but because GmtTimeZone instances are different, when invoking
> GmtTimeZone.equals, the logic is
> {code:java}
> @Override
> public boolean equals(final Object other) {
> if (!(other instanceof GmtTimeZone)) {
> return false;
> }
> return zoneId == ((GmtTimeZone) other).zoneId;
> }
> {code}
> every zoneId address are different so, equals function return false, but
> actually the zoneId values are the same.
> so FastDateFormat will be put into cInstanceCache in FormatCache again and
> again, which will increase the capability of the map, and cause over stack
> risk, also, it will cause application run much slower.
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
[GitHub] [commons-lang] coveralls edited a comment on pull request #703: LANG-1641: GmtTimeZone now implements #equals(Object) using it's time…
coveralls edited a comment on pull request #703: URL: https://github.com/apache/commons-lang/pull/703#issuecomment-770428498 [](https://coveralls.io/builds/36919471) Coverage increased (+0.001%) to 94.983% when pulling **9cabf7483af3215a1b80a33aa07af38965e8ea44 on mdbuck77:LANG_1641** into **3e27e51770124866c530ed321f21368ea07e7740 on apache:master**. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[jira] [Work logged] (LANG-1641) Over Stack Issue
[
https://issues.apache.org/jira/browse/LANG-1641?focusedWorklogId=549051=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-549051
]
ASF GitHub Bot logged work on LANG-1641:
Author: ASF GitHub Bot
Created on: 06/Feb/21 12:24
Start Date: 06/Feb/21 12:24
Worklog Time Spent: 10m
Work Description: mdbuck77 commented on a change in pull request #703:
URL: https://github.com/apache/commons-lang/pull/703#discussion_r571429319
##
File path: src/main/java/org/apache/commons/lang3/time/GmtTimeZone.java
##
@@ -33,8 +35,7 @@
// Serializable!
static final long serialVersionUID = 1L;
-private final int offset;
-private final String zoneId;
+private final SimpleTimeZone m_delegate;
Review comment:
done
##
File path: src/main/java/org/apache/commons/lang3/time/GmtTimeZone.java
##
@@ -67,39 +69,43 @@ public void setRawOffset(final int offsetMillis) {
@Override
public int getRawOffset() {
-return offset;
+return m_delegate.getRawOffset();
}
@Override
public String getID() {
-return zoneId;
+return m_delegate.getID();
}
@Override
public boolean useDaylightTime() {
-return false;
+return m_delegate.useDaylightTime();
}
@Override
public boolean inDaylightTime(final Date date) {
-return false;
+return m_delegate.inDaylightTime(date);
}
@Override
public String toString() {
-return "[GmtTimeZone id=\"" + zoneId + "\",offset=" + offset + ']';
+return "[GmtTimeZone id=\"" + getID() + "\",offset=" + getRawOffset()
+ ']';
}
@Override
-public int hashCode() {
-return offset;
+public boolean equals(Object o) {
+if (this == o) {
+return true;
+}
+if (o == null || getClass() != o.getClass()) {
Review comment:
I made the class final.
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Issue Time Tracking
---
Worklog Id: (was: 549051)
Time Spent: 40m (was: 0.5h)
> Over Stack Issue
>
>
> Key: LANG-1641
> URL: https://issues.apache.org/jira/browse/LANG-1641
> Project: Commons Lang
> Issue Type: Bug
> Components: lang.time.*
>Affects Versions: 3.7
>Reporter: Xia Yun
>Priority: Major
> Time Spent: 40m
> Remaining Estimate: 0h
>
> {code:java}
> //this is the demo
> package com.wcs233;
> import java.util.Date;
> import java.util.TimeZone;
> import org.apache.commons.lang3.time.DateFormatUtils;
> import org.apache.commons.lang3.time.FastTimeZone;
> public class GmtTimeZoneIssue {
> public static void main(String[] args) {
> final String timeZoneString = "GMT+7:00";
> final String ISO8601_DATE_FORMAT = "-MM-dd'T'HH:mm:ssZZ";
> for (int i = 0; i < 100; i++) {
> Date date = new Date();
> TimeZone timeZone = FastTimeZone.getTimeZone(timeZoneString);
> String dateString = DateFormatUtils.format(date,
> ISO8601_DATE_FORMAT, timeZone);
> System.out.println(dateString);
> }
> }
> }{code}
> when running the code above , there is over stack risk
> 1. When invoking FastTimeZone.getTimeZone(timeZoneString), a new GmtTimeZone
> will be created
> {code:java}
> GmtTimeZone(final boolean negate, final int hours, final int minutes) {
> if (hours >= HOURS_PER_DAY) {
> throw new IllegalArgumentException(hours + " hours out of range");
> }
> if (minutes >= MINUTES_PER_HOUR) {
> throw new IllegalArgumentException(minutes + " minutes out of
> range");
> }
> final int milliseconds = (minutes + (hours * MINUTES_PER_HOUR)) *
> MILLISECONDS_PER_MINUTE;
> offset = negate ? -milliseconds : milliseconds;
> zoneId = twoDigits(
> twoDigits(new StringBuilder(9).append("GMT").append(negate ? '-'
> : '+'), hours)
> .append(':'), minutes).toString();
> }
> {code}
> the every GmtTimeZone instance , the zoneIds instance are different , even
> through the values are the same.
> 2. When invoking DateFormatUtils.format(), there is a ConCurrentHashMap in
> FormatCache with key is MultipartKey. when invoking
> FormatCache.getInstance(), first get from cInstanceCache, if value is null,
> putIfAbsent, but because GmtTimeZone instances are different, when invoking
> GmtTimeZone.equals, the logic is
> {code:java}
> @Override
> public boolean
[GitHub] [commons-lang] mdbuck77 commented on a change in pull request #703: LANG-1641: GmtTimeZone now implements #equals(Object) using it's time…
mdbuck77 commented on a change in pull request #703:
URL: https://github.com/apache/commons-lang/pull/703#discussion_r571429319
##
File path: src/main/java/org/apache/commons/lang3/time/GmtTimeZone.java
##
@@ -33,8 +35,7 @@
// Serializable!
static final long serialVersionUID = 1L;
-private final int offset;
-private final String zoneId;
+private final SimpleTimeZone m_delegate;
Review comment:
done
##
File path: src/main/java/org/apache/commons/lang3/time/GmtTimeZone.java
##
@@ -67,39 +69,43 @@ public void setRawOffset(final int offsetMillis) {
@Override
public int getRawOffset() {
-return offset;
+return m_delegate.getRawOffset();
}
@Override
public String getID() {
-return zoneId;
+return m_delegate.getID();
}
@Override
public boolean useDaylightTime() {
-return false;
+return m_delegate.useDaylightTime();
}
@Override
public boolean inDaylightTime(final Date date) {
-return false;
+return m_delegate.inDaylightTime(date);
}
@Override
public String toString() {
-return "[GmtTimeZone id=\"" + zoneId + "\",offset=" + offset + ']';
+return "[GmtTimeZone id=\"" + getID() + "\",offset=" + getRawOffset()
+ ']';
}
@Override
-public int hashCode() {
-return offset;
+public boolean equals(Object o) {
+if (this == o) {
+return true;
+}
+if (o == null || getClass() != o.getClass()) {
Review comment:
I made the class final.
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [commons-vfs] kinow commented on pull request #154: Rework SoftRefFilesCache locking
kinow commented on pull request #154: URL: https://github.com/apache/commons-vfs/pull/154#issuecomment-774442260 >If that makes it easier for you, OK for me. Here's part 1: #158 >As soon as you merge it, I'll rebase this PR and drop the merged commits. Thanks @MaxKellermann ! >In projects I maintain, if I like some obvious-good commits, I cherry-pick them and then start discussing the others, and I expect the submitter to rebase the PR with only the unmerged commits remaining. :+1: >What I certainly would never do is fold all PR commits into one large commit like you did with #155 - 3 commits were folded into one (9c4302e). That loses a lot of context, makes the resulting commit hard to read/verify/understand, and makes git-bisect very hard. That's your project, your maintenance decision, so I'm not going to argue, just mentioning it because that's one thing that would make project management hard for me. My workflow is normally to check out the branch locally, rebase onto master — or make sure it's rebased — and then checkout master and merge with `--no-ff`. If the PR has many commits, I may squash them if they appear to be draft/WIP commits, or ask the user if s/he can squash it (asked last week I think, in Apache Commons Imaging). But sometimes we may squash without thinking/asking. Some times when I create a PR I mention that I haven't squashed for some specific reason, and tell the maintainers I'd be happy to squash it if they think it's necessary. If you mention that I'm sure the final reviewer will consider it too. Some projects also have a hard-policy to always merge a single commit (Apache OpenNLP for example, though not sure if that changed). Finally, I forgot to say **thank you** for the great commit messages. They are short, and explain what's included in that commit :+1: This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [commons-vfs] MaxKellermann commented on pull request #154: Rework SoftRefFilesCache locking
MaxKellermann commented on pull request #154: URL: https://github.com/apache/commons-vfs/pull/154#issuecomment-774433274 > @MaxKellermann, @garydgregory what do you think if we break this PR into a few other smaller PR's, I think that would be easier to review, and grouping that way in case some bugs/issue is raised later, it might be easier to look at the changelog and see what could be causing it. If that makes it easier for you, OK for me. Here's part 1: https://github.com/apache/commons-vfs/pull/158 As soon as you merge it, I'll rebase this PR and drop the merged commits. In projects I maintain, if I like some obvious-good commits, I cherry-pick them and then start discussing the others, and I expect the submitter to rebase the PR with only the unmerged commits remaining. What I certainly would never do is fold all PR commits into one large commit like you did with https://github.com/apache/commons-vfs/pull/155 - 3 commits were folded into one (https://github.com/apache/commons-vfs/commit/9c4302e4559acb1e2a627f57f49105c54fe09895). That loses a lot of context, makes the resulting commit hard to read/verify/understand, and makes git-bisect very hard. That's your project, your maintenance decision, so I'm not going to argue, just mentioning it because that's one thing that would make project management hard for me. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [commons-vfs] MaxKellermann opened a new pull request #158: Rework SoftRefFilesCache locking part 1
MaxKellermann opened a new pull request #158: URL: https://github.com/apache/commons-vfs/pull/158 Splitted out of https://github.com/apache/commons-vfs/pull/154 as suggested by @kinow This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [commons-vfs] kinow removed a comment on pull request #154: Rework SoftRefFilesCache locking
kinow removed a comment on pull request #154:
URL: https://github.com/apache/commons-vfs/pull/154#issuecomment-774422095
>I totally sympathise with @garydgregory about the lack of test, but also
with @MaxKellermann in that writing tests that exercise thread-safety issues is
a PITA.
Ditto here. Writing tests for the thread-safety would be nice, but
understandable if it's too hard to write.
@MaxKellermann, @garydgregory what do you think if we break this PR into a
few other smaller PR's, I think that would be easier to review, and grouping
that way in case some bugs/issue is raised later, it might be easier to look at
the changelog and see what could be causing it.
For example, I think we could have one PR with smal improvements like (easy
to review, don't think we need tests):
- remove `super.close()` (maybe add a comment to the code where it was
removed so we don't accidentally add it back? Will leave a comment there...)
- use isEmpty() instead of size < 1
- remove redundant isInterrupted() check
- simplify the InterruptedException catch block
Another for the Lock issues (great catch! in some cases we locked twice, in
others forgot to require the lock, and in other cases we also locked the Lock
object without acquiring the lock; I'd say we could code-review this change
without a test [testing synchronized code blocks is hard, but code that is
acquiring a Lock is even harder IMO])
- fix ReentrantLock usage in {start,end}Thread()
- add missing lock to removeFile()
- require lock while calling removeFile(FileSystemAn…
- require lock while calling getOrCreateFilesystemCa…
- move endThread() call inside the lock
- require lock for startThread(), endThread()
- move code to removeFile(Reference)
- don't use ConcurrentMap (oh, I liked this! Would need to read the code
with calm to understand if there was no other case for the concurrent map, but
I think you might be correct here!)
Another PR for the volatile change (code-review might do I think)
- eliminate "volatile" on softRefReleaseThread
And one more for the removal of `ReentrantLock`? Or maybe just create an
issue for discussion regarding the Loom project with fibers and possible issues
(I think that's what @efge meant?).
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [commons-vfs] kinow commented on pull request #154: Rework SoftRefFilesCache locking
kinow commented on pull request #154:
URL: https://github.com/apache/commons-vfs/pull/154#issuecomment-774422095
>I totally sympathise with @garydgregory about the lack of test, but also
with @MaxKellermann in that writing tests that exercise thread-safety issues is
a PITA.
Ditto here. Writing tests for the thread-safety would be nice, but
understandable if it's too hard to write.
@MaxKellermann, @garydgregory what do you think if we break this PR into a
few other smaller PR's, I think that would be easier to review, and grouping
that way in case some bugs/issue is raised later, it might be easier to look at
the changelog and see what could be causing it.
For example, I think we could have one PR with smal improvements like (easy
to review, don't think we need tests):
- remove `super.close()` (maybe add a comment to the code where it was
removed so we don't accidentally add it back? Will leave a comment there...)
- use isEmpty() instead of size < 1
- remove redundant isInterrupted() check
- simplify the InterruptedException catch block
Another for the Lock issues (great catch! in some cases we locked twice, in
others forgot to require the lock, and in other cases we also locked the Lock
object without acquiring the lock; I'd say we could code-review this change
without a test [testing synchronized code blocks is hard, but code that is
acquiring a Lock is even harder IMO])
- fix ReentrantLock usage in {start,end}Thread()
- add missing lock to removeFile()
- require lock while calling removeFile(FileSystemAn…
- require lock while calling getOrCreateFilesystemCa…
- move endThread() call inside the lock
- require lock for startThread(), endThread()
- move code to removeFile(Reference)
- don't use ConcurrentMap (oh, I liked this! Would need to read the code
with calm to understand if there was no other case for the concurrent map, but
I think you might be correct here!)
Another PR for the volatile change (code-review might do I think)
- eliminate "volatile" on softRefReleaseThread
And one more for the removal of `ReentrantLock`? Or maybe just create an
issue for discussion regarding the Loom project with fibers and possible issues
(I think that's what @efge meant?).
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
