Re: cygport development

[Federico Kircheis <federico.kircheis@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 1961 bytes --]

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


[-- Attachment #2: path-with-spaces.patch --]
[-- Type: text/x-patch, Size: 2467 bytes --]

From 9dec371efa2f4f943bdd660618a0e1d91b6cfb4a Mon Sep 17 00:00:00 2001
From: Federico Kircheis <federico.kircheis@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