You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by GitBox <gi...@apache.org> on 2020/04/20 16:17:43 UTC

[GitHub] [arrow] tpboudreau opened a new pull request #6993: ARROW-8477: [C++] Enable reading and writing of long filenames for Windows

tpboudreau opened a new pull request #6993:
URL: https://github.com/apache/arrow/pull/6993


   This patch enables reading/writing of files with long (>260 characters) pathnames in Windows.
   
   In order for the new test to run under Windows, both (1) the test host must have long paths enabled in its registry, and (2) the test executable (arrow_utility_test.exe) must include a manifest indicating support for long paths (see https://docs.microsoft.com/en-us/windows/win32/fileio/naming-a-file?redirectedfrom=MSDN#enable-long-paths-in-windows-10-version-1607-and-later).  The test source code checks for (1) and the cmake file changes ensure (2).


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [arrow] tpboudreau commented on a change in pull request #6993: ARROW-8477: [C++] Enable reading and writing of long filenames for Windows

Posted by GitBox <gi...@apache.org>.
tpboudreau commented on a change in pull request #6993:
URL: https://github.com/apache/arrow/pull/6993#discussion_r411569025



##########
File path: cpp/src/arrow/util/io_util_test.cc
##########
@@ -446,6 +446,56 @@ TEST(CreateDirTree, Basics) {
   ASSERT_OK_AND_ASSIGN(fn, temp_dir->path().Join("EF"));
   ASSERT_OK_AND_ASSIGN(created, CreateDirTree(fn));
   ASSERT_TRUE(created);
+
+#ifndef __APPLE__

Review comment:
       I experienced failures in the CI pipeline on macOS and I was unable to locate clear documentation of the path name limits (I'm not a macOS expert). 
   
   I figured it might be best to separately address macOS in another issue, if there's community support for that.  (This patch leaves macOS builds and tests unchanged.)
   
   If you believe this test should run as is under macOS, I'll remove the #ifndef and follow up on any issues.
   
   




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [arrow] pitrou commented on issue #6993: ARROW-8477: [C++] Enable reading and writing of long filenames for Windows

Posted by GitBox <gi...@apache.org>.
pitrou commented on issue #6993:
URL: https://github.com/apache/arrow/pull/6993#issuecomment-616748578


   The remaining CI failure is unrelated.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [arrow] github-actions[bot] commented on issue #6993: ARROW-8477: [C++] Enable reading and writing of long filenames for Windows

Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on issue #6993:
URL: https://github.com/apache/arrow/pull/6993#issuecomment-616667220


   https://issues.apache.org/jira/browse/ARROW-8477


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [arrow] pitrou commented on issue #6993: ARROW-8477: [C++] Enable reading and writing of long filenames for Windows

Posted by GitBox <gi...@apache.org>.
pitrou commented on issue #6993:
URL: https://github.com/apache/arrow/pull/6993#issuecomment-616703869


   Looks like Windows long paths are enabled by default on Github Actions. Cool!


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [arrow] tpboudreau commented on issue #6993: ARROW-8477: [C++] Enable reading and writing of long filenames for Windows

Posted by GitBox <gi...@apache.org>.
tpboudreau commented on issue #6993:
URL: https://github.com/apache/arrow/pull/6993#issuecomment-616714142


   Your changes look good.  Thanks!


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [arrow] pitrou commented on a change in pull request #6993: ARROW-8477: [C++] Enable reading and writing of long filenames for Windows

Posted by GitBox <gi...@apache.org>.
pitrou commented on a change in pull request #6993:
URL: https://github.com/apache/arrow/pull/6993#discussion_r411570280



##########
File path: cpp/src/arrow/util/io_util_test.cc
##########
@@ -446,6 +446,56 @@ TEST(CreateDirTree, Basics) {
   ASSERT_OK_AND_ASSIGN(fn, temp_dir->path().Join("EF"));
   ASSERT_OK_AND_ASSIGN(created, CreateDirTree(fn));
   ASSERT_TRUE(created);
+
+#ifndef __APPLE__

Review comment:
       I was just wondering. According to Google searches, the path length limit on macOS may be 1024, which this test exceeds. We can keep it like that.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [arrow] tpboudreau commented on issue #6993: ARROW-8477: [C++] Enable reading and writing of long filenames for Windows

Posted by GitBox <gi...@apache.org>.
tpboudreau commented on issue #6993:
URL: https://github.com/apache/arrow/pull/6993#issuecomment-616759574


   Thanks @pitrou for jumping on this so quickly.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [arrow] pitrou commented on a change in pull request #6993: ARROW-8477: [C++] Enable reading and writing of long filenames for Windows

Posted by GitBox <gi...@apache.org>.
pitrou commented on a change in pull request #6993:
URL: https://github.com/apache/arrow/pull/6993#discussion_r411547763



##########
File path: cpp/src/arrow/util/io_util_test.cc
##########
@@ -446,6 +446,56 @@ TEST(CreateDirTree, Basics) {
   ASSERT_OK_AND_ASSIGN(fn, temp_dir->path().Join("EF"));
   ASSERT_OK_AND_ASSIGN(created, CreateDirTree(fn));
   ASSERT_TRUE(created);
+
+#ifndef __APPLE__

Review comment:
       Why did you have to disable this test on macOS?




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [arrow] tpboudreau edited a comment on issue #6993: ARROW-8477: [C++] Enable reading and writing of long filenames for Windows

Posted by GitBox <gi...@apache.org>.
tpboudreau edited a comment on issue #6993:
URL: https://github.com/apache/arrow/pull/6993#issuecomment-616714142


   Your fixups look good.  Thanks!


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org