Skip to content

Commit

Permalink
ARROW-8477: [C++] Enable reading and writing of long filenames for Wi…
Browse files Browse the repository at this point in the history
…ndows

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).

Closes #6993 from tpboudreau/ARROW-8477

Lead-authored-by: TP Boudreau <tpboudreau@gmail.com>
Co-authored-by: Antoine Pitrou <antoine@python.org>
Signed-off-by: Antoine Pitrou <antoine@python.org>
  • Loading branch information
tpboudreau and pitrou committed Apr 20, 2020
1 parent 456a6f6 commit 1b71ca7
Show file tree
Hide file tree
Showing 6 changed files with 192 additions and 9 deletions.
4 changes: 3 additions & 1 deletion .github/workflows/cpp.yml
Expand Up @@ -228,7 +228,9 @@ jobs:
run: ci/scripts/util_checkout.sh
- name: Build
shell: bash
run: ci/scripts/cpp_build.sh $(pwd) $(pwd)/build
run: |
export BOOST_ROOT=$BOOST_ROOT_1_72_0
ci/scripts/cpp_build.sh $(pwd) $(pwd)/build
- name: Test
shell: bash
run: ci/scripts/cpp_test.sh $(pwd) $(pwd)/build
Expand Down
14 changes: 13 additions & 1 deletion cpp/src/arrow/util/CMakeLists.txt
Expand Up @@ -26,6 +26,18 @@ arrow_install_all_headers("arrow/util")
# arrow_test_main
#

if(WIN32)
# This manifest enables long file paths on Windows 10+
# See https://docs.microsoft.com/en-us/windows/win32/fileio/naming-a-file#enable-long-paths-in-windows-10-version-1607-and-later
if(MSVC)
set(IO_UTIL_TEST_SOURCES io_util_test.cc io_util_test.manifest)
else()
set(IO_UTIL_TEST_SOURCES io_util_test.cc io_util_test.rc)
endif()
else()
set(IO_UTIL_TEST_SOURCES io_util_test.cc)
endif()

add_arrow_test(utility-test
SOURCES
align_util_test.cc
Expand All @@ -37,7 +49,7 @@ add_arrow_test(utility-test
key_value_metadata_test.cc
hashing_test.cc
int_util_test.cc
io_util_test.cc
${IO_UTIL_TEST_SOURCES}
iterator_test.cc
logging_test.cc
parsing_util_test.cc
Expand Down
40 changes: 33 additions & 7 deletions cpp/src/arrow/util/io_util.cc
Expand Up @@ -837,8 +837,19 @@ Result<int> FileOpenReadable(const PlatformFilename& file_name) {
int fd, errno_actual;
#if defined(_WIN32)
SetLastError(0);
errno_actual = _wsopen_s(&fd, file_name.ToNative().c_str(),
_O_RDONLY | _O_BINARY | _O_NOINHERIT, _SH_DENYNO, _S_IREAD);
HANDLE file_handle = CreateFileW(file_name.ToNative().c_str(), GENERIC_READ,
FILE_SHARE_READ | FILE_SHARE_WRITE, NULL,
OPEN_EXISTING, FILE_ATTRIBUTE_NORMAL, NULL);

DWORD last_error = GetLastError();
if (last_error == ERROR_SUCCESS) {
errno_actual = 0;
fd = _open_osfhandle(reinterpret_cast<intptr_t>(file_handle),
_O_RDONLY | _O_BINARY | _O_NOINHERIT);
} else {
return IOErrorFromWinError(last_error, "Failed to open local file '",
file_name.ToString(), "'");
}
#else
fd = open(file_name.ToNative().c_str(), O_RDONLY);
errno_actual = errno;
Expand Down Expand Up @@ -868,23 +879,38 @@ Result<int> FileOpenWritable(const PlatformFilename& file_name, bool write_only,
#if defined(_WIN32)
SetLastError(0);
int oflag = _O_CREAT | _O_BINARY | _O_NOINHERIT;
int pmode = _S_IREAD | _S_IWRITE;
DWORD desired_access = GENERIC_WRITE;
DWORD share_mode = FILE_SHARE_READ | FILE_SHARE_WRITE;
DWORD creation_disposition = OPEN_ALWAYS;

if (truncate) {
oflag |= _O_TRUNC;
}
if (append) {
oflag |= _O_APPEND;
}

if (truncate) {
oflag |= _O_TRUNC;
creation_disposition = CREATE_ALWAYS;
}

if (write_only) {
oflag |= _O_WRONLY;
} else {
oflag |= _O_RDWR;
desired_access |= GENERIC_READ;
}

errno_actual = _wsopen_s(&fd, file_name.ToNative().c_str(), oflag, _SH_DENYNO, pmode);
HANDLE file_handle =
CreateFileW(file_name.ToNative().c_str(), desired_access, share_mode, NULL,
creation_disposition, FILE_ATTRIBUTE_NORMAL, NULL);

DWORD last_error = GetLastError();
if (last_error == ERROR_SUCCESS || last_error == ERROR_ALREADY_EXISTS) {
errno_actual = 0;
fd = _open_osfhandle(reinterpret_cast<intptr_t>(file_handle), oflag);
} else {
return IOErrorFromWinError(last_error, "Failed to open local file '",
file_name.ToString(), "'");
}
#else
int oflag = O_CREAT;

Expand Down
60 changes: 60 additions & 0 deletions cpp/src/arrow/util/io_util_test.cc
Expand Up @@ -394,6 +394,10 @@ TEST(DeleteDirContents, Basics) {
#else
ASSERT_EQ(ErrnoFromStatus(status), ENOENT);
#endif

// Now actually delete the test directory
ASSERT_OK_AND_ASSIGN(deleted, DeleteDirTree(parent));
ASSERT_TRUE(deleted);
}

TEST(TemporaryDir, Basics) {
Expand Down Expand Up @@ -523,5 +527,61 @@ TEST(DeleteFile, Basics) {
ASSERT_RAISES(IOError, DeleteFile(fn));
}

#ifndef __APPLE__
TEST(FileUtils, LongPaths) {
// ARROW-8477: check using long file paths under Windows (> 260 characters).
bool created, deleted;
#ifdef _WIN32
const char* kRegKeyName = R"(SYSTEM\CurrentControlSet\Control\FileSystem)";
const char* kRegValueName = "LongPathsEnabled";
DWORD value = 0;
DWORD size = sizeof(value);
LSTATUS status = RegGetValueA(HKEY_LOCAL_MACHINE, kRegKeyName, kRegValueName,
RRF_RT_REG_DWORD, NULL, &value, &size);
bool test_long_paths = (status == ERROR_SUCCESS && value == 1);
if (!test_long_paths) {
ARROW_LOG(WARNING)
<< "Tests for accessing files with long path names have been disabled. "
<< "To enable these tests, set the value of " << kRegValueName
<< " in registry key \\HKEY_LOCAL_MACHINE\\" << kRegKeyName
<< " to 1 on the test host.";
return;
}
#endif

const std::string BASE = "xxx-io-util-test-dir-long";
PlatformFilename base_path, long_path, long_filename;
int fd = -1;
std::stringstream fs;
fs << BASE;
for (int i = 0; i < 64; ++i) {
fs << "/123456789ABCDEF";
}
ASSERT_OK_AND_ASSIGN(base_path,
PlatformFilename::FromString(BASE)); // long_path length > 1024
ASSERT_OK_AND_ASSIGN(
long_path, PlatformFilename::FromString(fs.str())); // long_path length > 1024
ASSERT_OK_AND_ASSIGN(created, CreateDirTree(long_path));
ASSERT_TRUE(created);
AssertExists(long_path);
ASSERT_OK_AND_ASSIGN(long_filename,
PlatformFilename::FromString(fs.str() + "/file.txt"));
ASSERT_OK_AND_ASSIGN(fd, FileOpenWritable(long_filename));
ASSERT_OK(FileClose(fd));
AssertExists(long_filename);
fd = -1;
ASSERT_OK_AND_ASSIGN(fd, FileOpenReadable(long_filename));
ASSERT_OK(FileClose(fd));
ASSERT_OK_AND_ASSIGN(deleted, DeleteDirContents(long_path));
ASSERT_TRUE(deleted);
ASSERT_OK_AND_ASSIGN(deleted, DeleteDirTree(long_path));
ASSERT_TRUE(deleted);

// Now delete the whole test directory tree
ASSERT_OK_AND_ASSIGN(deleted, DeleteDirTree(base_path));
ASSERT_TRUE(deleted);
}
#endif

} // namespace internal
} // namespace arrow
39 changes: 39 additions & 0 deletions cpp/src/arrow/util/io_util_test.manifest
@@ -0,0 +1,39 @@
<?xml version="1.0" encoding="utf-8"?>
<!--
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.
-->

<!--
Enable long file paths on the target application
See https://docs.microsoft.com/en-us/windows/win32/fileio/naming-a-file#enable-long-paths-in-windows-10-version-1607-and-later
-->
<assembly xmlns="urn:schemas-microsoft-com:asm.v1" manifestVersion="1.0">
<assemblyIdentity type="win32" name="ArrowUtilityTest" version="1.1.1.1"/>
<application xmlns="urn:schemas-microsoft-com:asm.v3">
<windowsSettings xmlns:ws2="http://schemas.microsoft.com/SMI/2016/WindowsSettings">
<ws2:longPathAware>true</ws2:longPathAware>
</windowsSettings>
</application>
<trustInfo xmlns="urn:schemas-microsoft-com:asm.v3">
<security>
<requestedPrivileges>
<requestedExecutionLevel level="asInvoker" uiAccess="false"/>
</requestedPrivileges>
</security>
</trustInfo>
</assembly>
44 changes: 44 additions & 0 deletions cpp/src/arrow/util/io_util_test.rc
@@ -0,0 +1,44 @@
// 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.

#ifndef CREATEPROCESS_MANIFEST_RESOURCE_ID
#define CREATEPROCESS_MANIFEST_RESOURCE_ID 1
#endif
#ifndef RT_MANIFEST
#define RT_MANIFEST 24
#endif

CREATEPROCESS_MANIFEST_RESOURCE_ID RT_MANIFEST
BEGIN
"<?xml version=""1.0"" encoding=""UTF-8"" standalone=""yes""?>"
"<assembly xmlns=""urn:schemas-microsoft-com:asm.v1"" manifestVersion=""1.0"">"
"<assemblyIdentity type=""win32"" name=""ArrowUtilityTest"" version=""1.1.1.1""/>"
"<application xmlns=""urn:schemas-microsoft-com:asm.v3"">"
"<windowsSettings xmlns:ws2=""http://schemas.microsoft.com/SMI/2016/WindowsSettings"">"
"<ws2:longPathAware>true</ws2:longPathAware>"
"</windowsSettings>"
"</application>"
"<trustInfo xmlns=""urn:schemas-microsoft-com:asm.v3"">"
"<security>"
"<requestedPrivileges>"
"<requestedExecutionLevel level=""asInvoker"" uiAccess=""false""/>"
"</requestedPrivileges>"
"</security>"
"</trustInfo>"
"</assembly>"
END

0 comments on commit 1b71ca7

Please sign in to comment.