cygport development

classic Classic list List threaded Threaded
11 messages Options
Reply | Threaded
Open this post in threaded view
|

cygport development

Federico Kircheis
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
Reply | Threaded
Open this post in threaded view
|

Re: cygport development

Brian Inglis
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.
Reply | Threaded
Open this post in threaded view
|

[PATCH 1/2] Add support for path with spaces

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

Reply | Threaded
Open this post in threaded view
|

[PATCH 2/2] Exit in case `cd` fails

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

Reply | Threaded
Open this post in threaded view
|

Re: cygport development

Federico Kircheis
In reply to this post by Brian Inglis
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


Reply | Threaded
Open this post in threaded view
|

Re: cygport development

Achim Gratz
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
Reply | Threaded
Open this post in threaded view
|

Re: cygport development

Federico Kircheis
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.
Reply | Threaded
Open this post in threaded view
|

Re: cygport development

Doug Henderson
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

>
Reply | Threaded
Open this post in threaded view
|

Re: cygport development

cygwin-apps mailing list
In reply to this post by Federico Kircheis
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.

path-with-spaces.patch (21K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: cygport development

Yaakov Selkowitz
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


Reply | Threaded
Open this post in threaded view
|

Re: cygport development

cygwin-apps mailing list
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.