I did not get any response to my last questions, so I hope this patch is
enough.
Any thought about my other arguments?
Federico
On 5/17/20 7:54 PM, Federico Kircheis wrote:
Thank you for the feedback.
This patch is clearly not limited to the protection of data, as it
quotes variables that could in no way contain a space or have anything
to do with file paths.
Could you please point me to which variables are unrelated to files.
AFAIK i quoted files and arguments, which can all contain whitespace.
For example I did quote ${unpack_file_path}, but not ${unpack_cmd}.
As mentioned multiple times, using filenames
ore directories with spaces is asking for trouble, and I have no
interest in trying to support such a case.
The first commit makes sure that no information is lost while processing
file with spaces or other characters that cause globbing. This prevents
writing or modifying the wrong files, which is what happened to me.
The second commit add exit in case `cd` fails, which prevents other
errors afterwards.
Those modification do not add support for path with whitespace, as I was
still unable to compile the software, they did however prevent cygport
to delete unrelated data (or create data in the wrong location).
I'm willing to consider a
*limited* patch that makes sure that cygport doesn't do something it
shouldn't in that case, but that's about it.
Also because if the underlying tool like `make` does not support spaces,
it has no benefit.
The most minimal patch I can imagine is exiting if `cd` fails (just the
second commit).
Would you accept that?
But please also consider my other arguments.
Yaakov
PS:
A "nice" side-effect to quoting most variables that could contain white
space is that static-analyzers like shellcheck will emit less
diagnostic, making it easier to discover potential errors.
>From 9dec371efa2f4f943bdd660618a0e1d91b6cfb4a Mon Sep 17 00:00:00 2001
From: Federico Kircheis <federico.kirch...@gmail.com>
Date: Tue, 2 Jul 2019 21:02:36 +0200
Subject: [PATCH] Exit in case `cd` fails
---
lib/src_fetch.cygpart | 2 +-
lib/src_prep.cygpart | 14 +++++++-------
2 files changed, 8 insertions(+), 8 deletions(-)
diff --git a/lib/src_fetch.cygpart b/lib/src_fetch.cygpart
index a273045..acea3a6 100644
--- a/lib/src_fetch.cygpart
+++ b/lib/src_fetch.cygpart
@@ -156,7 +156,7 @@ __src_fetch() {
done
# the RCS_fetch functions change PWD
- cd ${top};
+ cd ${top} || error "Unable to cd to ${top}"
for uri in ${SRC_URI} ${PATCH_URI}
do
diff --git a/lib/src_prep.cygpart b/lib/src_prep.cygpart
index 80ba8d5..fb99bfd 100644
--- a/lib/src_prep.cygpart
+++ b/lib/src_prep.cygpart
@@ -189,7 +189,7 @@ __gpg_verify() {
}
__mkdirs() {
- cd ${top};
+ cd ${top} || error "Unable to cd to ${top}";
mkdir -p ${srcdir} ${origsrcdir} ${B} ${D} ${T} ${configdir} ${logdir} ${distdir} ${patchdir} ${spkgdir};
}
@@ -286,7 +286,7 @@ __src_prep() {
local tar_patch;
local n=1;
- cd ${top};
+ cd ${top} || error "Unable to cd to ${top}";
__mkdirs;
@@ -345,7 +345,7 @@ __src_prep() {
__gpg_verify ${top}/${src_patchfile} "SOURCE PATCH";
fi
- cd ${origsrcdir};
+ cd ${origsrcdir} || error "Unable to cd to ${origsrcdir}";
for src_pkg in ${_src_orig_pkgs}
do
@@ -377,7 +377,7 @@ __src_prep() {
# cd will fail if not executable (e.g. dot2tex)
chmod +x ${origsrcdir}/${SRC_DIR};
- cd ${origsrcdir}/${SRC_DIR};
+ cd ${origsrcdir}/${SRC_DIR} || error "Unable to cd to ${origsrcdir}/${SRC_DIR}";
#****v* Preparation/DISTCLEANFILES
# DESCRIPTION
@@ -404,7 +404,7 @@ __src_prep() {
if __check_function src_unpack_hook
then
__check_unstable src_unpack_hook;
- cd ${origsrcdir}/${SRC_DIR};
+ cd ${origsrcdir}/${SRC_DIR} | error "Unable to cd to ${origsrcdir}/${SRC_DIR}";
fi
for src_patch in ${_src_orig_patches}
@@ -446,7 +446,7 @@ __src_prep() {
if __check_function src_patch_hook
then
__check_unstable src_patch_hook;
- cd ${origsrcdir}/${SRC_DIR};
+ cd ${origsrcdir}/${SRC_DIR} || error "Unable to cd to ${origsrcdir}/${SRC_DIR}";
fi
__step "Preparing working source directory";
@@ -456,7 +456,7 @@ __src_prep() {
mkdir -p ${C};
ln -sfn ${C} ${workdir}/CYGWIN-PATCHES;
- cd ${S};
+ cd ${S} || error "Unable to cd to ${S}";
if [ -f ${top}/${cygwin_patchfile} ]
then
--
2.26.2
