Skip to content

Commit 3bd178b

Browse files
rouaultrouault
authored
Merge pull request #14099 from rouault/fix_14097
gdal dataset copy/rename: make it work with vector datasets, and directories
2 parents 4b0e3b3 + 6d3c4db commit 3bd178b

File tree

6 files changed

+164
-35
lines changed

6 files changed

+164
-35
lines changed

‎autotest/utilities/test_gdalalg_dataset_copy.py‎

Lines changed: 30 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,7 @@ def test_gdalalg_dataset_copy(tmp_vsimem):
5454
assert ds.GetDriver().GetDescription() == "GTiff"
5555

5656

57-
def test_gdalalg_dataset_overwrite_existing_directory(tmp_vsimem):
57+
def test_gdalalg_dataset_copy_overwrite_existing_directory(tmp_vsimem):
5858

5959
gdal.Mkdir(tmp_vsimem / "my_dir", 0o755)
6060

@@ -72,7 +72,7 @@ def test_gdalalg_dataset_overwrite_existing_directory(tmp_vsimem):
7272

7373

7474
@pytest.mark.require_driver("OpenFileGDB")
75-
def test_gdalalg_dataset_overwrite_existing_dataset_directory(tmp_vsimem):
75+
def test_gdalalg_dataset_copy_overwrite_existing_dataset_directory(tmp_vsimem):
7676

7777
gdal.GetDriverByName("OpenFileGDB").CreateVector(tmp_vsimem / "out.gdb")
7878

@@ -111,3 +111,31 @@ def test_gdalalg_dataset_copy_complete():
111111
out = gdaltest.runexternal(f"{gdal_path} completion gdal dataset copy --format=")
112112
assert "GTiff " in out
113113
assert "ESRI\\ Shapefile " in out
114+
115+
116+
def test_gdalalg_dataset_copy_shapefile_dir(tmp_vsimem):
117+
118+
gdal.alg.vector.convert(
119+
input="../ogr/data/poly.shp",
120+
output=tmp_vsimem / "in_dir",
121+
output_format="ESRI Shapefile",
122+
)
123+
gdal.alg.dataset.copy(
124+
source=tmp_vsimem / "in_dir", destination=tmp_vsimem / "out_dir"
125+
)
126+
assert set(gdal.ReadDir(tmp_vsimem / "out_dir")) == set(
127+
["poly.shp", "poly.dbf", "poly.shx", "poly.prj"]
128+
)
129+
130+
131+
def test_gdalalg_dataset_copy_shapefile_dir_error(tmp_vsimem):
132+
133+
gdal.alg.vector.convert(
134+
input="../ogr/data/poly.shp",
135+
output=tmp_vsimem / "in_dir",
136+
output_format="ESRI Shapefile",
137+
)
138+
with pytest.raises(Exception, match="Cannot create directory"):
139+
gdal.alg.dataset.copy(
140+
source=tmp_vsimem / "in_dir", destination="/vsisubfile/out_dir"
141+
)

‎autotest/utilities/test_gdalalg_dataset_rename.py‎

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,3 +44,32 @@ def test_gdalalg_dataset_rename_error(tmp_vsimem):
4444
source="/i_do/not/exist.tif",
4545
destination=tmp_vsimem / "out.tif",
4646
)
47+
48+
49+
def test_gdalalg_dataset_rename_shapefile_dir(tmp_vsimem):
50+
51+
gdal.alg.vector.convert(
52+
input="../ogr/data/poly.shp",
53+
output=tmp_vsimem / "in_dir",
54+
output_format="ESRI Shapefile",
55+
)
56+
gdal.alg.dataset.rename(
57+
source=tmp_vsimem / "in_dir", destination=tmp_vsimem / "out_dir"
58+
)
59+
assert gdal.ReadDir(tmp_vsimem / "in_dir") is None
60+
assert set(gdal.ReadDir(tmp_vsimem / "out_dir")) == set(
61+
["poly.shp", "poly.dbf", "poly.shx", "poly.prj"]
62+
)
63+
64+
65+
def test_gdalalg_dataset_rename_shapefile_dir_error(tmp_vsimem):
66+
67+
gdal.alg.vector.convert(
68+
input="../ogr/data/poly.shp",
69+
output=tmp_vsimem / "in_dir",
70+
output_format="ESRI Shapefile",
71+
)
72+
with pytest.raises(Exception, match="Cannot create directory"):
73+
gdal.alg.dataset.rename(
74+
source=tmp_vsimem / "in_dir", destination="/vsisubfile/out_dir"
75+
)

‎gcore/gdaldriver.cpp‎

Lines changed: 65 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -1805,9 +1805,11 @@ CPLErr GDALDriver::DefaultRename(const char *pszNewName, const char *pszOldName)
18051805
/* -------------------------------------------------------------------- */
18061806
/* Collect file list. */
18071807
/* -------------------------------------------------------------------- */
1808-
GDALDatasetH hDS = GDALOpen(pszOldName, GA_ReadOnly);
1808+
auto poDS = std::unique_ptr<GDALDataset>(
1809+
GDALDataset::Open(pszOldName, GDAL_OF_ALL | GDAL_OF_VERBOSE_ERROR,
1810+
nullptr, nullptr, nullptr));
18091811

1810-
if (hDS == nullptr)
1812+
if (!poDS)
18111813
{
18121814
if (CPLGetLastErrorNo() == 0)
18131815
CPLError(CE_Failure, CPLE_OpenFailed,
@@ -1816,11 +1818,11 @@ CPLErr GDALDriver::DefaultRename(const char *pszNewName, const char *pszOldName)
18161818
return CE_Failure;
18171819
}
18181820

1819-
char **papszFileList = GDALGetFileList(hDS);
1821+
const CPLStringList aosFileList(poDS->GetFileList());
18201822

1821-
GDALClose(hDS);
1823+
poDS.reset();
18221824

1823-
if (CSLCount(papszFileList) == 0)
1825+
if (aosFileList.empty())
18241826
{
18251827
CPLError(CE_Failure, CPLE_NotSupported,
18261828
"Unable to determine files associated with %s,\n"
@@ -1835,31 +1837,44 @@ CPLErr GDALDriver::DefaultRename(const char *pszNewName, const char *pszOldName)
18351837
/* names. */
18361838
/* -------------------------------------------------------------------- */
18371839
CPLErr eErr = CE_None;
1838-
char **papszNewFileList =
1839-
CPLCorrespondingPaths(pszOldName, pszNewName, papszFileList);
1840+
const CPLStringList aosNewFileList(
1841+
CPLCorrespondingPaths(pszOldName, pszNewName, aosFileList.List()));
18401842

1841-
if (papszNewFileList == nullptr)
1843+
if (aosNewFileList.empty())
18421844
return CE_Failure;
18431845

1844-
for (int i = 0; papszFileList[i] != nullptr; ++i)
1846+
// Guaranteed by CPLCorrespondingPaths()
1847+
CPLAssert(aosNewFileList.size() == aosFileList.size());
1848+
1849+
VSIStatBufL sStatBuf;
1850+
if (VSIStatL(pszOldName, &sStatBuf) == 0 && VSI_ISDIR(sStatBuf.st_mode) &&
1851+
VSIStatL(pszNewName, &sStatBuf) != 0)
18451852
{
1846-
if (CPLMoveFile(papszNewFileList[i], papszFileList[i]) != 0)
1853+
if (VSIMkdirRecursive(pszNewName, 0755) != 0)
18471854
{
1855+
CPLError(CE_Failure, CPLE_AppDefined,
1856+
"Cannot create directory '%s'", pszNewName);
1857+
return CE_Failure;
1858+
}
1859+
}
1860+
1861+
for (int i = 0; i < aosFileList.size(); ++i)
1862+
{
1863+
if (CPLMoveFile(aosNewFileList[i], aosFileList[i]) != 0)
1864+
{
1865+
// Above method will have emitted an error in case of failure.
18481866
eErr = CE_Failure;
18491867
// Try to put the ones we moved back.
18501868
for (--i; i >= 0; i--)
18511869
{
18521870
// Nothing we can do if the moving back doesn't work...
18531871
CPL_IGNORE_RET_VAL(
1854-
CPLMoveFile(papszFileList[i], papszNewFileList[i]));
1872+
CPLMoveFile(aosFileList[i], aosNewFileList[i]));
18551873
}
18561874
break;
18571875
}
18581876
}
18591877

1860-
CSLDestroy(papszNewFileList);
1861-
CSLDestroy(papszFileList);
1862-
18631878
return eErr;
18641879
}
18651880

@@ -1939,9 +1954,11 @@ CPLErr GDALDriver::DefaultCopyFiles(const char *pszNewName,
19391954
/* -------------------------------------------------------------------- */
19401955
/* Collect file list. */
19411956
/* -------------------------------------------------------------------- */
1942-
GDALDatasetH hDS = GDALOpen(pszOldName, GA_ReadOnly);
1957+
auto poDS = std::unique_ptr<GDALDataset>(
1958+
GDALDataset::Open(pszOldName, GDAL_OF_ALL | GDAL_OF_VERBOSE_ERROR,
1959+
nullptr, nullptr, nullptr));
19431960

1944-
if (hDS == nullptr)
1961+
if (!poDS)
19451962
{
19461963
if (CPLGetLastErrorNo() == 0)
19471964
CPLError(CE_Failure, CPLE_OpenFailed,
@@ -1950,16 +1967,15 @@ CPLErr GDALDriver::DefaultCopyFiles(const char *pszNewName,
19501967
return CE_Failure;
19511968
}
19521969

1953-
char **papszFileList = GDALGetFileList(hDS);
1970+
const CPLStringList aosFileList(poDS->GetFileList());
19541971

1955-
GDALClose(hDS);
1956-
hDS = nullptr;
1972+
poDS.reset();
19571973

1958-
if (CSLCount(papszFileList) == 0)
1974+
if (aosFileList.empty())
19591975
{
19601976
CPLError(CE_Failure, CPLE_NotSupported,
19611977
"Unable to determine files associated with %s,\n"
1962-
"rename fails.",
1978+
"copy fails.",
19631979
pszOldName);
19641980

19651981
return CE_Failure;
@@ -1970,27 +1986,46 @@ CPLErr GDALDriver::DefaultCopyFiles(const char *pszNewName,
19701986
/* names. */
19711987
/* -------------------------------------------------------------------- */
19721988
CPLErr eErr = CE_None;
1973-
char **papszNewFileList =
1974-
CPLCorrespondingPaths(pszOldName, pszNewName, papszFileList);
1989+
const CPLStringList aosNewFileList(
1990+
CPLCorrespondingPaths(pszOldName, pszNewName, aosFileList.List()));
19751991

1976-
if (papszNewFileList == nullptr)
1992+
if (aosNewFileList.empty())
19771993
return CE_Failure;
19781994

1979-
for (int i = 0; papszFileList[i] != nullptr; ++i)
1995+
// Guaranteed by CPLCorrespondingPaths()
1996+
CPLAssert(aosNewFileList.size() == aosFileList.size());
1997+
1998+
VSIStatBufL sStatBuf;
1999+
if (VSIStatL(pszOldName, &sStatBuf) == 0 && VSI_ISDIR(sStatBuf.st_mode) &&
2000+
VSIStatL(pszNewName, &sStatBuf) != 0)
19802001
{
1981-
if (CPLCopyFile(papszNewFileList[i], papszFileList[i]) != 0)
2002+
if (VSIMkdirRecursive(pszNewName, 0755) != 0)
19822003
{
2004+
CPLError(CE_Failure, CPLE_AppDefined,
2005+
"Cannot create directory '%s'", pszNewName);
2006+
return CE_Failure;
2007+
}
2008+
}
2009+
2010+
for (int i = 0; i < aosFileList.size(); ++i)
2011+
{
2012+
if (CPLCopyFile(aosNewFileList[i], aosFileList[i]) != 0)
2013+
{
2014+
// Above method will have emitted an error in case of failure.
19832015
eErr = CE_Failure;
19842016
// Try to put the ones we moved back.
19852017
for (--i; i >= 0; --i)
1986-
VSIUnlink(papszNewFileList[i]);
2018+
{
2019+
if (VSIUnlink(aosNewFileList[i]) != 0)
2020+
{
2021+
CPLError(CE_Warning, CPLE_AppDefined, "Cannot delete '%s'",
2022+
aosNewFileList[i]);
2023+
}
2024+
}
19872025
break;
19882026
}
19892027
}
19902028

1991-
CSLDestroy(papszNewFileList);
1992-
CSLDestroy(papszFileList);
1993-
19942029
return eErr;
19952030
}
19962031

‎port/cpl_conv.cpp‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3345,7 +3345,13 @@ int CPLMoveFile(const char *pszNewPath, const char *pszOldPath)
33453345
const int nRet = CPLCopyFile(pszNewPath, pszOldPath);
33463346

33473347
if (nRet == 0)
3348-
VSIUnlink(pszOldPath);
3348+
{
3349+
if (VSIUnlink(pszOldPath) != 0)
3350+
{
3351+
CPLError(CE_Warning, CPLE_AppDefined, "Cannot delete '%s'",
3352+
pszOldPath);
3353+
}
3354+
}
33493355
return nRet;
33503356
}
33513357

‎port/cpl_conv.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -212,7 +212,7 @@ const char CPL_DLL *CPLExtractRelativePath(const char *, const char *, int *)
212212
CPL_WARN_UNUSED_RESULT CPL_RETURNS_NONNULL;
213213
char CPL_DLL **
214214
CPLCorrespondingPaths(const char *pszOldFilename, const char *pszNewFilename,
215-
char **papszFileList) CPL_WARN_UNUSED_RESULT;
215+
CSLConstList papszFileList) CPL_WARN_UNUSED_RESULT;
216216
int CPL_DLL CPLCheckForFile(char *pszFilename, CSLConstList papszSiblingList);
217217

218218
const char CPL_DLL *CPLGetHomeDir(void) CPL_WARN_UNUSED_RESULT;

‎port/cpl_path.cpp‎

Lines changed: 32 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@
2727

2828
#include <algorithm>
2929
#include <string>
30+
#include <string_view>
3031

3132
#include "cpl_atomic_ops.h"
3233
#include "cpl_config.h"
@@ -1314,12 +1315,42 @@ const char *CPLCleanTrailingSlash(const char *pszPath)
13141315
*/
13151316

13161317
char **CPLCorrespondingPaths(const char *pszOldFilename,
1317-
const char *pszNewFilename, char **papszFileList)
1318+
const char *pszNewFilename,
1319+
CSLConstList papszFileList)
13181320

13191321
{
13201322
if (CSLCount(papszFileList) == 0)
13211323
return nullptr;
13221324

1325+
VSIStatBufL sStatBuf;
1326+
if (VSIStatL(pszOldFilename, &sStatBuf) == 0 && VSI_ISDIR(sStatBuf.st_mode))
1327+
{
1328+
CPLStringList aosNewList;
1329+
std::string_view svOldFilename(pszOldFilename);
1330+
for (int i = 0; papszFileList[i] != nullptr; i++)
1331+
{
1332+
if (cpl::starts_with(std::string_view(papszFileList[i]),
1333+
svOldFilename) &&
1334+
(papszFileList[i][svOldFilename.size()] == '/' ||
1335+
papszFileList[i][svOldFilename.size()] == '\\'))
1336+
{
1337+
// If the old file list contains entries like oldpath/filename,
1338+
// generate newpath/filename
1339+
aosNewList.push_back(CPLFormFilenameSafe(
1340+
pszNewFilename, papszFileList[i] + svOldFilename.size() + 1,
1341+
nullptr));
1342+
}
1343+
else
1344+
{
1345+
CPLError(CE_Failure, CPLE_AppDefined,
1346+
"Unable to copy/rename fileset due to unexpected "
1347+
"source filename.");
1348+
return nullptr;
1349+
}
1350+
}
1351+
return aosNewList.StealList();
1352+
}
1353+
13231354
/* -------------------------------------------------------------------- */
13241355
/* There is a special case for a one item list which exactly */
13251356
/* matches the old name, to rename to the new name. */

0 commit comments

Comments
 (0)