public inbox for cygwin-apps@cygwin.com
 help / color / mirror / Atom feed
* cygport development
@ 2019-07-14 13:25 Federico Kircheis
  2019-07-14 17:11 ` Brian Inglis
  0 siblings, 1 reply; 15+ messages in thread
From: Federico Kircheis @ 2019-07-14 13:25 UTC (permalink / raw)
  To: cygwin

Hello,

I had the unfortunate idea to execute cygport in a directory that had in 
it's path at least one whitespace (it's not that uncommon under Windows).

Cygport did not report a clear error, and created files at the wrong 
location.


I've posted a patch on github a couple of weeks ago (see 
https://github.com/cygwinports/cygport/pull/12), but I did not get any 
feedback.

Does the development of cygport take place on github?
Should I post my patches somewhere else in order to get them integrated 
in cygport?

Best regards

Federico Kircheis

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: cygport development
  2019-07-14 13:25 cygport development Federico Kircheis
@ 2019-07-14 17:11 ` Brian Inglis
  2019-07-14 19:39   ` [PATCH 1/2] Add support for path with spaces Federico Kircheis
  2019-09-28 11:56   ` cygport development Federico Kircheis
  0 siblings, 2 replies; 15+ messages in thread
From: Brian Inglis @ 2019-07-14 17:11 UTC (permalink / raw)
  To: cygwin-apps

On 2019-07-14 07:25, Federico Kircheis wrote:
> I had the unfortunate idea to execute cygport in a directory that had in it's
> path at least one whitespace (it's not that uncommon under Windows).
> Cygport did not report a clear error, and created files at the wrong location.
> I've posted a patch on github a couple of weeks ago (see
> https://github.com/cygwinports/cygport/pull/12), but I did not get any feedback.
> Does the development of cygport take place on github?
> Should I post my patches somewhere else in order to get them integrated in cygport?

Cygport is a regular Cygwin app used by package maintainers.
Send here and attach them as text with a git send-email subject and cover letter
summarising the change and impact (as to cygwin-patches).

-- 
Take care. Thanks, Brian Inglis, Calgary, Alberta, Canada

This email may be disturbing to some readers as it contains
too much technical detail. Reader discretion is advised.

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH 1/2] Add support for path with spaces
  2019-07-14 17:11 ` Brian Inglis
@ 2019-07-14 19:39   ` Federico Kircheis
  2019-07-14 19:39     ` [PATCH 2/2] Exit in case `cd` fails Federico Kircheis
  2019-09-28 11:56   ` cygport development Federico Kircheis
  1 sibling, 1 reply; 15+ messages in thread
From: Federico Kircheis @ 2019-07-14 19:39 UTC (permalink / raw)
  To: cygwin-apps; +Cc: Federico Kircheis

Quote most variables

Otherwise path with spaces or other special characters are interpreted 
incorrectly
---
 bin/cygport.in              |  74 +++++++++++++-------------
 lib/config_registry.cygpart |   8 +--
 lib/src_compile.cygpart     |   2 +-
 lib/src_fetch.cygpart       |  30 +++++------
 lib/src_prep.cygpart        | 102 ++++++++++++++++++------------------
 lib/syntax.cygpart          |  10 ++--
 6 files changed, 113 insertions(+), 113 deletions(-)

diff --git a/bin/cygport.in b/bin/cygport.in
index a5989f8..3a1ae51 100755
--- a/bin/cygport.in
+++ b/bin/cygport.in
@@ -42,7 +42,7 @@ declare -r _privsysconfdir=@sysconfdir@;
 
 
 ### import defined, pushd, popd
-source ${_privlibdir}/syntax.cygpart
+source "${_privlibdir}/syntax.cygpart"
 ###
 
 
@@ -166,7 +166,7 @@ source ${_privlibdir}/help.cygpart
 # Accept --help and --version arguments without specifying a cygport file
 while true
 do
-	case ${1} in
+	case "${1}" in
 	--help|-h|-\?)
 		__show_help;
 		exit 0;
@@ -204,7 +204,7 @@ do
 	esac
 done
 
-declare -ar argv=(${0} ${@})
+declare -ar argv=(${0} "${@}")
 declare -ir argc=$(( $# + 1 ))
 
 # Show help if no commands are given
@@ -222,7 +222,7 @@ fi
 ################################################################################
 
 ### import check_prog and friends
-source ${_privlibdir}/check_funcs.cygpart
+source "${_privlibdir}/check_funcs.cygpart"
 ###
 
 # check now for all mandatory programs
@@ -349,11 +349,11 @@ unset _autotools_CYGCLASS_ _autotools_CYGCLASS_stage1_
 ################################################################################
 
 unset NAME VERSION RELEASE
-if [ -f ${argv[1]} ]
+if [ -f "${argv[1]}" ]
 then
-	eval $(grep '^NAME=' ${argv[1]})
-	eval $(grep '^VERSION=' ${argv[1]})
-	eval $(grep '^RELEASE=' ${argv[1]})
+	eval "$(grep '^NAME=' "${argv[1]}")"
+	eval "$(grep '^VERSION=' "${argv[1]}")"
+	eval "$(grep '^RELEASE=' "${argv[1]}")"
 fi
 
 if [ "${NAME+y}${VERSION+y}${RELEASE+y}" = "yyy" ]
@@ -371,7 +371,7 @@ declare -r  PN=${PF%%-[0-9]*};
 declare     NAME=${PN}
 declare -r  PR=${PF##*-};
 declare     RELEASE=${PR}
-            PV=$(echo ${PF} | sed -e "s/${PN}\-\(.*\)\-${PR}$/\1/");
+            PV=$(echo "${PF}" | sed -e "s/${PN}\-\(.*\)\-${PR}$/\1/");
 declare     VERSION=${PV}
 declare -r  cygportfile=${PF}.cygport;
 fi
@@ -395,7 +395,7 @@ _topdir=${argv[1]%/*};
 
 if [ "x${_topdir}" = "x${argv[1]}" ]
 then
-	if [ -f ./${cygportfile} ]
+	if [ -f "./${cygportfile}" ]
 	then
 		_topdir=.;
 	else
@@ -406,13 +406,13 @@ fi
 declare -r top=$(cd ${_topdir}; pwd);
 unset _topdir;
 
-if [ ! -e ${top}/${cygportfile} ]
+if [ ! -e "${top}/${cygportfile}" ]
 then
 	error "${cygportfile} not found.";
 fi
 
 ### load .cygport
-source ${top}/${cygportfile} || error "could not read ${cygportfile}"
+source "${top}/${cygportfile}" || error "could not read ${cygportfile}"
 ###
 
 case ${ARCH} in
@@ -422,7 +422,7 @@ esac
 
 if defined CYGPORT_DEPEND
 then
-	if ! __version_at_least ${CYGPORT_DEPEND} ${_cygport_version}
+	if ! __version_at_least "${CYGPORT_DEPEND}" "${_cygport_version}"
 	then
 		error "This package requires cygport ${CYGPORT_DEPEND} or newer";
 	fi
@@ -485,7 +485,7 @@ declare -r uploadlog="${logdir}/${PF}-upload.log";
 
 for _src_uri in ${SRC_URI}
 do
-	if [ -f ${top}/${_src_uri} ]
+	if [ -f "${top}/${_src_uri}" ]
 	then
 		_src_orig_pkgs+=" ${_src_uri}";
 		continue;
@@ -499,7 +499,7 @@ unset _src_uri;
 
 for _patch_uri in ${PATCH_URI}
 do
-	if [ -f ${top}/${_patch_uri} ]
+	if [ -f "${top}/${_patch_uri}" ]
 	then
 		_src_orig_patches+=" ${_patch_uri}";
 		continue;
@@ -511,8 +511,8 @@ done
 readonly _src_orig_patches;
 unset _patch_uri;
 
-declare -r cygwin_patchfile=${PF}.cygwin.patch;
-declare -r src_patchfile=${PF}.src.patch;
+declare -r cygwin_patchfile="${PF}.cygwin.patch";
+declare -r src_patchfile="${PF}.src.patch";
 
 declare -ar pkg_name=(${PKG_NAMES:-${PN}});
 declare -r  pkg_count=${#pkg_name[*]};
@@ -560,21 +560,21 @@ do
 			;;
 		compile|build|make)
 			__stage Compiling;
-			__log_init ${compilelog};
+			__log_init "${compilelog}";
 			__check_depends && \
-			src_compile 2>&1 | tee -a ${compilelog};
+			src_compile 2>&1 | tee -a "${compilelog}";
 			_status=${PIPESTATUS[0]};
 			;;
 		check|test)
 			__stage Testing;
-			__log_init ${checklog};
-			src_test 2>&1 | tee -a ${checklog};
+			__log_init "${checklog}";
+			src_test 2>&1 | tee -a "${checklog}";
 			_status=${PIPESTATUS[0]};
 			;;
 		inst*)
 			__stage Installing;
-			__log_init ${installlog};
-			(__prepinstalldirs && src_install && __src_postinst) 2>&1 | tee -a ${installlog};
+			__log_init "${installlog}";
+			(__prepinstalldirs && src_install && __src_postinst) 2>&1 | tee -a "${installlog}";
 			_status=${PIPESTATUS[0]};
 			;;
 		postinst*)
@@ -606,8 +606,8 @@ do
 			;&
 		package|pkg)
 			__stage "Packaging${_pkg_tag:+ ${_pkg_tag%:} release}";
-			__log_init ${pkglog};
-			(__pkg_binpkg && __pkg_pkgcheck && __pkg_srcpkg && __pkg_dist ${_pkg_tag}) 2>&1 | tee -a ${pkglog};
+			__log_init "${pkglog}";
+			(__pkg_binpkg && __pkg_pkgcheck && __pkg_srcpkg && __pkg_dist "${_pkg_tag}") 2>&1 | tee -a "${pkglog}";
 			_status=${PIPESTATUS[0]};
 			;;
 		diff|mkdiff|mkpatch)
@@ -616,14 +616,14 @@ do
 			;;
 		upload|up)
 			__stage Uploading;
-			__log_init ${uploadlog};
-			(__pkg_upload full) 2>&1 | tee -a ${uploadlog};
+			__log_init "${uploadlog}";
+			(__pkg_upload full) 2>&1 | tee -a "${uploadlog}";
 			_status=${PIPESTATUS[0]};
 			;;
 		stage)
 			__stage Staging;
-			__log_init ${uploadlog};
-			(__pkg_upload stage) 2>&1 | tee -a ${uploadlog};
+			__log_init "${uploadlog}";
+			(__pkg_upload stage) 2>&1 | tee -a "${uploadlog}";
 			_status=${PIPESTATUS[0]};
 			;;
 		announce)
@@ -640,15 +640,15 @@ do
 			;&
 		almostall|all)
 			__stage Preparing && __src_prep && \
-			__log_init ${compilelog} && \
+			__log_init "${compilelog}" && \
 			__check_depends && \
-			__stage Compiling && src_compile 2>&1 | tee -a ${compilelog} && \
+			__stage Compiling && src_compile 2>&1 | tee -a "${compilelog}" && \
 			test ${PIPESTATUS[0]} -eq 0 && \
-			__log_init ${installlog} && \
-			__stage Installing && (__prepinstalldirs && src_install && __src_postinst) 2>&1 | tee -a ${installlog} && \
+			__log_init "${installlog}" && \
+			__stage Installing && (__prepinstalldirs && src_install && __src_postinst) 2>&1 | tee -a "${installlog}" && \
 			test ${PIPESTATUS[0]} -eq 0 && \
-			__log_init ${pkglog} && \
-			__stage Packaging && (__pkg_binpkg && __pkg_pkgcheck && __pkg_srcpkg && __pkg_dist ${_pkg_tag}) 2>&1 | tee -a ${pkglog} && \
+			__log_init "${pkglog}" && \
+			__stage Packaging && (__pkg_binpkg && __pkg_pkgcheck && __pkg_srcpkg && __pkg_dist "${_pkg_tag}") 2>&1 | tee -a "${pkglog}" && \
 			test ${PIPESTATUS[0]} -eq 0
 			_status=$?;
 			;;
@@ -665,9 +665,9 @@ do
 			exit 1;
 			;;
 		*)
-			if __check_function ${argv[${arg_n}]} && ! __check_function_ro ${argv[${arg_n}]}
+			if __check_function "${argv[${arg_n}]}" && ! __check_function_ro "${argv[${arg_n}]}"
 			then
-				${argv[${arg_n}]};
+				"${argv[${arg_n}]}";
 			else
 				error "unknown command ${argv[${arg_n}]}";
 			fi
diff --git a/lib/config_registry.cygpart b/lib/config_registry.cygpart
index d23c924..dadbd4f 100644
--- a/lib/config_registry.cygpart
+++ b/lib/config_registry.cygpart
@@ -21,16 +21,16 @@
 ################################################################################
 
 __config_get() {
-	if [ -f ${configdir}/${1} ]
+	if [ -f "${configdir}/${1}" ]
 	then
-		echo -n $(cat ${configdir}/${1});
+		echo -n "$(cat "${configdir}/${1}")";
 	else
 		echo -n "0";
 	fi
 }
 
 __config_equals() {
-	if [ -f ${configdir}/${1} ] && [ $(cat ${configdir}/${1}) = ${2} ]
+	if [ -f "${configdir}/${1}" ] && [ "$(cat "${configdir}/${1}")" = "${2}" ]
 	then
 		return 0;
 	else
@@ -39,7 +39,7 @@ __config_equals() {
 }
 
 __config_set() {
-	echo -n ${2} > ${configdir}/${1};
+	echo -n "${2}" > "${configdir}/${1}";
 }
 
 readonly -f __config_equals __config_get __config_set
diff --git a/lib/src_compile.cygpart b/lib/src_compile.cygpart
index 0ecd7b8..660b8af 100644
--- a/lib/src_compile.cygpart
+++ b/lib/src_compile.cygpart
@@ -49,7 +49,7 @@ lndirs() {
 	fi
 
 	check_prog_req lndir
-	lndir -silent ${fromdir} ${todir} || error "lndir failed"
+	lndir -silent "${fromdir}" "${todir}" || error "lndir failed"
 }
 
 #****C* Compiling/manifestize
diff --git a/lib/src_fetch.cygpart b/lib/src_fetch.cygpart
index 0208bfc..ed61d25 100644
--- a/lib/src_fetch.cygpart
+++ b/lib/src_fetch.cygpart
@@ -72,36 +72,36 @@ fetch() {
 	urifile=${urifile%\?*};
 	urifile=${urifile##*/};
 
-	if defined __DL_ONLY_MISSING && defined DISTDIR && [ -f ${DISTDIR}/${urifile} ]
+	if defined __DL_ONLY_MISSING && defined DISTDIR && [ -f "${DISTDIR}/${urifile}" ]
 	then
 		inform "Using ${urifile} from DISTDIR"
 		return 0
 	elif check_prog wget
 	then
-		if wget --no-check-certificate -O ${urifile}.tmp ${uri}
+		if wget --no-check-certificate -O "${urifile}.tmp" "${uri}"
 		then
-			mv -f ${urifile}.tmp ${urifile}
+			mv -f "${urifile}.tmp" "${urifile}"
 		else
-			rm -f ${urifile}.tmp
+			rm -f "${urifile}.tmp"
 			error "wget ${uri} failed"
 		fi
 	elif check_prog curl
 	then
-		if curl -k --url ${uri} -o ${urifile}.tmp
+		if curl -k --url "${uri}" -o "${urifile}.tmp"
 		then
-			mv -f ${urifile}.tmp ${urifile}
+			mv -f "${urifile}.tmp" "${urifile}"
 		else
-			rm -f ${urifile}.tmp
+			rm -f "${urifile}.tmp"
 			error "curl ${uri} failed"
 		fi
 	else
 		error "Either wget or curl are required to fetch sources.";
 	fi
 
-	if defined DISTDIR && [ -f ${urifile} ]
+	if defined DISTDIR && [ -f "${urifile}" ]
 	then
-		[ -d ${DISTDIR} ] || mkdir -p ${DISTDIR}
-		mv ${urifile} ${DISTDIR}/
+		[ -d "${DISTDIR}" ] || mkdir -p "${DISTDIR}"
+		mv "${urifile}" "${DISTDIR}/"
 	fi
 }
 
@@ -156,7 +156,7 @@ __src_fetch() {
 	done
 
 	# the RCS_fetch functions change PWD
-	cd ${top};
+	cd "${top}";
 
 	for uri in ${SRC_URI} ${PATCH_URI}
 	do
@@ -165,11 +165,11 @@ __src_fetch() {
 				continue
 		fi
 		case ${uri%%//*} in
-			mirror:)	__mirror_fetch ${uri} ;;
-			http:|https:|ftp:)	fetch ${uri} || error "Download ${uri##*/} failed" ;;
-			file:)		[ -f ${uri#file://} ] || error "${uri##*/}: does not exist" ;;
+			mirror:)	__mirror_fetch "${uri}" ;;
+			http:|https:|ftp:)	fetch "${uri}" || error "Download ${uri##*/} failed" ;;
+			file:)		[ -f "${uri#file://}" ] || error "${uri##*/}: does not exist" ;;
 			*:)		error "Unsupported download protocol ${uri%%//*}" ;;
-			*/*)		[ -f ${uri#file://} ] || error "${uri##*/}: does not exist" ;;
+			*/*)		[ -f "${uri#file://}" ] || error "${uri##*/}: does not exist" ;;
 			${uri})		;; # file in working directory
 			*)		error "Invalid download URI ${uri}" ;;
 		esac
diff --git a/lib/src_prep.cygpart b/lib/src_prep.cygpart
index 773f5fb..fb94c56 100644
--- a/lib/src_prep.cygpart
+++ b/lib/src_prep.cygpart
@@ -49,14 +49,14 @@ __srpm_extract() {
 
 	if check_prog rpm2tar
 	then
-		rpm2tar ${rpmpath};
-		tar xf ${tarfile};
-		srcfiles="$(tar tf ${tarfile})";
+		rpm2tar "${rpmpath}";
+		tar xf "${tarfile}";
+		srcfiles="$(tar tf "${tarfile}")";
 	elif check_prog rpm2cpio cpio
 	then
-		rpm2cpio ${rpmpath} > ${cpiofile};
-		cpio -i --quiet < ${cpiofile};
-		srcfiles="$(cpio -t --quiet < ${cpiofile})";
+		rpm2cpio "${rpmpath}" > "${cpiofile}";
+		cpio -i --quiet < "${cpiofile}";
+		srcfiles="$(cpio -t --quiet < "${cpiofile}")";
 	else
 		error "${rpmfile} requires rpm2targz or rpm to unpack";
 	fi
@@ -80,7 +80,7 @@ unpack() {
 	do
 		unpack_file_name=${unpack_file_path##*/};
 
-		if [ ! -f ${unpack_file_path} ]
+		if [ ! -f "${unpack_file_path}" ]
 		then
 			error "Cannot find source package ${unpack_file_name}";
 		fi
@@ -150,12 +150,12 @@ unpack() {
 
 		if defined unpack_out
 		then
-			if ! ${unpack_cmd} ${unpack_file_path} > ${unpack_out}
+			if ! ${unpack_cmd} "${unpack_file_path}" > "${unpack_out}"
 			then
 				error "${unpack_cmd} ${unpack_file_name} failed";
 			fi
 		else
-			if ! ${unpack_cmd} ${unpack_file_path}
+			if ! ${unpack_cmd} "${unpack_file_path}"
 			then
 				error "${unpack_cmd} ${unpack_file_name} failed";
 			fi
@@ -180,7 +180,7 @@ __gpg_verify() {
 		return 0;
 	fi
 
-	if [ -f ${_file}.${_sigext} ]
+	if [ -f "${_file}.${_sigext}" ]
 	then
 		inform "${_filetype} signature follows:";
 		gpg --verify ${_file}.${_sigext} ${_file} || true;
@@ -188,8 +188,8 @@ __gpg_verify() {
 }
 
 __mkdirs() {
-	cd ${top};
-	mkdir -p ${srcdir} ${origsrcdir} ${B} ${D} ${T} ${configdir} ${logdir} ${distdir} ${patchdir} ${spkgdir};
+	cd "${top}";
+	mkdir -p "${srcdir}" "${origsrcdir}" "${B}" "${D}" "${T}" "${configdir}" "${logdir}" "${distdir}" "${patchdir}" "${spkgdir}";
 }
 
 cygpatch() {
@@ -267,7 +267,7 @@ __src_prep() {
 	local tar_patch;
 	local n=1;
 
-	cd ${top};
+	cd "${top}";
 
 	__mkdirs;
 
@@ -275,17 +275,17 @@ __src_prep() {
 	# wasn't upgraded since prep
 	__config_set cygport_version ${_cygport_version}
 
-	if [ -f ${top}/${cygportfile}.sig ]
+	if [ -f "${top}/${cygportfile}.sig" ]
 	then
-		__gpg_verify ${top}/${cygportfile} "CYGPORT SCRIPT";
+		__gpg_verify "${top}/${cygportfile}" "CYGPORT SCRIPT";
 	fi
 
 	for src_pkg in ${_src_orig_pkgs}
 	do
-		if [ -f ${DISTDIR}/${src_pkg} -a ! -f ${top}/${src_pkg} ]
+		if [ -f ${DISTDIR}/${src_pkg} -a ! -f "${top}/${src_pkg}" ]
 		then
 			src_pkg=${DISTDIR}/${src_pkg};
-		elif [ -f ${top}/${src_pkg##*/} -a ! -f ${top}/${src_pkg} ]
+		elif [ -f "${top}"/${src_pkg##*/} -a ! -f "${top}/${src_pkg}" ]
 		then
 			src_pkg=${src_pkg##*/};
 		fi
@@ -300,10 +300,10 @@ __src_prep() {
 
 	for src_patch in ${_src_orig_patches}
 	do
-		if [ -f ${DISTDIR}/${src_patch} -a ! -f ${top}/${src_patch} ]
+		if [ -f "${DISTDIR}/${src_patch}" -a ! -f "${top}/${src_patch}" ]
 		then
 			src_patch=${DISTDIR}/${src_patch};
-		elif [ -f ${top}/${src_patch##*/} -a ! -f ${src_patch} ]
+		elif [ -f "${top}"/${src_patch##*/} -a ! -f "${src_patch}" ]
 		then
 			src_patch=${src_patch##*/};
 		fi
@@ -316,30 +316,30 @@ __src_prep() {
 		done
 	done
 
-	if [ -f ${top}/${cygwin_patchfile}.sig ]
+	if [ -f "${top}/${cygwin_patchfile}.sig" ]
 	then
 		__gpg_verify ${top}/${cygwin_patchfile} "CYGWIN PATCH";
 	fi
 
-	if [ -f ${top}/${src_patchfile}.sig ]
+	if [ -f "${top}/${src_patchfile}.sig" ]
 	then
-		__gpg_verify ${top}/${src_patchfile} "SOURCE PATCH";
+		__gpg_verify "${top}/${src_patchfile}" "SOURCE PATCH";
 	fi
 
-	cd ${origsrcdir};
+	cd "${origsrcdir}";
 
 	for src_pkg in ${_src_orig_pkgs}
 	do
-		if [ -f ${DISTDIR}/${src_pkg} -a ! -f ${top}/${src_pkg} ]
+		if [ -f "${DISTDIR}/${src_pkg}" -a ! -f "${top}/${src_pkg}" ]
 		then
 			src_pkg=${DISTDIR}/${src_pkg};
-		elif [ -f ${top}/${src_pkg##*/} -a ! -f ${top}/${src_pkg} ]
+		elif [ -f "${top}"/${src_pkg##*/} -a ! -f "${top}/${src_pkg}" ]
 		then
-			src_pkg=${top}/${src_pkg##*/};
+			src_pkg="${top}"/${src_pkg##*/};
 		else
-			src_pkg=${top}/${src_pkg};
+			src_pkg="${top}/${src_pkg}";
 		fi
-		unpack ${src_pkg};
+		unpack "${src_pkg}";
 	done
 
 #****v* Preparation/SRC_DIR
@@ -351,14 +351,14 @@ __src_prep() {
 #  files are unpacked, use SRC_DIR=".".
 #****
 
-	if [ ! -d ${origsrcdir}/${SRC_DIR} ]
+	if [ ! -d "${origsrcdir}/${SRC_DIR}" ]
 	then
 		error "SRC_DIR is not correctly defined"
 	fi
 
 	# cd will fail if not executable (e.g. dot2tex)
-	chmod +x ${origsrcdir}/${SRC_DIR};
-	cd ${origsrcdir}/${SRC_DIR};
+	chmod +x "${origsrcdir}/${SRC_DIR}";
+	cd "${origsrcdir}/${SRC_DIR}";
 
 #****v* Preparation/DISTCLEANFILES
 #  DESCRIPTION
@@ -370,7 +370,7 @@ __src_prep() {
 	if defined DISTCLEANFILES
 	then
 		inform "Removing DISTCLEANFILES..."
-		rm -f ${DISTCLEANFILES}
+		rm -f "${DISTCLEANFILES}"
 	fi
 
 	# src_unpack_hook() is an optional function which can be defined
@@ -379,33 +379,33 @@ __src_prep() {
 	if __check_function src_unpack_hook
 	then
 		__check_unstable src_unpack_hook;
-		cd ${origsrcdir}/${SRC_DIR};
+		cd "${origsrcdir}/${SRC_DIR}";
 	fi
 
 	for src_patch in ${_src_orig_patches}
 	do
-		if [ -f ${DISTDIR}/${src_patch} -a ! -f ${top}/${src_patch} ]
+		if [ -f "${DISTDIR}/${src_patch}" -a ! -f "${top}/${src_patch}" ]
 		then
 			src_patch=${DISTDIR}/${src_patch};
-		elif [ -f ${top}/${src_patch##*/} -a ! -f ${top}/${src_patch} ]
+		elif [ -f "${top}"/${src_patch##*/} -a ! -f "${top}/${src_patch}" ]
 		then
-			src_patch=${top}/${src_patch##*/};
+			src_patch="${top}"/${src_patch##*/};
 		else
-			src_patch=${top}/${src_patch};
+			src_patch="${top}/${src_patch}";
 		fi
 		case ${src_patch} in
 			*.tar.gz|*.tgz|*.tar.bz2|*.tbz2)
-				pushd ${T};
-				unpack ${src_patch};
+				pushd "${T}";
+				unpack "${src_patch}";
 				popd;
 
-				for tar_patch in $(tar tf ${src_patch} | sort | grep -E '(diff|patch)$')
+				for tar_patch in $(tar tf "${src_patch}" | sort | grep -E '(diff|patch)$')
 				do
-					cygpatch ${T}/${tar_patch};
+					cygpatch "${T}/${tar_patch}";
 				done
 				;;
 			*)
-				cygpatch ${src_patch};
+				cygpatch "${src_patch}";
 				;;
 		esac
 	done
@@ -415,26 +415,26 @@ __src_prep() {
 	if __check_function src_patch_hook
 	then
 		__check_unstable src_patch_hook;
-		cd ${origsrcdir}/${SRC_DIR};
+		cd "${origsrcdir}/${SRC_DIR}";
 	fi
 
 	__step "Preparing working source directory";
 
-	rsync -aq --delete-before ${origsrcdir}/ ${srcdir}/;
+	rsync -aq --delete-before "${origsrcdir}/" "${srcdir}/";
 
-	mkdir -p ${C};
-	ln -sfn ${C} ${workdir}/CYGWIN-PATCHES;
+	mkdir -p "${C}";
+	ln -sfn "${C}" "${workdir}/CYGWIN-PATCHES";
 
-	cd ${S};
+	cd "${S}";
 
-	if [ -f ${top}/${cygwin_patchfile} ]
+	if [ -f "${top}/${cygwin_patchfile}" ]
 	then
-		cygpatch ${top}/${cygwin_patchfile};
+		cygpatch "${top}/${cygwin_patchfile}";
 	fi
 
-	if [ -f ${top}/${src_patchfile} ]
+	if [ -f "${top}/${src_patchfile}" ]
 	then
-		cygpatch ${top}/${src_patchfile};
+		cygpatch "${top}/${src_patchfile}";
 	fi
 }
 
diff --git a/lib/syntax.cygpart b/lib/syntax.cygpart
index 8c7b227..2027858 100644
--- a/lib/syntax.cygpart
+++ b/lib/syntax.cygpart
@@ -150,19 +150,19 @@ __step() {
 }
 
 __log_init() {
-	local log=${1}
-	rm -f ${log}
+	local log="${1}"
+	rm -f "${log}"
 
-	echo -e cygport ${_cygport_version} '\n' >> ${log}
+	echo -e cygport "${_cygport_version}" '\n' >> "${log}"
 
 	for var in PF S B D C T CBUILD CHOST CTARGET CC CFLAGS CPPFLAGS CXX CXXFLAGS \
 	           F77 FFLAGS FC FCFLAGS GOC GOFLAGS OBJC OBJCFLAGS \
 	           OBJCXX OBJCXXFLAGS LDFLAGS LIBS MAKEOPTS
 	do
-		echo ${var} = ${!var} >> ${log}
+		echo ${var} = ${!var} >> "${log}"
 	done
 
-	echo -e '\n' >> ${log}
+	echo -e '\n' >> "${log}"
 }
 
 #****** Syntax/boolean
-- 
2.20.1

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH 2/2] Exit in case `cd` fails
  2019-07-14 19:39   ` [PATCH 1/2] Add support for path with spaces Federico Kircheis
@ 2019-07-14 19:39     ` Federico Kircheis
  0 siblings, 0 replies; 15+ messages in thread
From: Federico Kircheis @ 2019-07-14 19:39 UTC (permalink / raw)
  To: cygwin-apps; +Cc: Federico Kircheis

---
 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 ed61d25..4f3f17e 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 fb94c56..593dfb6 100644
--- a/lib/src_prep.cygpart
+++ b/lib/src_prep.cygpart
@@ -188,7 +188,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}";
 }
 
@@ -267,7 +267,7 @@ __src_prep() {
 	local tar_patch;
 	local n=1;
 
-	cd "${top}";
+	cd "${top}" || error "Unable to cd to ${top}";
 
 	__mkdirs;
 
@@ -326,7 +326,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
@@ -358,7 +358,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
@@ -379,7 +379,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}
@@ -415,7 +415,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";
@@ -425,7 +425,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.20.1

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: cygport development
  2019-07-14 17:11 ` Brian Inglis
  2019-07-14 19:39   ` [PATCH 1/2] Add support for path with spaces Federico Kircheis
@ 2019-09-28 11:56   ` Federico Kircheis
  2019-10-13 16:41     ` Achim Gratz
  1 sibling, 1 reply; 15+ messages in thread
From: Federico Kircheis @ 2019-09-28 11:56 UTC (permalink / raw)
  To: cygwin-apps

On 14.07.19 19:10, Brian Inglis wrote:
> On 2019-07-14 07:25, Federico Kircheis wrote:
>> I had the unfortunate idea to execute cygport in a directory that had in it's
>> path at least one whitespace (it's not that uncommon under Windows).
>> Cygport did not report a clear error, and created files at the wrong location.
>> I've posted a patch on github a couple of weeks ago (see
>> https://github.com/cygwinports/cygport/pull/12), but I did not get any feedback.
>> Does the development of cygport take place on github?
>> Should I post my patches somewhere else in order to get them integrated in cygport?
> 
> Cygport is a regular Cygwin app used by package maintainers.
> Send here and attach them as text with a git send-email subject and cover letter
> summarising the change and impact (as to cygwin-patches).
> 

I've sent the patches the 14.07.19, unfortunately I still got no answer.

I also checked https://github.com/cygwinports/cygport, AFAIK there was 
no activity.

Best regards

Federico


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: cygport development
  2019-09-28 11:56   ` cygport development Federico Kircheis
@ 2019-10-13 16:41     ` Achim Gratz
  2019-10-14  8:55       ` Federico Kircheis
  0 siblings, 1 reply; 15+ messages in thread
From: Achim Gratz @ 2019-10-13 16:41 UTC (permalink / raw)
  To: cygwin-apps

Federico Kircheis writes:
> I've sent the patches the 14.07.19, unfortunately I still got no answer.

The cygport maintainer is rather busy with non-Cygwin related work these
days, I suppose.  Anyway, one of the questions I have is why you need
these changes.  Most build systems do not actually work when they
encounter a path with spaces if they use make under the hood, so fixing
cygport to grok such path locations isn't getting you much further I'd
think.  Can you explain?


Regards,
Achim.
-- 
+<[Q+ Matrix-12 WAVE#46+305 Neuron microQkb Andromeda XTk Blofeld]>+

Wavetables for the Waldorf Blofeld:
http://Synth.Stromeko.net/Downloads.html#BlofeldUserWavetables

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: cygport development
  2019-10-13 16:41     ` Achim Gratz
@ 2019-10-14  8:55       ` Federico Kircheis
  2019-10-14 17:15         ` Doug Henderson
  2020-05-12 14:59         ` Federico Kircheis
  0 siblings, 2 replies; 15+ messages in thread
From: Federico Kircheis @ 2019-10-14  8:55 UTC (permalink / raw)
  To: cygwin-apps

On 13/10/2019 18.41, Achim Gratz wrote:
> Federico Kircheis writes:
>> I've sent the patches the 14.07.19, unfortunately I still got no answer.
> 
> The cygport maintainer is rather busy with non-Cygwin related work these
> days, I suppose.  Anyway, one of the questions I have is why you need
> these changes.  Most build systems do not actually work when they
> encounter a path with spaces if they use make under the hood, so fixing
> cygport to grok such path locations isn't getting you much further I'd
> think.  Can you explain?

Yep.

I've built some software in my windows home directory.
It contains a space.
I expected it to work.

Instead of failing with a clear error message, the build process deleted 
some unrelated files as it cd failed (or cd in the wrong directory, cant 
remember right now).

I believe it is unacceptable to delete unrelated data.

Even if it stated that there is no intention to support path with 
spaces, those scripts should fail fast and ideally with a clear error 
message.

I found it easier to quote the offending variables, as not only spaces 
might cause issues.

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: cygport development
  2019-10-14  8:55       ` Federico Kircheis
@ 2019-10-14 17:15         ` Doug Henderson
  2020-05-12 14:59         ` Federico Kircheis
  1 sibling, 0 replies; 15+ messages in thread
From: Doug Henderson @ 2019-10-14 17:15 UTC (permalink / raw)
  To: Cygwin Apps

On Mon., Oct. 14, 2019, 02:55 Federico Kircheis, <> wrote:

> On 13/10/2019 18.41, Achim Gratz wrote:
> > Federico Kircheis writes:
> >> I've sent the patches the 14.07.19, unfortunately I still got no answer.
> >
> > The cygport maintainer is rather busy with non-Cygwin related work these
> > days, I suppose.  Anyway, one of the questions I have is why you need
> > these changes.  Most build systems do not actually work when they
> > encounter a path with spaces if they use make under the hood, so fixing
> > cygport to grok such path locations isn't getting you much further I'd
> > think.  Can you explain?
>
> Yep.
>
> I've built some software in my windows home directory.
> It contains a space.
> I expected it to work.
>
> Instead of failing with a clear error message, the build process deleted
> some unrelated files as it cd failed (or cd in the wrong directory, cant
> remember right now).
>
> I believe it is unacceptable to delete unrelated data.
>
> Even if it stated that there is no intention to support path with
> spaces, those scripts should fail fast and ideally with a clear error
> message.
>
> I found it easier to quote the offending variables, as not only spaces
> might cause issues.
>

When I encountered a similar problem, I added code to quote env variables
that had spaces until cygport worked correctly. Then I submitted patches to
this list and the maintainer so the current code works for my problem case
OOB.

This is the best and fastest way to get your problem fixed, and, as a
bonus,  get credit as a contributor to open source software.

Doug

>

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: cygport development
  2019-10-14  8:55       ` Federico Kircheis
  2019-10-14 17:15         ` Doug Henderson
@ 2020-05-12 14:59         ` Federico Kircheis
  2020-05-15  4:55           ` Yaakov Selkowitz
  1 sibling, 1 reply; 15+ messages in thread
From: Federico Kircheis @ 2020-05-12 14:59 UTC (permalink / raw)
  To: cygwin-apps

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

On 10/14/19 10:55 AM, Federico Kircheis wrote:
> On 13/10/2019 18.41, Achim Gratz wrote:
>> Federico Kircheis writes:
>>> I've sent the patches the 14.07.19, unfortunately I still got no answer.
>>
>> The cygport maintainer is rather busy with non-Cygwin related work these
>> days, I suppose.  Anyway, one of the questions I have is why you need
>> these changes.  Most build systems do not actually work when they
>> encounter a path with spaces if they use make under the hood, so fixing
>> cygport to grok such path locations isn't getting you much further I'd
>> think.  Can you explain?
> 
> Yep.
> 
> I've built some software in my windows home directory.
> It contains a space.
> I expected it to work.
> 
> Instead of failing with a clear error message, the build process deleted 
> some unrelated files as it cd failed (or cd in the wrong directory, cant 
> remember right now).
> 
> I believe it is unacceptable to delete unrelated data.
> 
> Even if it stated that there is no intention to support path with 
> spaces, those scripts should fail fast and ideally with a clear error 
> message.
> 
> I found it easier to quote the offending variables, as not only spaces 
> might cause issues.

The merge request in the repository has been closed.

I'm attaching the updated patch.

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

From b927688b921988c9aa7dafe9fbde9b71f96aa5c3 Mon Sep 17 00:00:00 2001
From: Federico Kircheis <federico.kircheis@gmail.com>
Date: Tue, 2 Jul 2019 20:53:55 +0200
Subject: [PATCH 1/2] Add support for path with spaces

Quote most variables
---
 bin/cygport.in              |  74 +++++++++++++-------------
 lib/config_registry.cygpart |   8 +--
 lib/src_compile.cygpart     |   2 +-
 lib/src_fetch.cygpart       |  30 +++++------
 lib/src_prep.cygpart        | 102 ++++++++++++++++++------------------
 lib/syntax.cygpart          |  10 ++--
 6 files changed, 113 insertions(+), 113 deletions(-)

diff --git a/bin/cygport.in b/bin/cygport.in
index 12909fe..0503676 100755
--- a/bin/cygport.in
+++ b/bin/cygport.in
@@ -42,7 +42,7 @@ declare -r _privsysconfdir=@sysconfdir@;
 
 
 ### import defined, pushd, popd
-source ${_privlibdir}/syntax.cygpart
+source "${_privlibdir}/syntax.cygpart"
 ###
 
 
@@ -166,7 +166,7 @@ source ${_privlibdir}/help.cygpart
 # Accept --help and --version arguments without specifying a cygport file
 while true
 do
-	case ${1} in
+	case "${1}" in
 	--help|-h|-\?)
 		__show_help;
 		exit 0;
@@ -204,7 +204,7 @@ do
 	esac
 done
 
-declare -ar argv=(${0} ${@})
+declare -ar argv=(${0} "${@}")
 declare -ir argc=$(( $# + 1 ))
 
 # Show help if no commands are given
@@ -222,7 +222,7 @@ fi
 ################################################################################
 
 ### import check_prog and friends
-source ${_privlibdir}/check_funcs.cygpart
+source "${_privlibdir}/check_funcs.cygpart"
 ###
 
 # check now for all mandatory programs
@@ -349,11 +349,11 @@ unset _autotools_CYGCLASS_ _autotools_CYGCLASS_stage1_
 ################################################################################
 
 unset NAME VERSION RELEASE
-if [ -f ${argv[1]} ]
+if [ -f "${argv[1]}" ]
 then
-	eval $(grep '^NAME=' ${argv[1]})
-	eval $(grep '^VERSION=' ${argv[1]})
-	eval $(grep '^RELEASE=' ${argv[1]})
+	eval "$(grep '^NAME=' "${argv[1]}")"
+	eval "$(grep '^VERSION=' "${argv[1]}")"
+	eval "$(grep '^RELEASE=' "${argv[1]}")"
 fi
 
 if [ "${NAME+y}${VERSION+y}${RELEASE+y}" = "yyy" ]
@@ -371,7 +371,7 @@ declare -r  PN=${PF%%-[0-9]*};
 declare     NAME=${PN}
 declare -r  PR=${PF##*-};
 declare     RELEASE=${PR}
-            PV=$(echo ${PF} | sed -e "s/${PN}\-\(.*\)\-${PR}$/\1/");
+            PV=$(echo "${PF}" | sed -e "s/${PN}\-\(.*\)\-${PR}$/\1/");
 declare     VERSION=${PV}
 declare -r  cygportfile=${PF}.cygport;
 fi
@@ -395,7 +395,7 @@ _topdir=${argv[1]%/*};
 
 if [ "x${_topdir}" = "x${argv[1]}" ]
 then
-	if [ -f ./${cygportfile} ]
+	if [ -f "./${cygportfile}" ]
 	then
 		_topdir=.;
 	else
@@ -406,7 +406,7 @@ fi
 declare -r top=$(cd ${_topdir}; pwd);
 unset _topdir;
 
-if [ ! -e ${top}/${cygportfile} ]
+if [ ! -e "${top}/${cygportfile}" ]
 then
 	error "${cygportfile} not found.";
 fi
@@ -438,7 +438,7 @@ done
 unset n VALUE ARCHES VAR
 
 ### load .cygport
-source ${top}/${cygportfile} || error "could not read ${cygportfile}"
+source "${top}/${cygportfile}" || error "could not read ${cygportfile}"
 ###
 
 case ${ARCH} in
@@ -448,7 +448,7 @@ esac
 
 if defined CYGPORT_DEPEND
 then
-	if ! __version_at_least ${CYGPORT_DEPEND} ${_cygport_version}
+	if ! __version_at_least "${CYGPORT_DEPEND}" "${_cygport_version}"
 	then
 		error "This package requires cygport ${CYGPORT_DEPEND} or newer";
 	fi
@@ -511,7 +511,7 @@ declare -r uploadlog="${logdir}/${PF}-upload.log";
 
 for _src_uri in ${SRC_URI}
 do
-	if [ -f ${top}/${_src_uri} ]
+	if [ -f "${top}/${_src_uri}" ]
 	then
 		_src_orig_pkgs+=" ${_src_uri}";
 		continue;
@@ -525,7 +525,7 @@ unset _src_uri;
 
 for _patch_uri in ${PATCH_URI}
 do
-	if [ -f ${top}/${_patch_uri} ]
+	if [ -f "${top}/${_patch_uri}" ]
 	then
 		_src_orig_patches+=" ${_patch_uri}";
 		continue;
@@ -537,8 +537,8 @@ done
 readonly _src_orig_patches;
 unset _patch_uri;
 
-declare -r cygwin_patchfile=${PF}.cygwin.patch;
-declare -r src_patchfile=${PF}.src.patch;
+declare -r cygwin_patchfile="${PF}.cygwin.patch";
+declare -r src_patchfile="${PF}.src.patch";
 
 declare -ar pkg_name=(${PKG_NAMES:-${PN}});
 declare -r  pkg_count=${#pkg_name[*]};
@@ -586,21 +586,21 @@ do
 			;;
 		compile|build|make)
 			__stage Compiling;
-			__log_init ${compilelog};
+			__log_init "${compilelog}";
 			__check_depends && \
-			src_compile 2>&1 | tee -a ${compilelog};
+			src_compile 2>&1 | tee -a "${compilelog}";
 			_status=${PIPESTATUS[0]};
 			;;
 		check|test)
 			__stage Testing;
-			__log_init ${checklog};
-			src_test 2>&1 | tee -a ${checklog};
+			__log_init "${checklog}";
+			src_test 2>&1 | tee -a "${checklog}";
 			_status=${PIPESTATUS[0]};
 			;;
 		inst*)
 			__stage Installing;
-			__log_init ${installlog};
-			(__prepinstalldirs && src_install && __src_postinst) 2>&1 | tee -a ${installlog};
+			__log_init "${installlog}";
+			(__prepinstalldirs && src_install && __src_postinst) 2>&1 | tee -a "${installlog}";
 			_status=${PIPESTATUS[0]};
 			;;
 		postinst*)
@@ -632,8 +632,8 @@ do
 			;&
 		package|pkg)
 			__stage "Packaging${_pkg_tag:+ ${_pkg_tag%:} release}";
-			__log_init ${pkglog};
-			(__pkg_binpkg && __pkg_pkgcheck && __pkg_srcpkg && __pkg_dist ${_pkg_tag}) 2>&1 | tee -a ${pkglog};
+			__log_init "${pkglog}";
+			(__pkg_binpkg && __pkg_pkgcheck && __pkg_srcpkg && __pkg_dist "${_pkg_tag}") 2>&1 | tee -a "${pkglog}";
 			_status=${PIPESTATUS[0]};
 			;;
 		diff|mkdiff|mkpatch)
@@ -642,14 +642,14 @@ do
 			;;
 		upload|up)
 			__stage Uploading;
-			__log_init ${uploadlog};
-			(__pkg_upload full) 2>&1 | tee -a ${uploadlog};
+			__log_init "${uploadlog}";
+			(__pkg_upload full) 2>&1 | tee -a "${uploadlog}";
 			_status=${PIPESTATUS[0]};
 			;;
 		stage)
 			__stage Staging;
-			__log_init ${uploadlog};
-			(__pkg_upload stage) 2>&1 | tee -a ${uploadlog};
+			__log_init "${uploadlog}";
+			(__pkg_upload stage) 2>&1 | tee -a "${uploadlog}";
 			_status=${PIPESTATUS[0]};
 			;;
 		announce)
@@ -666,15 +666,15 @@ do
 			;&
 		almostall|all)
 			__stage Preparing && __src_prep && \
-			__log_init ${compilelog} && \
+			__log_init "${compilelog}" && \
 			__check_depends && \
-			__stage Compiling && src_compile 2>&1 | tee -a ${compilelog} && \
+			__stage Compiling && src_compile 2>&1 | tee -a "${compilelog}" && \
 			test ${PIPESTATUS[0]} -eq 0 && \
-			__log_init ${installlog} && \
-			__stage Installing && (__prepinstalldirs && src_install && __src_postinst) 2>&1 | tee -a ${installlog} && \
+			__log_init "${installlog}" && \
+			__stage Installing && (__prepinstalldirs && src_install && __src_postinst) 2>&1 | tee -a "${installlog}" && \
 			test ${PIPESTATUS[0]} -eq 0 && \
-			__log_init ${pkglog} && \
-			__stage Packaging && (__pkg_binpkg && __pkg_pkgcheck && __pkg_srcpkg && __pkg_dist ${_pkg_tag}) 2>&1 | tee -a ${pkglog} && \
+			__log_init "${pkglog}" && \
+			__stage Packaging && (__pkg_binpkg && __pkg_pkgcheck && __pkg_srcpkg && __pkg_dist "${_pkg_tag}") 2>&1 | tee -a "${pkglog}" && \
 			test ${PIPESTATUS[0]} -eq 0
 			_status=$?;
 			;;
@@ -691,9 +691,9 @@ do
 			exit 1;
 			;;
 		*)
-			if __check_function ${argv[${arg_n}]} && ! __check_function_ro ${argv[${arg_n}]}
+			if __check_function "${argv[${arg_n}]}" && ! __check_function_ro "${argv[${arg_n}]}"
 			then
-				${argv[${arg_n}]};
+				"${argv[${arg_n}]}";
 			else
 				error "unknown command ${argv[${arg_n}]}";
 			fi
diff --git a/lib/config_registry.cygpart b/lib/config_registry.cygpart
index 4d6c6d1..b7662d9 100644
--- a/lib/config_registry.cygpart
+++ b/lib/config_registry.cygpart
@@ -21,16 +21,16 @@
 ################################################################################
 
 __config_get() {
-	if [ -f ${configdir}/${1} ]
+	if [ -f "${configdir}/${1}" ]
 	then
-		echo -n $(cat ${configdir}/${1});
+		echo -n "$(cat "${configdir}/${1}")";
 	else
 		echo -n "0";
 	fi
 }
 
 __config_equals() {
-	if [ -f ${configdir}/${1} ] && [ $(cat ${configdir}/${1}) = ${2} ]
+	if [ -f "${configdir}/${1}" ] && [ "$(cat "${configdir}/${1}")" = "${2}" ]
 	then
 		return 0;
 	else
@@ -39,7 +39,7 @@ __config_equals() {
 }
 
 __config_set() {
-	echo -n ${2} > ${configdir}/${1};
+	echo -n "${2}" > "${configdir}/${1}";
 }
 
 readonly -f __config_equals __config_get __config_set
diff --git a/lib/src_compile.cygpart b/lib/src_compile.cygpart
index 4944520..f1b9c37 100644
--- a/lib/src_compile.cygpart
+++ b/lib/src_compile.cygpart
@@ -49,7 +49,7 @@ lndirs() {
 	fi
 
 	check_prog_req lndir
-	lndir -silent ${fromdir} ${todir} || error "lndir failed"
+	lndir -silent "${fromdir}" "${todir}" || error "lndir failed"
 }
 
 #****C* Compiling/manifestize
diff --git a/lib/src_fetch.cygpart b/lib/src_fetch.cygpart
index a273045..942be53 100644
--- a/lib/src_fetch.cygpart
+++ b/lib/src_fetch.cygpart
@@ -72,36 +72,36 @@ fetch() {
 	urifile=${urifile%\?*};
 	urifile=${urifile##*/};
 
-	if defined __DL_ONLY_MISSING && defined DISTDIR && [ -f ${DISTDIR}/${urifile} ]
+	if defined __DL_ONLY_MISSING && defined DISTDIR && [ -f "${DISTDIR}/${urifile}" ]
 	then
 		inform "Using ${urifile} from DISTDIR"
 		return 0
 	elif check_prog wget
 	then
-		if wget --no-check-certificate -O ${urifile}.tmp ${uri}
+		if wget --no-check-certificate -O "${urifile}.tmp" "${uri}"
 		then
-			mv -f ${urifile}.tmp ${urifile}
+			mv -f "${urifile}.tmp" "${urifile}"
 		else
-			rm -f ${urifile}.tmp
+			rm -f "${urifile}.tmp"
 			error "wget ${uri} failed"
 		fi
 	elif check_prog curl
 	then
-		if curl -k --url ${uri} -o ${urifile}.tmp
+		if curl -k --url "${uri}" -o "${urifile}.tmp"
 		then
-			mv -f ${urifile}.tmp ${urifile}
+			mv -f "${urifile}.tmp" "${urifile}"
 		else
-			rm -f ${urifile}.tmp
+			rm -f "${urifile}.tmp"
 			error "curl ${uri} failed"
 		fi
 	else
 		error "Either wget or curl are required to fetch sources.";
 	fi
 
-	if defined DISTDIR && [ -f ${urifile} ]
+	if defined DISTDIR && [ -f "${urifile}" ]
 	then
-		[ -d ${DISTDIR} ] || mkdir -p ${DISTDIR}
-		mv ${urifile} ${DISTDIR}/
+		[ -d "${DISTDIR}" ] || mkdir -p "${DISTDIR}"
+		mv "${urifile}" "${DISTDIR}/"
 	fi
 }
 
@@ -156,7 +156,7 @@ __src_fetch() {
 	done
 
 	# the RCS_fetch functions change PWD
-	cd ${top};
+	cd "${top}";
 
 	for uri in ${SRC_URI} ${PATCH_URI}
 	do
@@ -165,11 +165,11 @@ __src_fetch() {
 				continue
 		fi
 		case ${uri%%//*} in
-			mirror:)	__mirror_fetch ${uri} ;;
-			http:|https:|ftp:)	fetch ${uri} || error "Download ${uri##*/} failed" ;;
-			file:)		[ -f ${uri#file://} ] || error "${uri##*/}: does not exist" ;;
+			mirror:)	__mirror_fetch "${uri}" ;;
+			http:|https:|ftp:)	fetch "${uri}" || error "Download ${uri##*/} failed" ;;
+			file:)		[ -f "${uri#file://}" ] || error "${uri##*/}: does not exist" ;;
 			*:)		error "Unsupported download protocol ${uri%%//*}" ;;
-			*/*)		[ -f ${uri#file://} ] || error "${uri##*/}: does not exist" ;;
+			*/*)		[ -f "${uri#file://}" ] || error "${uri##*/}: does not exist" ;;
 			${uri})		;; # file in working directory
 			*)		error "Invalid download URI ${uri}" ;;
 		esac
diff --git a/lib/src_prep.cygpart b/lib/src_prep.cygpart
index 80ba8d5..52440ac 100644
--- a/lib/src_prep.cygpart
+++ b/lib/src_prep.cygpart
@@ -50,14 +50,14 @@ __srpm_extract() {
 
 	if check_prog rpm2tar
 	then
-		rpm2tar ${rpmpath};
-		tar xf ${tarfile};
-		srcfiles="$(tar tf ${tarfile})";
+		rpm2tar "${rpmpath}";
+		tar xf "${tarfile}";
+		srcfiles="$(tar tf "${tarfile}")";
 	elif check_prog rpm2cpio cpio
 	then
-		rpm2cpio ${rpmpath} > ${cpiofile};
-		cpio -i --quiet < ${cpiofile};
-		srcfiles="$(cpio -t --quiet < ${cpiofile})";
+		rpm2cpio "${rpmpath}" > "${cpiofile}";
+		cpio -i --quiet < "${cpiofile}";
+		srcfiles="$(cpio -t --quiet < "${cpiofile}")";
 	else
 		error "${rpmfile} requires rpm2targz or rpm to unpack";
 	fi
@@ -81,7 +81,7 @@ unpack() {
 	do
 		unpack_file_name=${unpack_file_path##*/};
 
-		if [ ! -f ${unpack_file_path} ]
+		if [ ! -f "${unpack_file_path}" ]
 		then
 			error "Cannot find source package ${unpack_file_name}";
 		fi
@@ -151,12 +151,12 @@ unpack() {
 
 		if defined unpack_out
 		then
-			if ! ${unpack_cmd} ${unpack_file_path} > ${unpack_out}
+			if ! ${unpack_cmd} "${unpack_file_path}" > "${unpack_out}"
 			then
 				error "${unpack_cmd} ${unpack_file_name} failed";
 			fi
 		else
-			if ! ${unpack_cmd} ${unpack_file_path}
+			if ! ${unpack_cmd} "${unpack_file_path}"
 			then
 				error "${unpack_cmd} ${unpack_file_name} failed";
 			fi
@@ -181,7 +181,7 @@ __gpg_verify() {
 		return 0;
 	fi
 
-	if [ -f ${_file}.${_sigext} ]
+	if [ -f "${_file}.${_sigext}" ]
 	then
 		inform "${_filetype} signature follows:";
 		gpg --verify ${_file}.${_sigext} ${_file} || true;
@@ -189,8 +189,8 @@ __gpg_verify() {
 }
 
 __mkdirs() {
-	cd ${top};
-	mkdir -p ${srcdir} ${origsrcdir} ${B} ${D} ${T} ${configdir} ${logdir} ${distdir} ${patchdir} ${spkgdir};
+	cd "${top}";
+	mkdir -p "${srcdir}" "${origsrcdir}" "${B}" "${D}" "${T}" "${configdir}" "${logdir}" "${distdir}" "${patchdir}" "${spkgdir}";
 }
 
 cygpatch() {
@@ -286,7 +286,7 @@ __src_prep() {
 	local tar_patch;
 	local n=1;
 
-	cd ${top};
+	cd "${top}";
 
 	__mkdirs;
 
@@ -294,17 +294,17 @@ __src_prep() {
 	# wasn't upgraded since prep
 	__config_set cygport_version ${_cygport_version}
 
-	if [ -f ${top}/${cygportfile}.sig ]
+	if [ -f "${top}/${cygportfile}.sig" ]
 	then
-		__gpg_verify ${top}/${cygportfile} "CYGPORT SCRIPT";
+		__gpg_verify "${top}/${cygportfile}" "CYGPORT SCRIPT";
 	fi
 
 	for src_pkg in ${_src_orig_pkgs}
 	do
-		if [ -f ${DISTDIR}/${src_pkg} -a ! -f ${top}/${src_pkg} ]
+		if [ -f ${DISTDIR}/${src_pkg} -a ! -f "${top}/${src_pkg}" ]
 		then
 			src_pkg=${DISTDIR}/${src_pkg};
-		elif [ -f ${top}/${src_pkg##*/} -a ! -f ${top}/${src_pkg} ]
+		elif [ -f "${top}"/${src_pkg##*/} -a ! -f "${top}/${src_pkg}" ]
 		then
 			src_pkg=${src_pkg##*/};
 		fi
@@ -319,10 +319,10 @@ __src_prep() {
 
 	for src_patch in ${_src_orig_patches}
 	do
-		if [ -f ${DISTDIR}/${src_patch} -a ! -f ${top}/${src_patch} ]
+		if [ -f "${DISTDIR}/${src_patch}" -a ! -f "${top}/${src_patch}" ]
 		then
 			src_patch=${DISTDIR}/${src_patch};
-		elif [ -f ${top}/${src_patch##*/} -a ! -f ${src_patch} ]
+		elif [ -f "${top}"/${src_patch##*/} -a ! -f "${src_patch}" ]
 		then
 			src_patch=${src_patch##*/};
 		fi
@@ -335,30 +335,30 @@ __src_prep() {
 		done
 	done
 
-	if [ -f ${top}/${cygwin_patchfile}.sig ]
+	if [ -f "${top}/${cygwin_patchfile}.sig" ]
 	then
 		__gpg_verify ${top}/${cygwin_patchfile} "CYGWIN PATCH";
 	fi
 
-	if [ -f ${top}/${src_patchfile}.sig ]
+	if [ -f "${top}/${src_patchfile}.sig" ]
 	then
-		__gpg_verify ${top}/${src_patchfile} "SOURCE PATCH";
+		__gpg_verify "${top}/${src_patchfile}" "SOURCE PATCH";
 	fi
 
-	cd ${origsrcdir};
+	cd "${origsrcdir}";
 
 	for src_pkg in ${_src_orig_pkgs}
 	do
-		if [ -f ${DISTDIR}/${src_pkg} -a ! -f ${top}/${src_pkg} ]
+		if [ -f "${DISTDIR}/${src_pkg}" -a ! -f "${top}/${src_pkg}" ]
 		then
 			src_pkg=${DISTDIR}/${src_pkg};
-		elif [ -f ${top}/${src_pkg##*/} -a ! -f ${top}/${src_pkg} ]
+		elif [ -f "${top}"/${src_pkg##*/} -a ! -f "${top}/${src_pkg}" ]
 		then
-			src_pkg=${top}/${src_pkg##*/};
+			src_pkg="${top}"/${src_pkg##*/};
 		else
-			src_pkg=${top}/${src_pkg};
+			src_pkg="${top}/${src_pkg}";
 		fi
-		unpack ${src_pkg};
+		unpack "${src_pkg}";
 	done
 
 #****v* Preparation/SRC_DIR
@@ -370,14 +370,14 @@ __src_prep() {
 #  files are unpacked, use SRC_DIR=".".
 #****
 
-	if [ ! -d ${origsrcdir}/${SRC_DIR} ]
+	if [ ! -d "${origsrcdir}/${SRC_DIR}" ]
 	then
 		error "SRC_DIR is not correctly defined"
 	fi
 
 	# cd will fail if not executable (e.g. dot2tex)
-	chmod +x ${origsrcdir}/${SRC_DIR};
-	cd ${origsrcdir}/${SRC_DIR};
+	chmod +x "${origsrcdir}/${SRC_DIR}";
+	cd "${origsrcdir}/${SRC_DIR}";
 
 #****v* Preparation/DISTCLEANFILES
 #  DESCRIPTION
@@ -389,7 +389,7 @@ __src_prep() {
 	if defined DISTCLEANFILES
 	then
 		inform "Removing DISTCLEANFILES..."
-		rm -f ${DISTCLEANFILES}
+		rm -f "${DISTCLEANFILES}"
 	fi
 
 
@@ -404,33 +404,33 @@ __src_prep() {
 	if __check_function src_unpack_hook
 	then
 		__check_unstable src_unpack_hook;
-		cd ${origsrcdir}/${SRC_DIR};
+		cd "${origsrcdir}/${SRC_DIR}";
 	fi
 
 	for src_patch in ${_src_orig_patches}
 	do
-		if [ -f ${DISTDIR}/${src_patch} -a ! -f ${top}/${src_patch} ]
+		if [ -f "${DISTDIR}/${src_patch}" -a ! -f "${top}/${src_patch}" ]
 		then
 			src_patch=${DISTDIR}/${src_patch};
-		elif [ -f ${top}/${src_patch##*/} -a ! -f ${top}/${src_patch} ]
+		elif [ -f "${top}"/${src_patch##*/} -a ! -f "${top}/${src_patch}" ]
 		then
-			src_patch=${top}/${src_patch##*/};
+			src_patch="${top}"/${src_patch##*/};
 		else
-			src_patch=${top}/${src_patch};
+			src_patch="${top}/${src_patch}";
 		fi
 		case ${src_patch} in
 			*.tar.gz|*.tgz|*.tar.bz2|*.tbz2)
-				pushd ${T};
-				unpack ${src_patch};
+				pushd "${T}";
+				unpack "${src_patch}";
 				popd;
 
-				for tar_patch in $(tar tf ${src_patch} | sort | grep -E '(diff|patch)$')
+				for tar_patch in $(tar tf "${src_patch}" | sort | grep -E '(diff|patch)$')
 				do
-					cygpatch ${T}/${tar_patch};
+					cygpatch "${T}/${tar_patch}";
 				done
 				;;
 			*)
-				cygpatch ${src_patch};
+				cygpatch "${src_patch}";
 				;;
 		esac
 	done
@@ -446,26 +446,26 @@ __src_prep() {
 	if __check_function src_patch_hook
 	then
 		__check_unstable src_patch_hook;
-		cd ${origsrcdir}/${SRC_DIR};
+		cd "${origsrcdir}/${SRC_DIR}";
 	fi
 
 	__step "Preparing working source directory";
 
-	rsync -aq --delete-before ${origsrcdir}/ ${srcdir}/;
+	rsync -aq --delete-before "${origsrcdir}/" "${srcdir}/";
 
-	mkdir -p ${C};
-	ln -sfn ${C} ${workdir}/CYGWIN-PATCHES;
+	mkdir -p "${C}";
+	ln -sfn "${C}" "${workdir}/CYGWIN-PATCHES";
 
-	cd ${S};
+	cd "${S}";
 
-	if [ -f ${top}/${cygwin_patchfile} ]
+	if [ -f "${top}/${cygwin_patchfile}" ]
 	then
-		cygpatch ${top}/${cygwin_patchfile};
+		cygpatch "${top}/${cygwin_patchfile}";
 	fi
 
-	if [ -f ${top}/${src_patchfile} ]
+	if [ -f "${top}/${src_patchfile}" ]
 	then
-		cygpatch ${top}/${src_patchfile};
+		cygpatch "${top}/${src_patchfile}";
 	fi
 }
 
diff --git a/lib/syntax.cygpart b/lib/syntax.cygpart
index 9d3bcb4..37acfe7 100644
--- a/lib/syntax.cygpart
+++ b/lib/syntax.cygpart
@@ -150,19 +150,19 @@ __step() {
 }
 
 __log_init() {
-	local log=${1}
-	rm -f ${log}
+	local log="${1}"
+	rm -f "${log}"
 
-	echo -e cygport ${_cygport_version} '\n' >> ${log}
+	echo -e cygport "${_cygport_version}" '\n' >> "${log}"
 
 	for var in PF S B D C T CBUILD CHOST CTARGET CC CFLAGS CPPFLAGS CXX CXXFLAGS \
 	           F77 FFLAGS FC FCFLAGS GOC GOFLAGS OBJC OBJCFLAGS \
 	           OBJCXX OBJCXXFLAGS LDFLAGS LIBS MAKEOPTS
 	do
-		echo ${var} = ${!var} >> ${log}
+		echo ${var} = ${!var} >> "${log}"
 	done
 
-	echo -e '\n' >> ${log}
+	echo -e '\n' >> "${log}"
 }
 
 #****** Syntax/boolean
-- 
2.26.2


From feeaccd68eda66f8ad65ff48417347125ced9034 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 2/2] 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 942be53..21c9d7a 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 52440ac..eae0b95 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


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: cygport development
  2020-05-12 14:59         ` Federico Kircheis
@ 2020-05-15  4:55           ` Yaakov Selkowitz
  2020-05-17 17:54             ` Federico Kircheis
  0 siblings, 1 reply; 15+ messages in thread
From: Yaakov Selkowitz @ 2020-05-15  4:55 UTC (permalink / raw)
  To: cygwin-apps

On Tue, 2020-05-12 at 16:59 +0200, Federico Kircheis wrote:
> On 10/14/19 10:55 AM, Federico Kircheis wrote:
> > On 13/10/2019 18.41, Achim Gratz wrote:
> > > Federico Kircheis writes:
> > > > I've sent the patches the 14.07.19, unfortunately I still got no answer.
> > > 
> > > The cygport maintainer is rather busy with non-Cygwin related work these
> > > days, I suppose.  Anyway, one of the questions I have is why you need
> > > these changes.  Most build systems do not actually work when they
> > > encounter a path with spaces if they use make under the hood, so fixing
> > > cygport to grok such path locations isn't getting you much further I'd
> > > think.  Can you explain?
> > 
> > Yep.
> > 
> > I've built some software in my windows home directory.
> > It contains a space.
> > I expected it to work.
> > 
> > Instead of failing with a clear error message, the build process deleted 
> > some unrelated files as it cd failed (or cd in the wrong directory, cant 
> > remember right now).
> > 
> > I believe it is unacceptable to delete unrelated data.
> > 
> > Even if it stated that there is no intention to support path with 
> > spaces, those scripts should fail fast and ideally with a clear error 
> > message.
> > 
> > I found it easier to quote the offending variables, as not only spaces 
> > might cause issues.
> 
> The merge request in the repository has been closed.
> 
> I'm attaching the updated patch.

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

--
Yaakov 



^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: cygport development
  2020-05-15  4:55           ` Yaakov Selkowitz
@ 2020-05-17 17:54             ` Federico Kircheis
  2020-05-29  4:38               ` Federico Kircheis
  0 siblings, 1 reply; 15+ messages in thread
From: Federico Kircheis @ 2020-05-17 17:54 UTC (permalink / raw)
  To: cygwin-apps

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.


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: cygport development
  2020-05-17 17:54             ` Federico Kircheis
@ 2020-05-29  4:38               ` Federico Kircheis
  2020-06-12  7:55                 ` Federico Kircheis
  0 siblings, 1 reply; 15+ messages in thread
From: Federico Kircheis @ 2020-05-29  4:38 UTC (permalink / raw)
  To: cygwin-apps

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


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: cygport development
  2020-05-29  4:38               ` Federico Kircheis
@ 2020-06-12  7:55                 ` Federico Kircheis
  2020-06-29 16:04                   ` Federico Kircheis
  0 siblings, 1 reply; 15+ messages in thread
From: Federico Kircheis @ 2020-06-12  7:55 UTC (permalink / raw)
  To: cygwin-apps

On May 29, 2020 4:38:53 AM UTC, Federico Kircheis wrote:
>I did not get any response to my last questions, so I hope this patch
>is 
>enough.
>
>Any thought about my other arguments?
>
>Federico
Ping.

Any updates or comments?

Is the patch ok?

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: cygport development
  2020-06-12  7:55                 ` Federico Kircheis
@ 2020-06-29 16:04                   ` Federico Kircheis
  2021-11-06 14:53                     ` Federico Kircheis
  0 siblings, 1 reply; 15+ messages in thread
From: Federico Kircheis @ 2020-06-29 16:04 UTC (permalink / raw)
  To: cygwin-apps

On 6/12/20 9:55 AM, Federico Kircheis wrote:
> On May 29, 2020 4:38:53 AM UTC, Federico Kircheis wrote:
>> I did not get any response to my last questions, so I hope this patch
>> is
>> enough.
>>
>> Any thought about my other arguments?
>>
>> Federico
> Ping.
> 
> Any updates or comments?
> 
> Is the patch ok?
> 

Ping

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: cygport development
  2020-06-29 16:04                   ` Federico Kircheis
@ 2021-11-06 14:53                     ` Federico Kircheis
  0 siblings, 0 replies; 15+ messages in thread
From: Federico Kircheis @ 2021-11-06 14:53 UTC (permalink / raw)
  To: cygwin-apps


On 29/06/2020 18.04, Federico Kircheis wrote:
> On 6/12/20 9:55 AM, Federico Kircheis wrote:
>> On May 29, 2020 4:38:53 AM UTC, Federico Kircheis wrote:
>>> I did not get any response to my last questions, so I hope this patch
>>> is
>>> enough.
>>>
>>> Any thought about my other arguments?
>>>
>>> Federico
>> Ping.
>>
>> Any updates or comments?
>>
>> Is the patch ok?
>>
> 
> Ping



I know it's been a while, I still would like cygport to avoid messing up 
unrelated directories.

Are there any disadvantage stopping when cd fails?
I did not get any feedback.

AFAIK my patch has not been integrated and still applies to current master.

Best

Federico

^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2021-11-06 14:57 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-07-14 13:25 cygport development Federico Kircheis
2019-07-14 17:11 ` Brian Inglis
2019-07-14 19:39   ` [PATCH 1/2] Add support for path with spaces Federico Kircheis
2019-07-14 19:39     ` [PATCH 2/2] Exit in case `cd` fails Federico Kircheis
2019-09-28 11:56   ` cygport development Federico Kircheis
2019-10-13 16:41     ` Achim Gratz
2019-10-14  8:55       ` Federico Kircheis
2019-10-14 17:15         ` Doug Henderson
2020-05-12 14:59         ` Federico Kircheis
2020-05-15  4:55           ` Yaakov Selkowitz
2020-05-17 17:54             ` Federico Kircheis
2020-05-29  4:38               ` Federico Kircheis
2020-06-12  7:55                 ` Federico Kircheis
2020-06-29 16:04                   ` Federico Kircheis
2021-11-06 14:53                     ` Federico Kircheis

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).