diff options
author | Michal Domonkos <mdomonko@redhat.com> | 2022-11-21 17:36:21 +0100 |
---|---|---|
committer | Panu Matilainen <pmatilai@redhat.com> | 2022-11-29 11:58:03 +0200 |
commit | a06374be2f85e457cb526edb8ead40f57c82bfd6 (patch) | |
tree | 9442b96ffabdb91151d10d171e6a35d41de2b31e /build | |
parent | 6a950a3a56a86a2ed8825efe69c8fdd74721694b (diff) | |
download | rpm-a06374be2f85e457cb526edb8ead40f57c82bfd6.tar.gz |
Glob special files only once, internally
Currently, special files are globbed twice during build. When copying
them from builddir to buildroot using the %doc/%license script, we let
the shell handle the globbing as we pass the patterns verbatim to cp(1).
Then, when adding the copied files from buildroot to the internal file
list, we use rpmGlob() to expand the same patterns in builddir again.
This is not only redundant but also potentially error prone as it
assumes both shell globbing and rpmGlob() will yield the same results.
And most importantly, it means we can't just quote the cp commands to
allow the use of shell control chars like '&' in filenames. Selectively
escaping those would be possible but still too brittle.
To fix all that, just use rpmGlob() first and then pass the matches one
by one to cp, all quoted.
A side effect of this change is that there will be one cp command per
match, as opposed to one cp command per file entry with matches as
arguments, and the paths will be absolute. This will make build logs a
tiny bit more verbose but also more explicit and safe from hitting the
maximum number of arguments in a single shell command (although that's
more of a theoretical scenario, not likely with real packages).
Fixes: #1294
Diffstat (limited to 'build')
-rw-r--r-- | build/files.c | 55 |
1 files changed, 29 insertions, 26 deletions
diff --git a/build/files.c b/build/files.c index aab128aaa..cc9c5993d 100644 --- a/build/files.c +++ b/build/files.c @@ -2410,7 +2410,9 @@ static void processSpecialDir(rpmSpec spec, Package pkg, FileList fl, const char *sdname = (sd->sdtype == RPMFILE_DOC) ? "%doc" : "%license"; char *mkdocdir = rpmExpand("%{__mkdir_p} $", sdenv, NULL); StringBuf docScript = newStringBuf(); - char *basepath, **files; + char *basepath; + ARGV_t *files; + int count = sd->entriesCount; int fi; appendStringBuf(docScript, sdenv); @@ -2421,16 +2423,27 @@ static void processSpecialDir(rpmSpec spec, Package pkg, FileList fl, appendLineStringBuf(docScript, sdenv); appendLineStringBuf(docScript, mkdocdir); + basepath = rpmGenPath(spec->rootDir, "%{_builddir}", "%{?buildsubdir}"); + files = xmalloc(sizeof(*files) * count); + fi = 0; for (ARGV_const_t fn = sd->files; fn && *fn; fn++) { - /* Quotes would break globs, escape spaces instead */ - char *efn = rpmEscapeSpaces(*fn); - appendStringBuf(docScript, "cp -pr "); - appendStringBuf(docScript, efn); - appendStringBuf(docScript, " $"); - appendStringBuf(docScript, sdenv); - appendLineStringBuf(docScript, " ||:"); - free(efn); + char *origfile = rpmCleanPath(rstrscat(NULL, basepath, "/", *fn, NULL)); + ARGV_t globFiles = NULL; + int globFilesCount, i; + if (rpmGlobPath(origfile, RPMGLOB_NOCHECK, + &globFilesCount, &globFiles) == 0) { + for (i = 0; i < globFilesCount; i++) { + appendStringBuf(docScript, "cp -pr '"); + appendStringBuf(docScript, globFiles[i]); + appendStringBuf(docScript, "' $"); + appendStringBuf(docScript, sdenv); + appendLineStringBuf(docScript, " ||:"); + } + } + free(origfile); + files[fi++] = globFiles; } + free(basepath); if (install) { if (doScript(spec, RPMBUILD_STRINGBUF, sdname, @@ -2439,13 +2452,8 @@ static void processSpecialDir(rpmSpec spec, Package pkg, FileList fl, } } - basepath = rpmGenPath(spec->rootDir, "%{_builddir}", "%{?buildsubdir}"); - files = sd->files; fi = 0; - while (*files != NULL) { - char *origfile = rpmCleanPath(rstrscat(NULL, basepath, "/", *files, NULL)); - ARGV_t globFiles = NULL; - int globFilesCount, i; + for (int i = 0; i < count; i++) { char *newfile; FileEntryFree(&fl->cur); @@ -2454,19 +2462,14 @@ static void processSpecialDir(rpmSpec spec, Package pkg, FileList fl, copyFileEntry(&sd->entries[fi].defEntry, &fl->def); fi++; - if (rpmGlobPath(origfile, RPMGLOB_NOCHECK, - &globFilesCount, &globFiles) == 0) { - for (i = 0; i < globFilesCount; i++) { - rasprintf(&newfile, "%s/%s", sd->dirname, basename(globFiles[i])); - processBinaryFile(pkg, fl, newfile, 0); - free(newfile); - } - argvFree(globFiles); + for (ARGV_const_t fn = files[i]; fn && *fn; fn++) { + rasprintf(&newfile, "%s/%s", sd->dirname, basename(*fn)); + processBinaryFile(pkg, fl, newfile, 0); + free(newfile); } - free(origfile); - files++; + argvFree(files[i]); } - free(basepath); + free(files); FileEntryFree(&fl->cur); FileEntryFree(&fl->def); |