Re: [PATCH cygport] Add customization support for announce command
Adam Dinwoodie via Cygwin-apps wrote:
On Tue, Apr 30, 2024 at 12:27:35PM +0200, Christian Franke via Cygwin-apps
wrote:
Jon Turney wrote:
On 10/03/2024 16:33, Christian Franke via Cygwin-apps wrote:
+ /bin/bash -c "cd ${top} || exit 1
+${HOMEPAGE+HOMEPAGE=${HOMEPAGE@Q}}
+P=${P@Q}; PF=${PF@Q}; PN=${PN@Q}; PR=${PR@Q}; PV=(${PV[*]@Q})
+${SMTP_SENDER+SMTP_SENDER=${SMTP_SENDER@Q}}
+${SMTP_SERVER+SMTP_SERVER=${SMTP_SERVER@Q}}
+${SMTP_SERVER_PORT+SMTP_SERVER_PORT=${SMTP_SERVER_PORT@Q}}
+${SMTP_ENCRYPTION+SMTP_ENCRYPTION=${SMTP_ENCRYPTION@Q}}
+${SMTP_USER+SMTP_USER=${SMTP_USER@Q}}
+${SMTP_PASS+SMTP_PASS=${SMTP_PASS@Q}}
+${cmd}
+" $0 ${msg} || error "Command '\${${cmdvar}} ${msg}'
(cwd=${top}) failed"
+}
Sorry I didn't notice this before, and I am terrible at writing shell,
but perhaps you could share the reasoning behind writing this as above,
and not as, e.g.
(cd ${top} && env BLAH ${cmd})
avoiding all the verbiage in the description of ANNOUNCE_EDITOR about it
being fed into 'bash -c' (and hence getting evaluated twice??) rather
than just run?
None of the mentioned variables are exported to the environment by cygport.
I wanted to keep this fact in the subshell. Therefore the assignments are
added to the script instead of passing via env(ironment). The latter won't
even work with the PV variable because arrays could not be exported.
Variables would not be evaluated twice. For example in the rare case that
someone uses something like
SMTP_SERVER="smtp.$(hostname -d)"
in cygport.conf, this would immediately expand to
SMTP_SERVER="smtp.some.domain". The above
${SMTP_SERVER+SMTP_SERVER=${SMTP_SERVER@Q}}
would expand to
SMTP_SERVER=${SMTP_SERVER@Q}
and then to
SMTP_SERVER='smtp.some.domain'
(The @Q bash extension ensures proper quoting).
Using a subshell created by ( ... ) would achieve the behaviour you're
after, without requiring nearly so much quote handling. The code Jon
has pulled out could be rewritten as below; using ( ... ) would mean
that everything happens in a subshell and the exports don't affect the
rest of the environment.
```
(
cd "$top"
export HOMEPAGE P PF PN PR PV SMTP_SENDER SMTP_SERVER SMTP_SERVER_PORT
SMTP_ENCRYPTION SMTP_USER SMTP_PASS
This unnecessarily exports all variables (except PV, see below) to the
subcommands run by $cmd.
"$cmd" "$msg" || error "Command $cmd $msg (cwd=${top}) failed"
This would limit $cmd to simple commands instead of multiline scripts.
This reduces flexibility and some of the examples I provided in my
original post would no longer work:
https://sourceware.org/pipermail/cygwin-apps/2024-February/043501.html
)
```
I've removed the array handling for $PV, as it's not an array; possibly
you've confused it with $PVP?
No. PV it is initialized as a regular shell variable but is later
changed to an array by appending the PVP array.
/bin/cygport:
...
declare PV=${VERSION}
...
PV=$(echo ${PF} | sed -e "s/${PN}\-\(.*\)\-${PR}$/\1/");
...
declare -r PV=(${PV} ${PVP[*]});
...
$ git blame bin/cygport.in | grep 'declare -r PV='
deb528a88 (Yaakov Selkowitz 2009-12-31 08:05:52 + 397) declare -r
PV=(${PV} ${PVP[*]});
Bash silently ignores 'export PV'.
In any case, there is no way to pass an
array to "$cmd" unless "$cmd" is itself a Bash function, as there's no
standard way to store anything other than strings in environment
variables.
That's why I use 'PV=(${PV[*]@Q})' as a prefix of the configured $cmd
string instead of passing any new environment to $cmd.
I've also removed the `|| return 1` part, since cygport runs with `set
-e` enabled so you only want to catch non-zero return codes if you want
specific error handling.
There is no 'return 1' is my patch.
Finally, I've also added "$msg" to the arguments to "$cmd"; that seems
to be missing and seems to be critical to this working at all!
$msg is not missing in my patch but passed to the launched /bin/bash as $1.
Alternatively, if you really wanted to avoid the export statement, the
below will achieve the same thing; the only use of the subshell at this
point is to avoid the `cd` changing the working directory for the rest
of the script.
```
(
cd "$top"
HOMEPAGE="$HOMEPAGE" P="$P" PF="$PF" PN="$PN" PR="$PR" PV="$PV" SMTP_SENDER="$SMTP_SENDER" SMTP_SERVER="$SMTP_SERVER"
SMTP_SERVER_PORT="$SMTP_SERVER_PORT" SMTP_ENCRYPTION="$SMTP_ENCRYPTION" SMTP_USER="$SMTP_USER" SMTP_PASS="$SMTP_PASS" "$cmd" "$msg" || error "Command $cmd $msg
(cwd=${top}) failed"
)
```
Same problem with missing flexibility for $cmd as above.
Re: [PATCH cygport] Add check of SPDX expression provided by LICENSE variable
Brian Inglis via Cygwin-apps wrote: On 2024-04-30 15:07, Christian Franke via Cygwin-apps wrote: Brian Inglis via Cygwin-apps wrote: On 2024-04-30 11:45, Christian Franke via Cygwin-apps wrote: The new script uses the SPDX webpages to create the license file. I didn't find a usable single license list at https://github.com/spdx As usual, it is easier if you clearly state the purpose of the file you want, and its desired properties, like data content, format, etc. What about: https://spdx.github.io/license-list-data/ This is apparently a draft version of https://spdx.org/licenses/index.html which is used by the script to generate the local license file. Strip out the table entries and create what you want with a command or script. The spdx-check script from the patch optionally (-m, -u) downloads https://spdx.org/licenses/index.html and creates the local spdx-licenses file intended to distribute with cygport. The file is grep'able.and reduced to the bare minimum for this use case. and everything under: https://github.com/spdx/license-list-data I didn't find a single file which lists the licenses there. GH does not always make access easy, ... ... including that github.com is still unreachable via IPv6 without NAT64 (except for downloads from raw.githubusercontent.com) ... ... with its limited online displays and fixed display orders, and searches return a lot of junk, without easy access to better searching in context, but try: https://github.com/spdx/license-list-data/blob/main/licenses.md which also has xrefs to the text files; also there are: https://github.com/spdx/license-list-data/blob/main/json/licenses.json https://github.com/spdx/license-list-data/blob/main/json/exceptions.json which can be easily processed using `jq`. Indeed, thanks. I obviously missed these files when I wrote the spdx-check script some month ago. The current file format used by the script could then be created with: url="https://raw.githubusercontent.com/spdx/license-list-data/main/json; wget -O - "$url/licenses.json" \ | jq -j ' .licenses[] | ( if .isDeprecatedLicenseId then "!" else "" end, .licenseId, "\n" )' wget -O - "$url/exceptions.json" \ | jq -j ' .exceptions[] | ( if .isDeprecatedLicenseId then "!&" else "&" end, .licenseExceptionId, "\n" )' This adds these license ids not yet mentioned at https://spdx.org/licenses/index.html: AMD-newlib, BSD-2-clause-first-lines, Catharon, HPND-UC-export-US, MIT-Khronos-old, NCL, OAR, Sun-PPP-2000, pkgconf, threeparttable, xzoom I could provide a new patch with an updated script if desired.
Re: [PATCH cygport] Add check of SPDX expression provided by LICENSE variable
Brian Inglis via Cygwin-apps wrote: On 2024-04-30 11:45, Christian Franke via Cygwin-apps wrote: ... Attached. The new script uses the SPDX webpages to create the license file. I didn't find a usable single license list at https://github.com/spdx What about: https://spdx.github.io/license-list-data/ This is apparently a draft version of https://spdx.org/licenses/index.html which is used by the script to generate the local license file. and everything under: https://github.com/spdx/license-list-data I didn't find a single file which lists the licenses there.
[PATCH cygport] Add check of SPDX expression provided by LICENSE variable
Jon Turney via Cygwin-apps wrote (thread "[PATCH cygport] Add
repro-finish command"):
...
PS: I have a local script which checks SPDX Identifiers and
expressions. Any interest to add this to cygport and then check
LICENSE settings?
Oh, yes please. That sounds like a good idea.
Attached.
The new script uses the SPDX webpages to create the license file. I
didn't find a usable single license list at https://github.com/spdx
The data/spdx-licenses file is not included in the patch. It could be
generated from the source dir with:
$ tools/spdx-check -f data/spdx-licenses -m
...
data/spdx-licenses: created
$ sha1sum data/spdx-licenses
80a19d6891d08bf34113464464ee12308374c792 *data/spdx-licenses
The changes to the meson files are guessed. I didn't test the meson
build yet.
--
Regards,
Christian
From 61f75757fa8e9118207cc09cf4a621aac8a4da78 Mon Sep 17 00:00:00 2001
From: Christian Franke
Date: Tue, 30 Apr 2024 19:28:01 +0200
Subject: [PATCH] Add check of SPDX expression provided by LICENSE variable
The new script 'tools/spdx-checks' checks a SPDX license expression.
License identifiers are provided by the new file 'spdx-licenses'
which could be created by the script from the related SPDX webpages.
---
bin/cygport.in| 17
data/meson.build | 1 +
tools/meson.build | 1 +
tools/spdx-check | 198 ++
4 files changed, 217 insertions(+)
create mode 100644 tools/spdx-check
diff --git a/bin/cygport.in b/bin/cygport.in
index 15bd559e..3166beba 100755
--- a/bin/cygport.in
+++ b/bin/cygport.in
@@ -41,6 +41,7 @@ declare -r _cygport_version=@VERSION@;
declare -r _privdatadir=@pkgdatadir@;
declare -r _privclassdir=@cygclassdir@;
declare -r _privlibdir=@cygpartdir@;
+declare -r _privtoolsdir=@pkgdatadir@/tools;
declare -r _privgnuconfigdir=@gnuconfigdir@;
declare -r _privsysconfdir=@sysconfdir@;
@@ -489,6 +490,22 @@ do
fi
done
+if [ "${LICENSE+y}" = "y" ]
+then
+ if ! _out=$(${_privtoolsdir}/spdx-check -f
${_privdatadir}/spdx-licenses "${LICENSE}" 2>&1)
+ then
+ warning "LICENSE='${LICENSE}' is invalid:"
+ echo "${_out}"
+ elif [ "${_out:+y}" = "y" ]
+ then
+ warning "LICENSE='${LICENSE}' has warnings:"
+ echo "${_out}"
+ else
+ inform "LICENSE='${LICENSE}' is valid"
+ fi
+ unset _out
+fi
+
for restrict in ${RESTRICT//,/ }
do
declare _CYGPORT_RESTRICT_${restrict//-/_}_=1
diff --git a/data/meson.build b/data/meson.build
index 51c6a5fd..e83a90fe 100644
--- a/data/meson.build
+++ b/data/meson.build
@@ -2,6 +2,7 @@ datadocs = files('cygport.conf', 'mirrors')
install_data('mirrors',
'sample.cygport',
+ 'spdx-licenses',
install_dir: pkgdatadir)
install_data('gnuconfig/config.guess',
diff --git a/tools/meson.build b/tools/meson.build
index acd83926..96d8d19e 100644
--- a/tools/meson.build
+++ b/tools/meson.build
@@ -1,6 +1,7 @@
tools = files(
'deb2targz',
'pkgrip',
+'spdx-check',
'sysrootize'
)
diff --git a/tools/spdx-check b/tools/spdx-check
new file mode 100644
index ..bffcaae0
--- /dev/null
+++ b/tools/spdx-check
@@ -0,0 +1,198 @@
+#! /bin/bash
+#######
+#
+# spdx-check - check SPDX license expression
+#
+# Copyright (C) 2024 Christian Franke
+#
+# SPDX-License-Identifier: BSD-3-Clause
+#
+
+
+set -e -o pipefail
+myname=$0
+
+# SPDX license list web pages
+spdx_url_lic="https://spdx.org/licenses/index.html;
+spdx_url_exc="https://spdx.org/licenses/exceptions-index.html;
+
+# Default license file
+def_spdx_file="$(dirname "$myname")/spdx-licenses"
+
+usage()
+{
+ cat <&2
+ exit 1
+}
+
+warning()
+{
+ echo "Warning:" "$@" >&2
+}
+
+check_spdx_id()
+{
+ local id=$1
+ local m m_id
+
+ if ! [ -f "$spdx_file" ]; then
+warning "Missing '$spdx_file' - SPDX identifier '$1' not checked"
+return 0
+ fi
+
+ # SPDX identifiers are case insensitive but the correct case is recommended
+ m=$(grep -Ei -m 1 "^!?&?${id//+/\\+}\$" "$spdx_file" 2>/dev/null) \
+|| error "Unknown SPDX identifier '$id'"
+
+ # TODO: Distinguish licenses and exceptions
+ m_id=${m#!}; m_id=${m_id#&}
+
+ [ "$m_id" = "$id" ] || warning "It is recommended to use '$m_id' instead of
'$id'"
+ [ "$m" = "${m#!}" ] || warning "SPDX identifier '$m_id' is deprecated"
+}
+
+check_spdx_expr()
+{
+ local x=$1
+ local f s t
+
+ # Insert spaces around tokens to simplify parsing
+ x=" $x "; x=${x//(/ ( }; x=${
Re: [PATCH cygport] Add customization support for announce command
Jon Turney wrote:
On 10/03/2024 16:33, Christian Franke via Cygwin-apps wrote:
Jon Turney wrote:
On 23/02/2024 11:23, Christian Franke via Cygwin-apps wrote:
Christian Franke wrote:
The email generated by the cygport announce command is useful, but
actual use cases are somewhat limited due to the hard-coded email
submission.
The attached patch adds more flexibility. The patch is on top of
the "Use correct wording if only one package is announced" patch.
Slightly changed patch attached. Also adjusted to new version of
"Use correct wording if only one package is announced" patch.
[...]
Thanks for this.
Possible (better?) alternative names for the new settings:
ANNOUNCEMENT_EDITOR
ANNOUNCEMENT_MAILER
Hmmm... I think "ANNOUNCE_EDITOR" and "ANNOUNCE_MAILER" would be
the best for clarity and conciseness.
New patch attached. Is still on top of "Use correct wording ..." patch.
I also added HOMEPAGE to the propagated variables as this should be
included in an announcement.
Thanks.
+ /bin/bash -c "cd ${top} || exit 1
+${HOMEPAGE+HOMEPAGE=${HOMEPAGE@Q}}
+P=${P@Q}; PF=${PF@Q}; PN=${PN@Q}; PR=${PR@Q}; PV=(${PV[*]@Q})
+${SMTP_SENDER+SMTP_SENDER=${SMTP_SENDER@Q}}
+${SMTP_SERVER+SMTP_SERVER=${SMTP_SERVER@Q}}
+${SMTP_SERVER_PORT+SMTP_SERVER_PORT=${SMTP_SERVER_PORT@Q}}
+${SMTP_ENCRYPTION+SMTP_ENCRYPTION=${SMTP_ENCRYPTION@Q}}
+${SMTP_USER+SMTP_USER=${SMTP_USER@Q}}
+${SMTP_PASS+SMTP_PASS=${SMTP_PASS@Q}}
+${cmd}
+" $0 ${msg} || error "Command '\${${cmdvar}} ${msg}'
(cwd=${top}) failed"
+}
Sorry I didn't notice this before, and I am terrible at writing shell,
but perhaps you could share the reasoning behind writing this as
above, and not as, e.g.
(cd ${top} && env BLAH ${cmd})
avoiding all the verbiage in the description of ANNOUNCE_EDITOR about
it being fed into 'bash -c' (and hence getting evaluated twice??)
rather than just run?
None of the mentioned variables are exported to the environment by
cygport. I wanted to keep this fact in the subshell. Therefore the
assignments are added to the script instead of passing via
env(ironment). The latter won't even work with the PV variable because
arrays could not be exported.
Variables would not be evaluated twice. For example in the rare case
that someone uses something like
SMTP_SERVER="smtp.$(hostname -d)"
in cygport.conf, this would immediately expand to
SMTP_SERVER="smtp.some.domain". The above
${SMTP_SERVER+SMTP_SERVER=${SMTP_SERVER@Q}}
would expand to
SMTP_SERVER=${SMTP_SERVER@Q}
and then to
SMTP_SERVER='smtp.some.domain'
(The @Q bash extension ensures proper quoting).
Re: [PATCH cygport] Increase _FORTIFY_SOURCE level from 2 to 3 in CFLAGS
ASSI via Cygwin-apps wrote: Christian Franke via Cygwin-apps writes: _FORTIFY_SOURCE=3 is supported by Cygwin 3.5.0 headers and Cygwin gcc 13.2.1 test release. Silently falls back to level 2 if level 3 is unsupported (older headers or gcc) or to level 0 if unsupported at all (C++, clang). Well, if only that was the case… --8<---cut here---start->8--- from /usr/include/w32api/windows.h:9, from /mnt/share/cygpkgs/libarchive/libarchive.x86_64/src/libarchive-3.7.4/test_utils/test_common.h:88, from /mnt/share/cygpkgs/libarchive/libarchive.x86_64/src/libarchive-3.7.4/tar/test/test.h:38, from /mnt/share/cygpkgs/libarchive/libarchive.x86_64/src/libarchive-3.7.4/tar/test/test_extract_tar_lrz.c:25: /usr/include/w32api/_mingw_mac.h:319:8: warning: #warning Using _FORTIFY_SOURCE=2 (level 3 requires __builtin_dynamic_object_size support) [-Wcpp] 319 | # warning Using _FORTIFY_SOURCE=2 (level 3 requires __builtin_dynamic_object_size support) --8<---cut here---end--->8--- Can't we conditiohnalize this to depend on the actual compiler support? This is a bogus warning. Sorry, my bad. In my contribution of _FORTIFY_SOURCE support to MinGW-w64 from 2019, I didn't realize that these warnings also appear if only Win32 API includes (windows.h, ...) are used. The related internal macros have only an effect if MinGW-w64 runtime includes (stdio.h, string.h, ...) are used. Meantime this has been fixed upstream: https://sourceforge.net/p/mingw-w64/mingw-w64/ci/f8e088e -- Regards, Christian
Re: [ITP] afflib 3.7.20-1
Christian Franke wrote: marco atzeri wrote: On Wed, Mar 6, 2024 at 11:26 PM Christian Franke via Cygwin-apps wrote: ... Should I also rename libtsk to libtsk19 in the planned sleutkit-*-2 package which will add afflib support ? yes please The original package is only a few days old and has possibly only a small but experienced audience, so I expect not much worries if the change will be explained in the announcement. not worries at all if you use libtsk19_OBSOLETES=libtsk see https://cygwin.github.io/cygport/pkg_pkg_cygpart.html#PKG_OBSOLETES Thanks for the info. I will do this that way when libafflib0 package is available. Just for info: This worked as expected. Setup automatically selected "Uninstall" for libtsk.
Re: [ITP] afflib 3.7.20-1
On Wed, 6 Mar 2024 23:26:05 +0100, Christian Franke wrote: Jon Turney wrote: ... be added only when needed for new not backward compatible releases. The upstream afflib project is mostly idling, so I don't expect any new major lib versions in the near future. If course, I could rename it to libafflib0 if desired. As far as I know, there is no cost for doing this, and it saves grief if upstream ever bumps the soversion. Also, it's probably best to explicitly list the filename with soversion in the CONTENTS, so that if upstream ever does change the soversion, it will be detected as a packaging failure, rather than producing a package with a mismatch between the soversion in it's name and in it's contents. Good point, new cygport file is attached. Any further issues with this ITP?
[ITP] mandoc 1.14.6-1
I would like to contribute mandoc. Also present in Debian, Fedora,
Ubuntu, ... and as the default man page formatter on *BSD.
Useful to check man pages for compatibility with *BSD systems.
The build is reproducible without the need to export SOURCE_DATE_EPOCH.
SUMMARY="BSD mandoc compiler toolset"
DESCRIPTION="\
mandoc is a suite of tools compiling mdoc, the roff macro language of
choice for BSD manual pages, and man, the predominant historical language
for UNIX manuals. It is small, self-contained, and quite fast. The main
component of the toolset is the mandoc utility program, based on the
libmandoc validating compiler, to format output for UTF-8 and ASCII
terminals, HTML 5, PostScript, and PDF."
mandoc-1.14.6-1.tar.xz:
usr/bin/demandoc.exe
usr/bin/mandoc.exe
usr/bin/mapropos -> mandoc
usr/bin/mman -> mandoc
usr/bin/msoelim.exe
usr/bin/mwhatis -> mandoc
usr/sbin/mandocdb -> ../bin/mandoc
usr/share/doc/mandoc/*
usr/share/man/man1/demandoc.1.gz
usr/share/man/man1/mandoc.1.gz
usr/share/man/man1/mapropos.1.gz
usr/share/man/man1/mman.1.gz
usr/share/man/man1/msoelim.1.gz
usr/share/man/man1/mwhatis.1.gz
usr/share/man/man5/mandoc.conf.5.gz
usr/share/man/man5/mandoc.db.5.gz
usr/share/man/man7/mandoc_char.7.gz
usr/share/man/man7/mandoc_eqn.7.gz
usr/share/man/man7/mandoc_man.7.gz
usr/share/man/man7/mandoc_mdoc.7.gz
usr/share/man/man7/mandoc_roff.7.gz
usr/share/man/man7/mandoc_tbl.7.gz
usr/share/man/man8/mandocdb.8.gz
--
Regards,
Christian
# cygport script for mandoc
NAME=mandoc
VERSION=1.14.6
RELEASE=1
SOURCE_DATE="2024-03-11 18:00:00 UTC"
SUMMARY="BSD mandoc compiler toolset"
DESCRIPTION="\
mandoc is a suite of tools compiling mdoc, the roff macro language of
choice for BSD manual pages, and man, the predominant historical language
for UNIX manuals. It is small, self-contained, and quite fast. The main
component of the toolset is the mandoc utility program, based on the
libmandoc validating compiler, to format output for UTF-8 and ASCII
terminals, HTML 5, PostScript, and PDF."
LICENSE="ISC"
CATEGORY="Text"
REQUIRES="" # zlib0
BUILD_REQUIRES="binutils gcc-core perl_base" # make
HOMEPAGE="https://mandoc.bsd.lv/;
SRC_URI="https://mandoc.bsd.lv/snapshots/${P}.tar.gz;
SOURCE_DATE_EPOCH=$(date -d "${SOURCE_DATE}" +%s) # 'export' is not needed
src_compile() {
cd ${B}
lndirs
cat <<-EOF > configure.local
OSNAME="Cygwin"
PREFIX="/usr"
MANDIR="/usr/share/man"
MANPATH_BASE="/usr/share/man"
MANPATH_DEFAULT="/usr/share/man:/usr/local/share/man"
HAVE_WCHAR=1
CFLAGS="${CFLAGS}"
LDFLAGS="${LDFLAGS}"
LN="ln -sf"
BINM_APROPOS="mapropos" # "apropos"
BINM_MAKEWHATIS="mandocdb" # "makewhatis"
BINM_MAN="mman" # "man"
BINM_SOELIM="msoelim" # "soelim"
BINM_WHATIS="mwhatis" # "whatis"
MANM_EQN="mandoc_eqn" # "eqn"
MANM_MANCONF="mandoc.conf" # "man.conf"
MANM_MAN="mandoc_man" # "man"
MANM_MDOC="mandoc_mdoc" # "mdoc"
MANM_ROFF="mandoc_roff" # "roff"
MANM_TBL="mandoc_tbl" # "tbl"
EOF
# No cygconf because ./configure is not generated
./configure
cygmake
}
src_test() {
cd ${B}/regress
./regress.pl . ascii tag man html markdown lint
# Unicode chars >= U+1 do not work
# U+1D6C1 (Mathematical Bold Nabla) is output as U+D6C1 (Hangul
Syllable Hyot)
./regress.pl . utf8 ||
inform "The above failure of the 'nabla' testcase could be
safely ignored"
}
[PATCH cygport] Add repro-finish command
Thanks for accepting the repro-check patch. A minor enhancement is attached.
The function is in pkg_pkg.cygpart instead of pkg_cleanup.cygpart
because then it is easier to keep it in sync with the other __repro_*
functions.
PS: I have a local script which checks SPDX Identifiers and expressions.
Any interest to add this to cygport and then check LICENSE settings?
--
Regards,
Christian
From b08796262308cf1b3a2c063349d024a5ccfd2455 Mon Sep 17 00:00:00 2001
From: Christian Franke
Date: Mon, 11 Mar 2024 12:12:32 +0100
Subject: [PATCH] Add repro-finish command
This command removes the temporary directory used by repro-check.
---
README | 7 ---
bin/cygport.in | 4
lib/help.cygpart| 1 +
lib/pkg_pkg.cygpart | 24 +++-
4 files changed, 32 insertions(+), 4 deletions(-)
diff --git a/README b/README
index 3c9e4d4a..a0897a4f 100644
--- a/README
+++ b/README
@@ -163,9 +163,10 @@ Other COMMANDs are meant primarily for maintainers:
diff - write a patch file capturing changes to source in the working
directory
stage- as upload, but don't request processing of uploaded packages
announce - compose and send a package announcement
-repro-build - rebuild from created source package to temp directory
-repro-diff - check whether packages from original and rebuild differ
-repro-check - run repro-build and repro-diff
+repro-build - rebuild from created source package to temp directory
+repro-diff - check whether packages from original and rebuild differ
+repro-check - run repro-build and repro-diff
+repro-finish - delete the temp directory used for rebuild
The standard arguments --help or --version may also be passed to cygport.
diff --git a/bin/cygport.in b/bin/cygport.in
index df38a8b5..15bd559e 100755
--- a/bin/cygport.in
+++ b/bin/cygport.in
@@ -801,6 +801,10 @@ do
__pkg_repro_diff
_status=$?
;;
+ repro-finish)
+ __pkg_repro_finish
+ _status=$?
+ ;;
help)
__show_help;
exit 0;
diff --git a/lib/help.cygpart b/lib/help.cygpart
index d28fd7bb..ff03fb5f 100644
--- a/lib/help.cygpart
+++ b/lib/help.cygpart
@@ -59,6 +59,7 @@ __show_help() {
repro-build rebuild from created source package to temp
directory
repro-diffcheck whether packages from original and
rebuild differ
repro-check run repro-build and repro-diff
+ repro-finish delete the temp directory used for rebuild
See the included README file for further documentation.
diff --git a/lib/pkg_pkg.cygpart b/lib/pkg_pkg.cygpart
index 25b80906..07313c66 100644
--- a/lib/pkg_pkg.cygpart
+++ b/lib/pkg_pkg.cygpart
@@ -1048,6 +1048,28 @@ __pkg_repro_diff() {
inform "Rebuild produced identical packages"
}
+__pkg_repro_finish() {
+ local t_spkgdir=${T}/${spkgdir##*/}
+
+ cd ${top}
+
+ __step "Removing rebuild directory in 2 seconds..."
+ inform "Rebuild dir: ${t_spkgdir}"
+ if ! [ -d ${t_spkgdir} ]
+ then
+ inform "Rebuild directory does not exist"
+ return 0
+ fi
+
+ sleep 2 || exit $?
+
+ __step "Removing rebuild directory NOW."
+ rm -rf ${t_spkgdir}
+
+ __step "Finished."
+}
+
# protect functions
readonly -f __pkg_binpkg __pkg_diff __gpg_sign __pkg_srcpkg __pkg_dist \
-__pkg_repro_build __pkg_repro_diff __squeeze_whitespace __tar
+__pkg_repro_build __pkg_repro_diff __pkg_repro_finish \
+__squeeze_whitespace __tar
--
2.43.0
Re: [PATCH cygport] Add customization support for announce command
Jon Turney wrote:
On 23/02/2024 11:23, Christian Franke via Cygwin-apps wrote:
Christian Franke wrote:
The email generated by the cygport announce command is useful, but
actual use cases are somewhat limited due to the hard-coded email
submission.
The attached patch adds more flexibility. The patch is on top of the
"Use correct wording if only one package is announced" patch.
Slightly changed patch attached. Also adjusted to new version of "Use
correct wording if only one package is announced" patch.
[...]
Thanks for this.
Possible (better?) alternative names for the new settings:
ANNOUNCEMENT_EDITOR
ANNOUNCEMENT_MAILER
Hmmm... I think "ANNOUNCE_EDITOR" and "ANNOUNCE_MAILER" would be
the best for clarity and conciseness.
New patch attached. Is still on top of "Use correct wording ..." patch.
I also added HOMEPAGE to the propagated variables as this should be
included in an announcement.
-From: ${SMTP_SENDER}
-To: cygwin-annou...@cygwin.com
+${SMTP_SENDER:+From: ${SMTP_SENDER}
+}To: cygwin-annou...@cygwin.com
Date: $(date -R --date=${msgat})
-Message-Id: <$(date "+%Y%m%d%H%M%S.$$" --date=${msgat})-1-$(echo
${SMTP_SENDER} | sed 's|.*<\(.*\)>.*|\1|')>
+Message-Id: <$(date "+%Y%m%d%H%M%S.$$" --date=${msgat})-1-$(echo
${SMTP_SENDER:-cygport} | sed 's|.*<\(.*\)>.*|\1|')>
Subject: ${NAME} ${PVR}
Can you also explain what this is doing in the commit message, since
it's not immediately apparent.
If the mail infrastructure always replaces the "From:" line or the
default one is sufficient, then there is no need to generate one.
SMTP_SENDER could be left alone then. I added a related comment to
cygport.conf
From 335cbde3c6c2450051cc739cee60a555b236843e Mon Sep 17 00:00:00 2001
From: Christian Franke
Date: Sun, 10 Mar 2024 17:28:09 +0100
Subject: [PATCH] Add customization support for announce command
Two new configuration settings allow to override the launch of
a text editor (ANNOUNCE_EDITOR) and the builtin email submission
(ANNOUNCE_MAILER). Don't create a "From:" header line if
SMTP_SENDER is undefined or empty.
---
data/cygport.conf | 27 +++-
lib/pkg_upload.cygpart | 57 +-
2 files changed, 77 insertions(+), 7 deletions(-)
diff --git a/data/cygport.conf b/data/cygport.conf
index 34ccd291..3da744d9 100644
--- a/data/cygport.conf
+++ b/data/cygport.conf
@@ -101,10 +101,35 @@
#PAGER=
+#v* Configuration/ANNOUNCE_EDITOR
+# DESCRIPTION
+# Shell command string to process the email message created by cygport's
+# announce command before sending the email. If undefined, a text editor
+# will be run, see EDITOR setting above. If empty, nothing will be run.
+# If not empty, '/bin/bash' will be launched with the command string passed
+# with '-c' option and the path of the temporary email message file as '$1'.
+# The working directory of the shell will be the directory of the cygport
+# file. The specified command string will be prepended by shell assignments
+# of the cygport variables HOMEPAGE, P, PF, PN, PR and PV and all SMTP_*
+# settings described below.
+#ANNOUNCE_EDITOR=
+
+#v* Configuration/ANNOUNCE_MAILER
+# DESCRIPTION
+# Shell command string to process the email message created by cygport's
+# announce command after editing. If undefined, the email will be sent
+# using the builtin perl-based SMTP support. If empty, nothing will be run.
+# If not empty, the command string will be handled similar to ANNOUNCE_EDITOR
+# described above.
+#ANNOUNCE_MAILER=
+
+
#v* Configuration/SMTP_SENDER
# DESCRIPTION
# Name and email address, in the form of "First Last " to be used
-# by cygport's announcement command.
+# by cygport's announcement command. If undefined or empty, no "From:" email
+# header line will be generated. The local mail tool or the mail provider may
+# unconditionally replace this header line or only the "" part.
# NOTE
# Many webmail services do not allow using arbitrary sender address in SMTP
# mail, or may first require registering other email addresses as authorized
diff --git a/lib/pkg_upload.cygpart b/lib/pkg_upload.cygpart
index 37bc2d63..9ced1fb5 100644
--- a/lib/pkg_upload.cygpart
+++ b/lib/pkg_upload.cygpart
@@ -168,6 +168,34 @@ EOF
echo "Upload complete."
}
+__pkg_announce_run_cmd_on_msg() {
+ local cmdvar=$1
+ local msg=$2
+ local cmd
+
+ eval cmd="\${${cmdvar}}"
+
+ if [ "${cmd:+y}" != "y" ]
+ then
+ inform "\${${cmdvar}} is empty"
+ return 0
+ fi
+ echo
+ inform "Launching '\${${cmdvar}} ${msg}'"
+
+ /bin/bash -c "cd ${top} || exit 1
+${HOMEPAGE+HOMEPAGE=${HOMEPAGE@Q}}
+P=${P@Q}; PF=${PF@Q}; PN=${PN@Q}; PR=
Re: [PATCH cygport] dodoc: Skip a file if a compressed version already exists
Jon Turney wrote:
On 01/03/2024 13:13, Christian Franke via Cygwin-apps wrote:
It IMO makes sense to compress large and rarely viewed doc files like
change logs. This seems to be common practice on Debian etc.
With current cygport, the following results in ChangeLog and
ChangeLog.gz in the docdir:
src_install()
{
...
dodoc ChangeLog
gzip -9 -n "${D}/usr/share/doc/${PN}/ChangeLog"
}
Uh, I don't quite see how this patch will change the behavior of this
fragment.
Yes, it actually doesn't change the behavior of this fragment itself.
Even more confusing, why isn't this already doing what you want?
Unless you specify -k/--keep to gzip, the input file is removed, right?
Yes - but after this src_install() the file will be re-added by
__predoc() unless _CYGPORT_RESTRICT_postinst_doc_ is set. The patch
avoids this because __predoc() also uses dodoc().
Re: [PATCH cygport] Add more checks of SOURCE_DATE_EPOCH
Jon Turney wrote: On 26/02/2024 19:53, Christian Franke via Cygwin-apps wrote: Would it not make more sense to just re-export it if set? If the cygport file decides to set but not export it, there is possibly no need to do it. An example is smartmontools.cygport which passes the unexported variable as a parameter to configure. Ok, but exporting it is harmless there, right? I'm not aware of any corner cases where exporting would break something. But leaving the decision to the user would allow to handle such cases. It would also allow to check whether it makes a difference and if yes, which files are affected. (so that commands like "SOURCE_DATE_EPOCH=something cygport foo" work as expected?) Would make no difference as the 'VAR=val CMD...' syntax already exports the variable to the CMD: $ unset FOO; FOO=bar sh -c 'sh -c "sh -c printenv\ FOO"' bar Ah, right. So you seem to be saying that the only situation where it's set but not exported is where it's set in the cygport. So we're just making people (need to remember to) explicitly write "export SOURCE_DATE_EPOCH" in their cygport where needed? Exactly.
[ITP] bmake 20240301-1
I would like to contribute bmake. Also present in Debian, Fedora,
FreeBSD, Ubuntu, ...
I occasionally use it to check whether Makefiles are compatible with
non-GNU versions of make.
SUMMARY="Portable version of the NetBSD 'make' utility"
DESCRIPTION="\
bmake is a portable version of the NetBSD make(1) utility. It is similar
to GNU make, even though the syntax for the advanced features supported
in Makefiles is very different."
bmake_extras_SUMMARY="${SUMMARY} (additional support files)"
bmake_extras_DESCRIPTION="${DESCRIPTION}
This package contains many additional *.mk files and some support scripts
which are only required if used in Makefiles."
bmake-20240301-1.tar.xz:
usr/bin/bmake.exe
usr/share/doc/bmake/*
usr/share/man/man1/bmake.1.gz
usr/share/bmake/mk/host-target.mk
usr/share/bmake/mk/suffixes.mk
usr/share/bmake/mk/sys.mk
usr/share/bmake/mk/sys.*.mk
usr/share/bmake/mk/sys/Generic.mk
bmake-extras-20240301-1.tar.xz:
usr/share/bmake/mk/*.mk
usr/share/bmake/mk/install-sh
usr/share/bmake/mk/meta2deps.py
usr/share/bmake/mk/meta2deps.sh
usr/share/bmake/mk/mkopt.sh
usr/share/bmake/mk/stage-install.sh
usr/share/bmake/mk/sys/*.mk
--
Regards,
Christian
# cygport script for bmake
NAME=bmake
VERSION=20240301
RELEASE=1
SOURCE_DATE="2024-03-09 12:00:00 UTC"
SUMMARY="Portable version of the NetBSD 'make' utility"
DESCRIPTION="\
bmake is a portable version of the NetBSD make(1) utility. It is similar
to GNU make, even though the syntax for the advanced features supported
in Makefiles is very different."
LICENSE="BSD-3-Clause"
CATEGORY="Devel"
PKG_NAMES="bmake bmake-extras"
bmake_extras_SUMMARY="${SUMMARY} (additional support files)"
bmake_extras_DESCRIPTION="${DESCRIPTION}
This package contains many additional *.mk files and some support scripts
which are only required if used in Makefiles."
REQUIRES=""
bmake_extras_REQUIRES="bmake" # bash python3
BUILD_REQUIRES="binutils gcc-core" # "make" not required
HOMEPAGE="https://www.crufty.net/help/sjg/bmake.htm;
SRC_URI="https://www.crufty.net/ftp/pub/sjg/bmake-${PV}.tar.gz
https://www.crufty.net/ftp/pub/sjg/bmake-${PV}.tar.gz.asc;
SRC_DIR="bmake"
bmake_CONTENTS="
usr/bin
usr/share/doc
usr/share/man
usr/share/${PN}/mk/host-target.mk
usr/share/${PN}/mk/suffixes.mk
usr/share/${PN}/mk/sys.mk
usr/share/${PN}/mk/sys.*.mk
usr/share/${PN}/mk/sys/Generic.mk
"
bmake_extras_CONTENTS="
--exclude=usr/share/${PN}/mk/host-target.mk
--exclude=usr/share/${PN}/mk/suffixes.mk
--exclude=usr/share/${PN}/mk/sys.mk
--exclude=usr/share/${PN}/mk/sys.*.mk
--exclude=usr/share/${PN}/mk/sys/Generic.mk
usr/share/${PN}
"
SOURCE_DATE_EPOCH=$(date -d "$SOURCE_DATE" +%s) # 'export' not needed
src_compile() {
cd ${B}
${S}/boot-strap --prefix=/usr \
--with-default-sys-path=/usr/share/${PN}/mk \
--skip-test op=build
}
src_test() {
cd ${B}
${S}/boot-strap op=test
if false; then # BROKEN_TESTS (all harmless)
TESTS='export opt-chdir opt-keep-going-indirect' \
${S}/boot-strap op=test
fi
}
src_install() {
cd ${B}
# Don't use 'install -s' for bmake.exe.
STRIP_FLAG="" \
${S}/boot-strap --prefix=/usr \
--install-destdir=${D} \
--skip-test op=install
}
--- origsrc/bmake/Makefile 2024-03-09 12:00:00.0 +
+++ src/bmake/Makefile 2024-03-09 12:00:00.0 +
@@ -90,7 +90,7 @@ OS := ${.MAKE.OS:U${uname -s:L:sh}}
# are we 4.4BSD ?
isBSD44:=${BSD44_LIST:M${OS}}
-.if ${isBSD44} == "" && ${OS:NDarwin:NLinux} != ""
+.if ${isBSD44} == "" && ${OS:NCYGWIN*:NDarwin:NLinux} != ""
MANTARGET= cat
INSTALL?=${srcdir}/install-sh
.if ${MACHINE} == "sun386"
@@ -192,7 +192,7 @@ CONFIGURE_ARGS += --without-makefile
AUTOCONF_GENERATED_MAKEFILE = Makefile.config
.include
.endif
-SHARE_MK ?= ${SHAREDIR}/mk
+SHARE_MK ?= ${DEFAULT_SYS_PATH}
MKSRC = ${srcdir}/mk
INSTALL ?= ${srcdir}/install-sh
--- origsrc/bmake/boot-strap2024-03-09 12:00:00.0 +
+++ src/bmake/boot-strap2024-03-09 12:00:00.0 +
@@ -69,6 +69,9 @@
# This is useful when $prefix/ is shared by multiple
# machines.
#
+# --skip-test
+# Don't run test after build or before install.
+#
# Flags relevant when installing:
#
# -DWITHOUT_INSTALL_MK
@@ -212,6 +215,7 @@ srcdir=$Mydir
mksrc=$Mydir/mk
objdir=
quiet=:
+skip_test=false
${SKIP_RC:+:} source_rc .bmake-boot-strap.rc . "$Mydir/.." "$HOME"
@@ -249,6 +253,7 @@ do
INSTALL_BIN=$HOST_TARGET/bin;;
--install-destdir=*) INSTALL_DESTDIR=`get_optarg "$1"`;;
--install-prefix=*) INSTALL_PREFIX=`get_optarg "$1"`;;
+ --skip-test) skip_test=true;;
-DWITH*) INSTALL_ARGS="$INSTALL_ARGS $1";;
-s|--src)
Re: [ITP] afflib 3.7.20-1
marco atzeri wrote: On Wed, Mar 6, 2024 at 11:26 PM Christian Franke via Cygwin-apps wrote: Jon Turney wrote: On 06/03/2024 15:39, Christian Franke via Cygwin-apps wrote: Jon Turney wrote: Thanks! libafflib_CONTENTS=" usr/bin/cygafflib-*.dll Any reason why this package doesn't include the soversion, i.e. why not libafflib0? Libtsk and libafflib are my first library packages, so I'm not sure what the policy is. My recent package libtsk has been accepted without soversion, so I omitted it also here. I assumed that the soversion will I'm going to suggest that was an oversight in the review. Should I also rename libtsk to libtsk19 in the planned sleutkit-*-2 package which will add afflib support ? yes please The original package is only a few days old and has possibly only a small but experienced audience, so I expect not much worries if the change will be explained in the announcement. not worries at all if you use libtsk19_OBSOLETES=libtsk see https://cygwin.github.io/cygport/pkg_pkg_cygpart.html#PKG_OBSOLETES Thanks for the info. I will do this that way when libafflib0 package is available. -- Regards, Christian
Re: [ITP] afflib 3.7.20-1
Jon Turney wrote:
On 06/03/2024 15:39, Christian Franke via Cygwin-apps wrote:
Jon Turney wrote:
Thanks!
libafflib_CONTENTS="
usr/bin/cygafflib-*.dll
Any reason why this package doesn't include the soversion, i.e. why
not libafflib0?
Libtsk and libafflib are my first library packages, so I'm not sure
what the policy is. My recent package libtsk has been accepted
without soversion, so I omitted it also here. I assumed that the
soversion will
I'm going to suggest that was an oversight in the review.
Should I also rename libtsk to libtsk19 in the planned sleutkit-*-2
package which will add afflib support ?
The original package is only a few days old and has possibly only a
small but experienced audience, so I expect not much worries if the
change will be explained in the announcement.
be added only when needed for new not backward compatible releases.
The upstream afflib project is mostly idling, so I don't expect any
new major lib versions in the near future.
If course, I could rename it to libafflib0 if desired.
As far as I know, there is no cost for doing this, and it saves grief
if upstream ever bumps the soversion.
Also, it's probably best to explicitly list the filename with
soversion in the CONTENTS, so that if upstream ever does change the
soversion, it will be detected as a packaging failure, rather than
producing a package with a mismatch between the soversion in it's name
and in it's contents.
Good point, new cygport file is attached.
(Cygport should perhaps and detect and warn about apparently
soversioned libraries that aren't in appropriately named packages,
but...)
... this is possibly a good idea.
# cygport script for afflib
NAME=afflib
VERSION=3.7.20
RELEASE=1
SOURCE_DATE="2024-03-06 22:00:00 UTC"
SUMMARY="Library and tools for the Advanced Forensic Format"
DESCRIPTION="\
The Advanced Forensic Format (AFF) is a file format for storing computer
forensic information. It supports metadata, compression, encryption and
signing."
LICENSE="BSD-4-clause"
PKG_NAMES="afflib-tools libafflib0 libafflib-devel"
afflib_tools_SUMMARY="${SUMMARY} (tools)"
afflib_tools_DESCRIPTION="${DESCRIPTION}
This package contains tools to examine, convert, compare, copy, encrypt,
decrypt and sign AFF files."
libafflib0_SUMMARY="${SUMMARY} (runtime)"
libafflib0_DESCRIPTION="${DESCRIPTION}
This package contains the runtime library for afflib."
libafflib_devel_SUMMARY="${SUMMARY} (development)"
libafflib_devel_DESCRIPTION="${DESCRIPTION}
This package contains the development files for libafflib0."
CATEGORY="Devel Libs Utils"
afflib_tools_CATEGORY="Utils"
libafflib0_CATEGORY="Libs"
libafflib_devel_CATEGORY="Devel Libs"
afflib_tools_REQUIRES="" # libafflib0 libexpat1 libgcc1 libreadline7 libssl3
libstdc++6
libafflib0_REQUIRES="" # libcurl4 libexpat1 libgcc1 libssl3 libstdc++6 zlib0
libafflib_devel_REQUIRES="" # libafflib0 libssl-devel pkg-config
BUILD_REQUIRES="
binutils gcc-g++ gzip libcurl-devel libexpat-devel libreadline-devel
libssl-devel zlib-devel
" # make
HOMEPAGE="https://github.com/sshock/AFFLIBv3;
SRC_URI="https://codeload.github.com/sshock/AFFLIBv3/tar.gz/refs/tags/v${PV}#/${P}.tar.gz;
SRC_DIR="AFFLIBv3-${PV}"
afflib_tools_CONTENTS="
--exclude=usr/bin/cygafflib-0.dll
usr/bin
usr/share
"
libafflib0_CONTENTS="
usr/bin/cygafflib-0.dll
"
libafflib_devel_CONTENTS="
usr/include/afflib
usr/lib
"
DIFF_EXCLUDES="lzma443"
export SOURCE_DATE_EPOCH=$(date -d "$SOURCE_DATE" +%s)
src_compile() {
cd ${S}
cygautoreconf
cd ${B}
cygconf --enable-shared --disable-static \
--enable-qemu --enable-s3 --enable-threading \
--with-curl --with-expat --with-gnu-ld \
--disable-fuse --disable-python
# configure sets _FORTIFY_SOURCE=2 which would override level 3
# libtool requires '-no-undefined'
cygmake CFLAGS="${CFLAGS}" CXXFLAGS="${CXXFLAGS}" \
LDFLAGS="${LDFLAGS}${LDFLAGS:+ }-no-undefined"
}
src_install() {
cd ${B}
cyginstall
cd ${S}
dodoc doc/*.txt
cd ${D}
# affuse only prints that FUSE is not supported
rm -v usr/bin/affuse.exe usr/share/man/man1/affuse.1
gzip -9nv usr/share/doc/${PN}/*.txt
}
Re: [ITP] afflib 3.7.20-1
Jon Turney wrote: Thanks! libafflib_CONTENTS=" usr/bin/cygafflib-*.dll Any reason why this package doesn't include the soversion, i.e. why not libafflib0? Libtsk and libafflib are my first library packages, so I'm not sure what the policy is. My recent package libtsk has been accepted without soversion, so I omitted it also here. I assumed that the soversion will be added only when needed for new not backward compatible releases. The upstream afflib project is mostly idling, so I don't expect any new major lib versions in the near future. If course, I could rename it to libafflib0 if desired. rm -v usr/bin/affuse.exe usr/share/man/man1/affuse.1 # --disable-fuse I guess this comment means something to someone. But it doesn't tell me anything... :-) Long form: If (Linux-)FUSE is unavailable, the affuse tool only prints that this is the case and exits then. There is no need to distribute it.
[ITP] afflib 3.7.20-1
I would like to contribute afflib. Also present in Debian, Fedora,
Ubuntu, ... but package naming differs:
Debian/Ubuntu: afflib-tools, libafflib0v5, libafflib-dev
Fedora: afftools, afflib, afflib-devel
It is indented to enable afflib format support for the next version of
the sleuthkit package.
SUMMARY="Library and tools for the Advanced Forensic Format"
DESCRIPTION="\
The Advanced Forensic Format (AFF) is a file format for storing computer
forensic information. It supports metadata, compression, encryption and
signing."
afflib_tools_DESCRIPTION="...
This package contains tools to examine, convert, compare, copy, encrypt,
decrypt and sign AFF files."
afflib-tools-3.7.20-1.tar.xz:
usr/bin/affcat.exe
usr/bin/affcompare.exe
usr/bin/affconvert.exe
usr/bin/affcopy.exe
usr/bin/affcrypto.exe
usr/bin/affdiskprint.exe
usr/bin/affinfo.exe
usr/bin/affix.exe
usr/bin/affrecover.exe
usr/bin/affsegment.exe
usr/bin/affsign.exe
usr/bin/affstats.exe
usr/bin/affverify.exe
usr/bin/affxml.exe
usr/share/doc/afflib/*
usr/share/man/man1/aff*.1.gz
libafflib-3.7.20-1.tar.xz:
usr/bin/cygafflib-0.dll
libafflib-devel-3.7.20-1.tar.xz:
usr/include/afflib/*.h
usr/lib/libafflib.dll.a
usr/lib/pkgconfig/afflib.pc
--
Regards,
Christian
# cygport script for afflib
NAME=afflib
VERSION=3.7.20
RELEASE=1
SOURCE_DATE="2024-03-06 13:00:00 UTC"
SUMMARY="Library and tools for the Advanced Forensic Format"
DESCRIPTION="\
The Advanced Forensic Format (AFF) is a file format for storing computer
forensic information. It supports metadata, compression, encryption and
signing."
LICENSE="BSD-4-clause"
PKG_NAMES="afflib-tools libafflib libafflib-devel"
afflib_tools_SUMMARY="${SUMMARY} (tools)"
afflib_tools_DESCRIPTION="${DESCRIPTION}
This package contains tools to examine, convert, compare, copy, encrypt,
decrypt and sign AFF files."
libafflib_SUMMARY="${SUMMARY} (runtime)"
libafflib_DESCRIPTION="${DESCRIPTION}
This package contains the runtime library for afflib."
libafflib_devel_SUMMARY="${SUMMARY} (development)"
libafflib_devel_DESCRIPTION="${DESCRIPTION}
This package contains the development files for libafflib."
CATEGORY="Devel Libs Utils"
afflib_tools_CATEGORY="Utils"
libafflib_CATEGORY="Libs"
libafflib_devel_CATEGORY="Devel Libs"
afflib_tools_REQUIRES="" # libafflib libexpat1 libgcc1 libreadline7 libssl3
libstdc++6
libafflib_REQUIRES="" # libcurl4 libexpat1 libgcc1 libssl3 libstdc++6 zlib0
libafflib_devel_REQUIRES="" # libafflib libssl-devel pkg-config
BUILD_REQUIRES="
binutils gcc-g++ gzip libcurl-devel libexpat-devel libreadline-devel
libssl-devel zlib-devel
" # make
HOMEPAGE="https://github.com/sshock/AFFLIBv3;
SRC_URI="https://codeload.github.com/sshock/AFFLIBv3/tar.gz/refs/tags/v${PV}#/${P}.tar.gz;
SRC_DIR="AFFLIBv3-${PV}"
afflib_tools_CONTENTS="
--exclude=usr/bin/cygafflib-*.dll
usr/bin
usr/share
"
libafflib_CONTENTS="
usr/bin/cygafflib-*.dll
"
libafflib_devel_CONTENTS="
usr/include/afflib
usr/lib
"
DIFF_EXCLUDES="lzma443"
export SOURCE_DATE_EPOCH=$(date -d "$SOURCE_DATE" +%s)
src_compile() {
cd ${S}
cygautoreconf
cd ${B}
cygconf --enable-shared --disable-static \
--enable-s3 --enable-threading --enable-qemu \
--with-curl --with-gnu-ld --with-expat \
--disable-fuse --disable-python
# configure sets _FORTIFY_SOURCE=2 which would override level 3
# libtool requires '-no-undefined'
cygmake CFLAGS="${CFLAGS}" CXXFLAGS="${CXXFLAGS}" \
LDFLAGS="${LDFLAGS}${LDFLAGS:+ }-no-undefined"
}
src_install() {
cd ${B}
cyginstall
cd ${S}
dodoc doc/*.txt
cd ${D}
rm -v usr/bin/affuse.exe usr/share/man/man1/affuse.1 # --disable-fuse
gzip -9nv usr/share/doc/${PN}/*.txt
}
--- origsrc/AFFLIBv3-3.7.20/lib/Makefile.am 2024-03-06 13:00:00.0
+
+++ src/AFFLIBv3-3.7.20/lib/Makefile.am 2024-03-06 13:00:00.0 +
@@ -76,7 +76,7 @@ install-exec-hook:
then echo $(libdir) already installed ; \
else echo installing $(libdir) in $(DESTDIR)/etc/ld.so.conf ; \
echo $(libdir) >> $(DESTDIR)/etc/ld.so.conf ; \
- PATH=$(PATH):/sbin; \
+ PATH="$(PATH):/sbin"; \
ldconfig; \
fi ; \
echo "*" ;\
--- origsrc/AFFLIBv3-3.7.20/tools/affcopy.cpp 2024-03-06 13:00:00.0
+
+++ src/AFFLIBv3-3.7.20/tools/affcopy.cpp 2024-03-06 13:00:00.0
+
@@ -146,7 +146,7 @@ void unlink_outfiles(vector outf
if(failure) exit(1);
}
-#if !defined( __BSD_VISIBLE) && !defined(isnumber)
+#if (!defined(__BSD_VISIBLE) || defined(__CYGWIN__)) && !defined(isnumber)
#define isnumber(x) isdigit(x)
#endif
Re: [ITP] sleuthkit 4.12.1
Hi Marco,
Marco Atzeri via Cygwin-apps wrote:
On 02/03/2024 13:05, Christian Franke via Cygwin-apps wrote:
I would like to contribute sleuthkit. Also present in Debian, Fedora,
Ubuntu, ...
SUMMARY="Tools for analysis of volume and filesystem data"
DESCRIPTION="The Sleuth Kit (TSK) is a collection of command line tools
for disk images. It allows to analyze volume and filesystem data,
examine disk layout, recover deleted files, etc. Many partition and
filesystem formats are supported."
libtsk_SUMMARY="${SUMMARY} (runtime)"
libtsk_devel_SUMMARY="${SUMMARY} (development)"
I'm not sure about the LICENSE string:
LICENSE="CPL-1.0 AND GPL-2.0-or-later"
The license/README.md file mentions a bunch of licenses, see comment
in cygport file. CPL-1.0 is the main license, one separate tool uses
GPL-2.0-or-later.
The source package supports reproducible builds except for
libtsk-devel (timestamps in *.a files).
Hi Christian,
usually we do no distribute static library
Didn't know, sorry. Makes plenty of sense, at least to prevent that
other packages accidentally link to the static lib.
Any reason here ?
No, static lib removed.
except that GTG
$ git diff |grep "^+"
+++ b/cygwin-pkg-maint
+sleuthkit Christian Franke
Thanks,
Christian
[ITP] sleuthkit 4.12.1
I would like to contribute sleuthkit. Also present in Debian, Fedora,
Ubuntu, ...
SUMMARY="Tools for analysis of volume and filesystem data"
DESCRIPTION="The Sleuth Kit (TSK) is a collection of command line tools
for disk images. It allows to analyze volume and filesystem data,
examine disk layout, recover deleted files, etc. Many partition and
filesystem formats are supported."
libtsk_SUMMARY="${SUMMARY} (runtime)"
libtsk_devel_SUMMARY="${SUMMARY} (development)"
I'm not sure about the LICENSE string:
LICENSE="CPL-1.0 AND GPL-2.0-or-later"
The license/README.md file mentions a bunch of licenses, see comment in
cygport file. CPL-1.0 is the main license, one separate tool uses
GPL-2.0-or-later.
The source package supports reproducible builds except for libtsk-devel
(timestamps in *.a files).
Abbreviated list of files:
sleuthkit-4.12.1-1.tar.xz:
usr/bin/blkcalc.exe
usr/bin/blkcat.exe
usr/bin/blkls.exe
usr/bin/blkstat.exe
usr/bin/fcat.exe
usr/bin/ffind.exe
usr/bin/fiwalk.exe
usr/bin/fls.exe
usr/bin/fsstat.exe
usr/bin/hfind.exe
usr/bin/icat.exe
usr/bin/ifind.exe
usr/bin/ils.exe
usr/bin/img_cat.exe
usr/bin/img_stat.exe
usr/bin/istat.exe
usr/bin/jcat.exe
usr/bin/jls.exe
usr/bin/jpeg_extract.exe
usr/bin/mactime
usr/bin/mmcat.exe
usr/bin/mmls.exe
usr/bin/mmstat.exe
usr/bin/pstat.exe
usr/bin/sigfind.exe
usr/bin/sorter
usr/bin/srch_strings.exe
usr/bin/tsk_comparedir.exe
usr/bin/tsk_gettimes.exe
usr/bin/tsk_imageinfo.exe
usr/bin/tsk_loaddb.exe
usr/bin/tsk_recover.exe
usr/bin/usnjls.exe
usr/share/doc/sleuthkit/*
usr/share/man/man1/*
usr/share/tsk/sorter/*.sort
libtsk-4.12.1-1.tar.xz:
usr/bin/cygtsk-19.dll
libtsk-devel-4.12.1-1.tar.xz:
usr/include/tsk/*
usr/lib/libtsk.a
usr/lib/libtsk.dll.a
usr/lib/pkgconfig/tsk.pc
usr/share/doc/sleuthkit/samples/*.cpp
Real world use case: Check whether the SSD TRIM command actually works:
$ # Create a test file (> ~3*256B to prevent resident file)
$ printf 'Line %s\n' {0001..0100} > trim_check
$ # Get full path of file
$ cygpath -am trim_check
D:/tmp/trim_check
$ # Find raw device of partition
$ grep D: /proc/partitions # or: ls -l /dev/disk/by-drive/d
8 20 629145944 sdb4 D:\
$ # Find inode (here: $MFT index) of file
$ ifind -n /tmp/trim_check /dev/sdb4
339065
$ # Find cluster(s) used by inode
$ istat /dev/sdb4 339065
...
Name: TRIM_C~1
...
Name: trim_check
...
Type: $DATA (128-4) Name: N/A Non-Resident size: 1000 init_size: 1000
7876740
$ # Read cluster (assumes 4KiB cluster size, could be checked with fsstat)
$ dd if=/dev/sdb4 bs=4096 count=1 skip=7876740 iflag=direct status=none
| cat -A
Line 0001$
Line 0002$
...
$ # Remove file, flush buffers and wait
$ rm trim_check; sync; sleep 10
$ # Re-read cluster
$ dd if=/dev/sdb4 bs=4096 count=1 skip=7876740 iflag=direct status=none | od
000 00 00 00 00 00 00 00 00
*
001
$ echo "TRIM works!"
TRIM works!
--
Regards,
Christian
# cygport script for sleuthkit
NAME=sleuthkit
VERSION=4.12.1
RELEASE=1
SOURCE_DATE="2024-03-02 11:00:00 UTC"
SUMMARY="Tools for analysis of volume and filesystem data"
DESCRIPTION="The Sleuth Kit (TSK) is a collection of command line tools
for disk images. It allows to analyze volume and filesystem data,
examine disk layout, recover deleted files, etc. Many partition and
filesystem formats are supported."
LICENSE="CPL-1.0 AND GPL-2.0-or-later"
# Licenses mentioned in licenses/README.md:
# Apache-2.0 # case-uco/*, win32/rejistry++/* (code not used)
# BSD-3-Clause # samples/*, tsk/fs/lzvn.c
# CPL-1.0# The Sleuth Kit (TSK) license
# GPL-2.0-or-later # tools/srchtools/srch_strings.c
# GPL-3.0-or-later # m4/ax_pthread.m4 (... WITH Autoconf-exception-3.0)
# IPL-1.0# The Coroner's Toolkit (TCT) license
# ISC# tools/fiwalk/src/base64.*
# "public domain"# tools/fiwalk/*, tsk/base/sha1.c
# MIT# tsk/auto/guid.cpp
# RSA-MD # tsk/base/md5c.c
# "Unicode" # tsk/base/tsk_unicode.*
CATEGORY="Utils"
PKG_NAMES="sleuthkit libtsk libtsk-devel"
REQUIRES="" # libgcc1 libstdc++6 libtsk perl-DateTime-TimeZone perl_base
libtsk_REQUIRES="" # libgcc1 libsqlite3_0 libstdc++6 zlib0
libtsk_devel_REQUIRES="" # libtsk pkg-config
BUILD_REQUIRES="
binutils cygwin-devel gcc-g++ gzip libsqlite3-devel perl_base zlib-devel
" # make
HOMEPAGE="https://www.sleuthkit.org/sleuthkit/;
SRC_URI="https://github.com/sleuthkit/sleuthkit/releases/download/${P}/${P}.tar.gz;
libtsk_SUMMARY="${SUMMARY} (runtime)"
libtsk_DESCRIPTION="${DESCRIPTION}
This package contains the runtime library for sleuthkit."
libtsk_devel_SUMMARY="${SUMMARY} (development)"
libtsk_devel_DESCRIPTION="${DESCRIPTION}
This package contains the development files for libtsk."
sleuthkit_CONTENTS="
--exclude=usr/bin/cygtsk-*.dll
--exclude=usr/share/doc/${PN}/samples
usr/bin
usr/share
"
libtsk_CONTENTS="
Re: [PATCH cygport] Add repro-check command
Christian Franke wrote:
This could be used to check whether a package is possibly
reproducible. Then it could make sense to add a reasonable
SOURCE_DATE_EPOCH value to the cygport file.
Example:
$ export SOURCE_DATE_EPOCH=$(date +%s)
$ cygport project.cygport all repro-check
...
*** Info: Build reproducibility test succeeded
$ TZ=UTC cygport project.cygport repro-check
...
*** Info: Build reproducibility test succeeded
$ unset SOURCE_DATE_EPOCH
$ cygport project.cygport repro-check
...
*** ERROR: Build reproducibility test failed
An enhanced version of the patch is attached. The build and diff could
now be run also individually and the diff report includes individual
files from the packages.
As a side effect, this enables another use case: Check whether changes
to cygport only change the expected files.
$ cygport project.cygport all repro-check
...
*** Info: Rebuild produced identical packages
$ editor project.cygport
... Change some comments ...
$ cygport project.cygport all repro-diff
...
Differing files found:
! dist/project/project-1.2-3-src.tar.xz
! spkg/project-1.2-3.src/project.cygport
*** ERROR: Rebuild differs from original
--
Regards,
Christian
From 152a21dfad4c786cff1712e6aa1c33f2db0b6a75 Mon Sep 17 00:00:00 2001
From: Christian Franke
Date: Fri, 1 Mar 2024 19:46:50 +0100
Subject: [PATCH] Add repro-build, repro-diff und repro-check commands
These commands check for reproducibility of distribution packages.
The repro-build command unpacks the source package from the dist
directory to the temp directory and performs a nested rebuild of
the packages there.
The repro-diff command compares original and rebuild packages.
If different, a report about individual differing files in dist,
inst and spkg directories is printed.
The repro-check command combines both commands.
---
README | 3 +++
bin/cygport.in | 17 +
lib/help.cygpart| 3 +++
lib/pkg_pkg.cygpart | 58 -
4 files changed, 80 insertions(+), 1 deletion(-)
diff --git a/README b/README
index fd16df6b..3c9e4d4a 100644
--- a/README
+++ b/README
@@ -163,6 +163,9 @@ Other COMMANDs are meant primarily for maintainers:
diff - write a patch file capturing changes to source in the working
directory
stage- as upload, but don't request processing of uploaded packages
announce - compose and send a package announcement
+repro-build - rebuild from created source package to temp directory
+repro-diff - check whether packages from original and rebuild differ
+repro-check - run repro-build and repro-diff
The standard arguments --help or --version may also be passed to cygport.
diff --git a/bin/cygport.in b/bin/cygport.in
index 5fc89eaf..a2c2b5a3 100755
--- a/bin/cygport.in
+++ b/bin/cygport.in
@@ -29,6 +29,10 @@ set -e;
#
+# Preserve original environment for repro-check command
+declare -r _cygport_orig_env=$(export)
+declare -r _cygport_orig_pwd=$(pwd)
+
# for regexes, sort, etc.
export LC_COLLATE=C
@@ -784,6 +788,19 @@ do
test ${PIPESTATUS[0]} -eq 0
_status=$?;
;;
+ repro-build)
+ __pkg_repro_build
+ _status=$?
+ ;;
+ repro-diff)
+ __pkg_repro_diff
+ _status=$?
+ ;;
+ repro-check)
+ __pkg_repro_build && \
+ __pkg_repro_diff
+ _status=$?
+ ;;
help)
__show_help;
exit 0;
diff --git a/lib/help.cygpart b/lib/help.cygpart
index a7f30f7a..d28fd7bb 100644
--- a/lib/help.cygpart
+++ b/lib/help.cygpart
@@ -56,6 +56,9 @@ __show_help() {
finishdelete the working directory
all run prep, compile, install and package
all-test run prep, compile, install and package-test
+ repro-build rebuild from created source package to temp
directory
+ repro-diffcheck whether packages from original and
rebuild differ
+ repro-check run repro-build and repro-diff
See the included README file for further documentation.
diff --git a/lib/pkg_pkg.cygpart b/lib/pkg_pkg.cygpart
index 756a687c..3c531c0e 100644
--- a/lib/pkg_pkg.cygpart
+++ b/lib/pkg_pkg.cygpart
@@ -992,6 +992,62 @@ _EOF
fi
}
+__pkg_repro_build() {
+ local srcpkg=${distdir}/${PN}/${PF}-src.tar.${TAR_COMPRESSION_EXT}
+ local t_spkgdir=${T}/${spkgdir##*/}
+ local t_workdir=${t_spkgdir}/${PF}.${ARCH}
+ local t_cygport="cygport ${cygportfile} finish all"
+ local rc
+
+ __stage "Reb
[PATCH cygport] dodoc: Skip a file if a compressed version already exists
It IMO makes sense to compress large and rarely viewed doc files like
change logs. This seems to be common practice on Debian etc.
With current cygport, the following results in ChangeLog and
ChangeLog.gz in the docdir:
src_install()
{
...
dodoc ChangeLog
gzip -9 -n "${D}/usr/share/doc/${PN}/ChangeLog"
}
The attached patch fixes this and also adds some missing documentation.
--
Regards,
Christian
From 1934651b93cda92207429ac91b964cff220c76d5 Mon Sep 17 00:00:00 2001
From: Christian Franke
Date: Fri, 1 Mar 2024 13:56:45 +0100
Subject: [PATCH] dodoc: Skip a file if a compressed version already exists
This prevents that __prepdoc() also adds the uncompressed version
of a default doc file if src_install() installed the compressed
version. Also add missing documentation about the handling of
FILE.md, FILE.rst and FILE.txt.
---
lib/src_install.cygpart | 9 -
1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/lib/src_install.cygpart b/lib/src_install.cygpart
index 481457dc..7aca673c 100644
--- a/lib/src_install.cygpart
+++ b/lib/src_install.cygpart
@@ -162,9 +162,12 @@ docinto() {
# DESCRIPTION
# Installs the given files or directories into $D/usr/share/doc/PN/, or a
# subdirectory thereof specified by the previous call to docinto.
+# If a FILE does not exist, FILE.md, FILE.rst and FILE.txt are also
+# considered. A FILE is skipped if the destination file or a compressed
+# version (.bz2, .gz, .xz, .zstd) of it already exists.
#
dodoc() {
- local docdir d f i x
+ local docdir d e f i x
case "${_docinto_dir}" in
'') docdir=/usr/share/doc/${PN} ;;
@@ -191,6 +194,10 @@ dodoc() {
do
if [ -s "${i}${x}" -a ! -f
"${D}${docdir}/${i}${x}" ]
then
+ for e in bz2 gz xz zst
+ do
+ ! [ -f
"${D}${docdir}/${i}${x}.${e}" ] || continue 2
+ done
__doinstall 0644 "${i}${x}" ${docdir}
|| error "dodoc ${i} failed"
break
fi
--
2.43.0
Re: [PATCH cygport] Set TZ=UTC if SOURCE_DATE_EPOCH is used
Christian Franke wrote: Further tests of 'repro-check' patch revealed that the "origsrc/*" timestamps in patch files contain the local timezone offset. This would be no longer needed for patch files if "Modify origsrc timestamp in patch files if SOURCE_DATE_EPOCH is used" would be applied. This TZ=UTC patch could possibly be dropped then. The attached patch is on top of the "Add more checks of SOURCE_DATE_EPOCH" patch. It could also be applied independently but then requires conflict resolution due to the then missing 'else' branch.
[PATCH cygport] Modify origsrc timestamp in patch files if SOURCE_DATE_EPOCH is used
Found during testing of 'repro-check' patch with getent-2.18.90-5 source
package.
This patch also removes the requirement to set TZ=UTC before patches are
generated.
--
Regards,
Christian
From 342ff5113499a83b2ffda441ddc80b4952b400f8 Mon Sep 17 00:00:00 2001
From: Christian Franke
Date: Wed, 28 Feb 2024 16:46:51 +0100
Subject: [PATCH] Modify origsrc timestamp in patch files if SOURCE_DATE_EPOCH
is used
Also the timestamp of a file in origsrc directory may be newer than
SOURCE_DATE_EPOCH if modified after unpacking e.g. by a src_*_hook.
---
lib/pkg_pkg.cygpart | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/lib/pkg_pkg.cygpart b/lib/pkg_pkg.cygpart
index 756a687c..414ec4b4 100644
--- a/lib/pkg_pkg.cygpart
+++ b/lib/pkg_pkg.cygpart
@@ -508,9 +508,9 @@ __pkg_diff() {
if [ -n "${SOURCE_DATE_EPOCH}" ]
then
- # Ensure that the timestamp comment in the generated patch file
is reproducible
+ # Ensure that the timestamp comments in the patch files are
reproducible
source_date=$(date -d @"${SOURCE_DATE_EPOCH}" -u +'%Y-%m-%d
%H:%M:%S.0 +')
- sed -b -e "s|^\(+++ [^\t]*\t\).*\$|\1${source_date}|" \
+ sed -E -b -e "s/^((---|\+\+\+) [^\t]*\t).*\$/\1${source_date}/"
\
-i ${optional_patchfiles} ${patchdir}/${src_patchfile}
fi
--
2.43.0
Re: [PATCH cygport] Add more checks of SOURCE_DATE_EPOCH
Jon Turney wrote: On 16/02/2024 12:29, Christian Franke via Cygwin-apps wrote: Fail if it is out of range. Warn if it lies in the future. Inform whether it is set or set but not exported. What is the valid range here? The range accepted by 'date -d @EPOCH ...', later used to adjust the patch timestamps. The test could also be removed as date interestingly accepts a (too) wide range of values :-) $ date -d @$((1<<55)) Sun Jun 13 08:26:08 CEST 1141709097 $ date -d @$((1<<56)) date: time ‘72057594037927936’ is out of range $ date -d @$((-(1<<55))) Sun Jul 20 18:27:20 LMT -1141705158 $ date -d @$((-(1<<56))) date: time ‘-72057594037927936’ is out of range Would it not make more sense to just re-export it if set? If the cygport file decides to set but not export it, there is possibly no need to do it. An example is smartmontools.cygport which passes the unexported variable as a parameter to configure. (so that commands like "SOURCE_DATE_EPOCH=something cygport foo" work as expected?) Would make no difference as the 'VAR=val CMD...' syntax already exports the variable to the CMD: $ unset FOO; FOO=bar sh -c 'sh -c "sh -c printenv\ FOO"' bar
[PATCH cygport] Fix variable expansion in error message of embedded SMTP perl script
Harmless bug ...
--
Regards,
Christian
From b1074f4cfe549c852be7fa59d85d312c9579cf0d Mon Sep 17 00:00:00 2001
From: Christian Franke
Date: Fri, 23 Feb 2024 13:04:21 +0100
Subject: [PATCH] Fix variable expansion in error message of embedded SMTP perl
script
---
lib/pkg_upload.cygpart | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/lib/pkg_upload.cygpart b/lib/pkg_upload.cygpart
index dcae8e2c..cdeae283 100644
--- a/lib/pkg_upload.cygpart
+++ b/lib/pkg_upload.cygpart
@@ -222,7 +222,7 @@ if (\$smtp_encryption eq 'tls') {
require Net::SMTP::SSL;
\$smtp->command('STARTTLS');
\$smtp->response();
- \$smtp->code == 220 or die "$server does not support STARTTLS";
+ \$smtp->code == 220 or die "\$smtp_server does not support STARTTLS";
\$smtp = Net::SMTP::SSL->start_SSL(\$smtp) or die "STARTTLS failed";
\$smtp->hello(\$smtp_server);
}
--
2.43.0
Re: [PATCH cygport] Add customization support for announce command
Christian Franke wrote:
The email generated by the cygport announce command is useful, but
actual use cases are somewhat limited due to the hard-coded email
submission.
The attached patch adds more flexibility. The patch is on top of the
"Use correct wording if only one package is announced" patch.
Slightly changed patch attached. Also adjusted to new version of "Use
correct wording if only one package is announced" patch.
Examples for cygport.conf settings:
ANNOUNCE_EDITOR='printf "\\nRegards,\\n$PN package maintainer\\n" >>"$1"'
ANNOUNCE_EDITOR='
n=$(wc -l <"$1") && cat >>"$1" <>> This is an update to the latest upstream release.
>>> This is a bugfix release.
<<< PLEASE EDIT >>>
Regards,
$PN package maintainer
EOF
vim +$((n+2)) "$1" && ! grep -E "<<<|>>>" "$1"
'
ANNOUNCE_TO_CMD='cat "$1" >/dev/clipboard'
ANNOUNCE_TO_CMD='
sed "1,/^\$/d" "$1" >$PF-announcement.txt &&
echo "Announcement placed here: $(pwd)/$PF-announcement.txt"
'
ANNOUNCE_TO_CMD='
/usr/local/sbin/custom-mailer \
--sender="$SMTP_SENDER" \
--smarthost="$SMTP_SERVER" \
...more...options... \
cygwin-annou...@cygwin.com <"$1"
'
Possible (better?) alternative names for the new settings:
ANNOUNCEMENT_EDITOR
ANNOUNCEMENT_MAILER
From 14709f0a1ed19c7d00588fb2a1fa7273d47e00fd Mon Sep 17 00:00:00 2001
From: Christian Franke
Date: Fri, 23 Feb 2024 12:04:17 +0100
Subject: [PATCH] Add customization support for announce command
Two new configuration settings allow to override the launch of
a text editor (ANNOUNCE_EDITOR) and the builtin email submission
(ANNOUNCE_TO_CMD).
---
data/cygport.conf | 23 +
lib/pkg_upload.cygpart | 56 +-
2 files changed, 73 insertions(+), 6 deletions(-)
diff --git a/data/cygport.conf b/data/cygport.conf
index 34ccd291..48dc7bfe 100644
--- a/data/cygport.conf
+++ b/data/cygport.conf
@@ -101,6 +101,29 @@
#PAGER=
+#v* Configuration/ANNOUNCE_EDITOR
+# DESCRIPTION
+# Shell command string to process the email message created by cygport's
+# announce command before sending the email. If undefined, a text editor
+# will be run, see EDITOR setting above. If empty, nothing will be run.
+# If not empty, '/bin/bash' will be launched with the command string passed
+# with '-c' option and the path of the temporary email message file as '$1'.
+# The working directory of the shell will be the directory of the cygport
+# file. The specified command string will be prepended by shell assignments
+# of the cygport variables P, PF, PN, PR and PV and all SMTP_* settings
+# described below.
+#ANNOUNCE_EDITOR=
+
+#v* Configuration/ANNOUNCE_TO_CMD
+# DESCRIPTION
+# Shell command string to process the email message created by cygport's
+# announce command after editing. If undefined, the email will be sent
+# using the builtin perl-based SMTP support. If empty, nothing will be run.
+# If not empty, the command string will be handled similar to ANNOUNCE_EDITOR
+# described above.
+#ANNOUNCE_TO_CMD=
+
+
#v* Configuration/SMTP_SENDER
# DESCRIPTION
# Name and email address, in the form of "First Last " to be used
diff --git a/lib/pkg_upload.cygpart b/lib/pkg_upload.cygpart
index 37bc2d63..d38ea8b6 100644
--- a/lib/pkg_upload.cygpart
+++ b/lib/pkg_upload.cygpart
@@ -168,6 +168,33 @@ EOF
echo "Upload complete."
}
+__pkg_announce_run_cmd_on_msg() {
+ local cmdvar=$1
+ local msg=$2
+ local cmd
+
+ eval cmd="\${${cmdvar}}"
+
+ if [ "${cmd:+y}" != "y" ]
+ then
+ inform "\${${cmdvar}} is empty"
+ return 0
+ fi
+ echo
+ inform "Launching '\${${cmdvar}} ${msg}'"
+
+ /bin/bash -c "cd ${top} || exit 1
+P=${P@Q}; PF=${PF@Q}; PN=${PN@Q}; PR=${PR@Q}; PV=(${PV[*]@Q})
+${SMTP_SENDER+SMTP_SENDER=${SMTP_SENDER@Q}}
+${SMTP_SERVER+SMTP_SERVER=${SMTP_SERVER@Q}}
+${SMTP_SERVER_PORT+SMTP_SERVER_PORT=${SMTP_SERVER_PORT@Q}}
+${SMTP_ENCRYPTION+SMTP_ENCRYPTION=${SMTP_ENCRYPTION@Q}}
+${SMTP_USER+SMTP_USER=${SMTP_USER@Q}}
+${SMTP_PASS+SMTP_PASS=${SMTP_PASS@Q}}
+${cmd}
+" $0 ${msg} || error "Command '\${${cmdvar}} ${msg}' (cwd=${top})
failed"
+}
+
__pkg_announce() {
local msg=$(mktemp -t cygwin-announce-${PF}.XX)
local msgat=$(date +@%s)
@@ -178,10 +205,10 @@ __pkg_announce() {
cat > ${msg} <<_EOF
From cygwin-announce-${PF} $(date '+%a %b %d %H:%M:%S %Y' --date=${msgat})
-From: ${SMTP_SENDER}
-To: cygwin-annou...@cygwin.com
+${SMTP_SENDER:+From: ${SMTP_SENDER}
+}To: cygwin-annou...@cygwin.com
Date: $(date -R --date=${msgat})
-Message-Id: <$
Re: [PATCH cygport] Use correct wording if only one package is announced
Brian Inglis via Cygwin-apps wrote:
On 2024-02-21 07:25, Christian Franke via Cygwin-apps wrote:
Change variable name from $s to $has or $s_have as variable $s usually
implies only the plural letter s or nothing; e.g.
...
+ local has="s have"
+
+ [ $pkg_count != 1 ] || has=" has"
...
+The following package${has} been uploaded to the Cygwin distribution:
...
Agree - new patch attached.
From 6da0f806a94621d0ecfcca3c63e5c46e8ab3cd32 Mon Sep 17 00:00:00 2001
From: Christian Franke
Date: Fri, 23 Feb 2024 11:35:01 +0100
Subject: [PATCH] Use correct wording if only one package is announced
---
lib/pkg_upload.cygpart | 5 -
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/lib/pkg_upload.cygpart b/lib/pkg_upload.cygpart
index dcae8e2c..37bc2d63 100644
--- a/lib/pkg_upload.cygpart
+++ b/lib/pkg_upload.cygpart
@@ -172,6 +172,9 @@ __pkg_announce() {
local msg=$(mktemp -t cygwin-announce-${PF}.XX)
local msgat=$(date +@%s)
local -i n=0
+ local s_have="s have"
+
+ [ $pkg_count != 1 ] || s_have=" has"
cat > ${msg} <<_EOF
From cygwin-announce-${PF} $(date '+%a %b %d %H:%M:%S %Y' --date=${msgat})
@@ -181,7 +184,7 @@ Date: $(date -R --date=${msgat})
Message-Id: <$(date "+%Y%m%d%H%M%S.$$" --date=${msgat})-1-$(echo
${SMTP_SENDER} | sed 's|.*<\(.*\)>.*|\1|')>
Subject: ${NAME} ${PVR}
-The following packages have been uploaded to the Cygwin distribution:
+The following package${s_have} been uploaded to the Cygwin distribution:
_EOF
--
2.43.0
[PATCH cygport] Add customization support for announce command
The email generated by the cygport announce command is useful, but
actual use cases are somewhat limited due to the hard-coded email
submission.
The attached patch adds more flexibility. The patch is on top of the
"Use correct wording if only one package is announced" patch.
Examples for cygport.conf settings:
ANNOUNCE_EDITOR='printf "\\nRegards,\\n$PN package maintainer\\n" >>"$1"'
ANNOUNCE_EDITOR='
n=$(wc -l <"$1") && cat >>"$1" <>> This is an update to the latest upstream release.
>>> This is a bugfix release.
<<< PLEASE EDIT >>>
Regards,
$PN package maintainer
EOF
vim +$((n+2)) "$1" && ! grep -E "<<<|>>>" "$1"
'
ANNOUNCE_TO_CMD='cat "$1" >/dev/clipboard'
ANNOUNCE_TO_CMD='
sed "1,/^\$/d" "$1" >$PF-announcement.txt &&
echo "Announcement placed here: $(pwd)/$PF-announcement.txt"
'
ANNOUNCE_TO_CMD='
/usr/local/sbin/custom-mailer \
--sender="$SMTP_SENDER" \
--smarthost="$SMTP_SERVER" \
...more...options... \
cygwin-annou...@cygwin.com <"$1"
'
--
Regards,
Christian
From 1f13d54a40d639938fb67245eed4615be0a6e6c4 Mon Sep 17 00:00:00 2001
From: Christian Franke
Date: Wed, 21 Feb 2024 15:14:53 +0100
Subject: [PATCH] Add customization support for announce command
Two new configuration settings allow to override the launch of
a text editor (ANNOUNCE_EDITOR) and the builtin email submission
(ANNOUNCE_TO_CMD).
---
data/cygport.conf | 23 +
lib/pkg_upload.cygpart | 57 +-
2 files changed, 74 insertions(+), 6 deletions(-)
diff --git a/data/cygport.conf b/data/cygport.conf
index 34ccd291..48dc7bfe 100644
--- a/data/cygport.conf
+++ b/data/cygport.conf
@@ -101,6 +101,29 @@
#PAGER=
+#v* Configuration/ANNOUNCE_EDITOR
+# DESCRIPTION
+# Shell command string to process the email message created by cygport's
+# announce command before sending the email. If undefined, a text editor
+# will be run, see EDITOR setting above. If empty, nothing will be run.
+# If not empty, '/bin/bash' will be launched with the command string passed
+# with '-c' option and the path of the temporary email message file as '$1'.
+# The working directory of the shell will be the directory of the cygport
+# file. The specified command string will be prepended by shell assignments
+# of the cygport variables P, PF, PN, PR and PV and all SMTP_* settings
+# described below.
+#ANNOUNCE_EDITOR=
+
+#v* Configuration/ANNOUNCE_TO_CMD
+# DESCRIPTION
+# Shell command string to process the email message created by cygport's
+# announce command after editing. If undefined, the email will be sent
+# using the builtin perl-based SMTP support. If empty, nothing will be run.
+# If not empty, the command string will be handled similar to ANNOUNCE_EDITOR
+# described above.
+#ANNOUNCE_TO_CMD=
+
+
#v* Configuration/SMTP_SENDER
# DESCRIPTION
# Name and email address, in the form of "First Last " to be used
diff --git a/lib/pkg_upload.cygpart b/lib/pkg_upload.cygpart
index 8039ec5c..b81bf3d5 100644
--- a/lib/pkg_upload.cygpart
+++ b/lib/pkg_upload.cygpart
@@ -168,6 +168,28 @@ EOF
echo "Upload complete."
}
+__pkg_announce_run_cmd_on_msg() {
+ local cmdvar=$1
+ local msg=$2
+ local cmd
+
+ eval cmd="\${${cmdvar}}"
+
+ (
+ cd ${top} && /bin/bash -c "\
+P=${P@Q}; PF=${PF@Q}; PN=${PN@Q}; PR=${PR@Q}; PV=(${PV[*]@Q})
+${SMTP_SENDER+SMTP_SENDER=${SMTP_SENDER@Q}}
+${SMTP_SERVER+SMTP_SERVER=${SMTP_SERVER@Q}}
+${SMTP_SERVER_PORT+SMTP_SERVER_PORT=${SMTP_SERVER_PORT@Q}}
+${SMTP_ENCRYPTION+SMTP_ENCRYPTION=${SMTP_ENCRYPTION@Q}}
+${SMTP_USER+SMTP_USER=${SMTP_USER@Q}}
+${SMTP_PASS+SMTP_PASS=${SMTP_PASS@Q}}
+${cmd}
+" \
+ $0 ${msg}
+ ) || error "Command '\${${cmdvar}} ${msg}' (cwd=${top}) failed"
+}
+
__pkg_announce() {
local msg=$(mktemp -t cygwin-announce-${PF}.XX)
local msgat=$(date +@%s)
@@ -178,10 +200,10 @@ __pkg_announce() {
cat > ${msg} <<_EOF
From cygwin-announce-${PF} $(date '+%a %b %d %H:%M:%S %Y' --date=${msgat})
-From: ${SMTP_SENDER}
-To: cygwin-annou...@cygwin.com
+${SMTP_SENDER:+From: ${SMTP_SENDER}
+}To: cygwin-annou...@cygwin.com
Date: $(date -R --date=${msgat})
-Message-Id: <$(date "+%Y%m%d%H%M%S.$$" --date=${msgat})-1-$(echo
${SMTP_SENDER} | sed 's|.*<\(.*\)>.*|\1|')>
+Message-Id: <$(date "+%Y%m%d%H%M%S.$$" --date=${msgat})-1-$(echo
${SMTP_SENDER:-cygport} | sed 's|.*<\(.*\)>.*|\1|')>
Subject: ${NAME} ${PVR}
The following package${s} been uploaded to the Cygwin distribution:
@@ -199,7 +221,30 @@ _EOF
${DESCRIPTION}
_EOF
- ${EDITOR:-vi} $msg || error
[PATCH cygport] Use correct wording if only one package is announced
Only cosmetic ...
--
Regards,
Christian
From f1381ebc872f3b099c257677e2b8d5bf2451bb23 Mon Sep 17 00:00:00 2001
From: Christian Franke
Date: Wed, 21 Feb 2024 13:35:14 +0100
Subject: [PATCH] Use correct wording if only one package is announced
---
lib/pkg_upload.cygpart | 5 -
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/lib/pkg_upload.cygpart b/lib/pkg_upload.cygpart
index dcae8e2c..8039ec5c 100644
--- a/lib/pkg_upload.cygpart
+++ b/lib/pkg_upload.cygpart
@@ -172,6 +172,9 @@ __pkg_announce() {
local msg=$(mktemp -t cygwin-announce-${PF}.XX)
local msgat=$(date +@%s)
local -i n=0
+ local s="s have"
+
+ [ $pkg_count != 1 ] || s=" has"
cat > ${msg} <<_EOF
From cygwin-announce-${PF} $(date '+%a %b %d %H:%M:%S %Y' --date=${msgat})
@@ -181,7 +184,7 @@ Date: $(date -R --date=${msgat})
Message-Id: <$(date "+%Y%m%d%H%M%S.$$" --date=${msgat})-1-$(echo
${SMTP_SENDER} | sed 's|.*<\(.*\)>.*|\1|')>
Subject: ${NAME} ${PVR}
-The following packages have been uploaded to the Cygwin distribution:
+The following package${s} been uploaded to the Cygwin distribution:
_EOF
--
2.43.0
[PATCH cygport] Set TZ=UTC if SOURCE_DATE_EPOCH is used
Further tests of 'repro-check' patch revealed that the "origsrc/*"
timestamps in patch files contain the local timezone offset.
The attached patch is on top of the "Add more checks of
SOURCE_DATE_EPOCH" patch. It could also be applied independently but
then requires conflict resolution due to the then missing 'else' branch.
--
Regards,
Christian
From e9553d8541f9fb2fd4a4e49a488dfc8511fb9c8e Mon Sep 17 00:00:00 2001
From: Christian Franke
Date: Tue, 20 Feb 2024 10:32:52 +0100
Subject: [PATCH] Set TZ=UTC if SOURCE_DATE_EPOCH is used
---
bin/cygport.in | 4
1 file changed, 4 insertions(+)
diff --git a/bin/cygport.in b/bin/cygport.in
index 3fe8a52e..2261a5bd 100755
--- a/bin/cygport.in
+++ b/bin/cygport.in
@@ -517,6 +517,10 @@ then
4.6.[6-9]|4.[7-9]*|[5-9]*) ;;
*) error "SOURCE_DATE_EPOCH requires peflags 4.6.6 or later"
esac
+
+ # Ensure that date and time formatting (e.g. in patch files) is
reproducible
+ [ "${TZ}" = "UTC" ] || inform "Using TZ='UTC' instead of
TZ='${TZ-(unset)}'"
+ export TZ=UTC
else
inform "SOURCE_DATE_EPOCH is not set"
fi
--
2.43.0
[PATCH cygport] Add repro-check command
This could be used to check whether a package is possibly reproducible.
Then it could make sense to add a reasonable SOURCE_DATE_EPOCH value to
the cygport file.
Example:
$ export SOURCE_DATE_EPOCH=$(date +%s)
$ cygport project.cygport all repro-check
...
*** Info: Build reproducibility test succeeded
$ TZ=UTC cygport project.cygport repro-check
...
*** Info: Build reproducibility test succeeded
$ unset SOURCE_DATE_EPOCH
$ cygport project.cygport repro-check
...
*** ERROR: Build reproducibility test failed
--
Regards,
Christian
From 97f518478dac722647b8a423068f2a5461c82f19 Mon Sep 17 00:00:00 2001
From: Christian Franke
Date: Sun, 18 Feb 2024 16:33:07 +0100
Subject: [PATCH] Add repro-check command
This command checks for reproducibility of distribution packages.
The source package from the dist directory is unpacked to the
temp directory. A nested rebuild of the packages is run there.
If successful, original and rebuild packages are compared and the
result is reported.
---
README | 1 +
bin/cygport.in | 8
lib/help.cygpart| 1 +
lib/pkg_pkg.cygpart | 42 +-
4 files changed, 51 insertions(+), 1 deletion(-)
diff --git a/README b/README
index fd16df6b..fec46b13 100644
--- a/README
+++ b/README
@@ -163,6 +163,7 @@ Other COMMANDs are meant primarily for maintainers:
diff - write a patch file capturing changes to source in the working
directory
stage- as upload, but don't request processing of uploaded packages
announce - compose and send a package announcement
+repro-check - check whether a rebuild produces binary identical packages
The standard arguments --help or --version may also be passed to cygport.
diff --git a/bin/cygport.in b/bin/cygport.in
index 5fc89eaf..6acbc85b 100755
--- a/bin/cygport.in
+++ b/bin/cygport.in
@@ -29,6 +29,10 @@ set -e;
#
+# Preserve original environment for repro-check command
+declare -r _cygport_orig_env=$(export)
+declare -r _cygport_orig_pwd=$(pwd)
+
# for regexes, sort, etc.
export LC_COLLATE=C
@@ -784,6 +788,10 @@ do
test ${PIPESTATUS[0]} -eq 0
_status=$?;
;;
+ repro-check)
+ __pkg_repro_check
+ _status=$?
+ ;;
help)
__show_help;
exit 0;
diff --git a/lib/help.cygpart b/lib/help.cygpart
index a7f30f7a..d851762e 100644
--- a/lib/help.cygpart
+++ b/lib/help.cygpart
@@ -56,6 +56,7 @@ __show_help() {
finishdelete the working directory
all run prep, compile, install and package
all-test run prep, compile, install and package-test
+ repro-check check whether a rebuild produces binary
identical packages
See the included README file for further documentation.
diff --git a/lib/pkg_pkg.cygpart b/lib/pkg_pkg.cygpart
index 756a687c..719ffcd1 100644
--- a/lib/pkg_pkg.cygpart
+++ b/lib/pkg_pkg.cygpart
@@ -992,6 +992,46 @@ _EOF
fi
}
+__pkg_repro_check() {
+ local rc srcpkg t_cygport t_spkgdir
+
+ srcpkg=${distdir}/${PN}/${PF}-src.tar.${TAR_COMPRESSION_EXT}
+ t_spkgdir=${T}/${spkgdir##*/}
+
+ echo
+ __stage "Checking reproducibility of"
+
+ echo
+ __step "Unpacking ${srcpkg}"
+ [ -f "${srcpkg}" ] || error "Packages not built yet"
+ tar xf ${srcpkg} -C ${T} || error "tar xf ${srcpkg} -C ${T} failed"
+
+ echo
+ __step "Rebuilding in ${t_spkgdir}"
+ t_cygport="cygport ${cygportfile} finish all"
+ echo "${_cygport_orig_env}" > ${T}/.cygport_orig_env
+ __step "=== Start: ${t_cygport} ="
+
+ # Start nested cygport with original environment in temp directory
+ rc=0
+ env --chdir=${_cygport_orig_pwd} --ignore-environment /bin/bash -c \
+ "source ${T}/.cygport_orig_env && cd ${t_spkgdir} &&
${t_cygport}" \
+ || rc=$?
+
+ __step "=== Done: ${t_cygport} (exit $rc) ="
+ echo
+ [ $rc = 0 ] || error "Rebuild in ${t_spkgdir} failed"
+
+ __step "Comparing original and rebuilt packages"
+ if ! diff -qr ${distdir} ${t_spkgdir}/${PF}.${ARCH}/dist
+ then
+ echo
+ error "Build reproducibility test failed"
+ fi
+ echo
+ inform "Build reproducibility test succeeded"
+}
+
# protect functions
readonly -f __pkg_binpkg __pkg_diff __gpg_sign __pkg_srcpkg __pkg_dist \
-__squeeze_whitespace __tar
+__pkg_repro_check __squeeze_whitespace __tar
--
2.43.0
[ITP] f3 8.0
I would like to contribute f3. Also present in Debian, Fedora, Ubuntu, ...
SUMMARY="Test real flash memory capacity"
DESCRIPTION="f3 is a simple tool that tests flash cards capacity and
performance to see if they live up to claimed specifications.
It fills the device with pseudorandom data and then checks if it
returns the same on reading.
F3 stands for Fight Flash Fraud, or Fight Fake Flash."
List of files:
f3-8.0-1.tar.xz:
usr/bin/f3read.exe
usr/bin/f3write.exe
usr/share/doc/f3/LICENSE
usr/share/doc/f3/README.rst
usr/share/doc/f3/changelog.gz
usr/share/doc/f3/contribute.rst.gz
usr/share/doc/f3/examples/f3write.h2w
usr/share/doc/f3/examples/log-f3wr
usr/share/doc/f3/history.rst.gz
usr/share/doc/f3/usage.rst.gz
usr/share/man/man1/f3read.1.gz
usr/share/man/man1/f3write.1.gz
f3-debuginfo-8.0-1.tar.xz:
usr/lib/debug/usr/bin/f3read.exe.dbg
usr/lib/debug/usr/bin/f3write.exe.dbg
usr/src/debug/f3-8.0-1/f3read.c
usr/src/debug/f3-8.0-1/f3write.c
usr/src/debug/f3-8.0-1/libflow.c
usr/src/debug/f3-8.0-1/libflow.h
usr/src/debug/f3-8.0-1/utils.c
usr/src/debug/f3-8.0-1/utils.h
f3-8.0-1.src.tar.xz:
f3-8.0-1.src/f3-8.0.tar.gz
f3-8.0-1.src/f3.cygport
The source package supports reproducible builds.
--
Regards,
Christian
# cygport script for f3
NAME=f3
VERSION=8.0
RELEASE=1
SOURCE_DATE="2024-02-17 13:00:00 UTC"
SUMMARY="Test real flash memory capacity"
DESCRIPTION="f3 is a simple tool that tests flash cards capacity and
performance to see if they live up to claimed specifications.
It fills the device with pseudorandom data and then checks if it
returns the same on reading.
F3 stands for Fight Flash Fraud, or Fight Fake Flash."
LICENSE="GPL-3.0-only" # or GPL-3.0-or-later ?
CATEGORY="Utils"
REQUIRES="" # libargp
BUILD_REQUIRES="binutils gcc-core gzip libargp-devel" # make
HOMEPAGE="https://fight-flash-fraud.readthedocs.io/;
SRC_URI="https://codeload.github.com/AltraMayor/f3/tar.gz/refs/tags/v${PV}#/${P}.tar.gz;
export SOURCE_DATE_EPOCH=$(date -d "$SOURCE_DATE" +%s)
src_compile() {
cd ${B}
lndirs
cygmake PREFIX=/usr \
LDFLAGS="${LDFLAGS}${LDFLAGS:+ }-Wl,--stack,400 -largp"
}
src_install() {
cd ${B}
cyginstall PREFIX=/usr
cd ${S}
dodoc changelog doc/{contribute,history,usage}.rst
gzip -9 -n ${D}/usr/share/doc/${PN}/{changelog,*.rst}
docinto examples
dodoc f3write.h2w log-f3wr
}
[PATCH cygport] Add more checks of SOURCE_DATE_EPOCH
From b04c8f5e9becd6e91095e2add551f72870c9e869 Mon Sep 17 00:00:00 2001
From: Christian Franke
Date: Fri, 16 Feb 2024 13:16:06 +0100
Subject: [PATCH] Add more checks of SOURCE_DATE_EPOCH
Fail if it is out of range. Warn if it lies in the future.
Inform whether it is set or set but not exported.
---
bin/cygport.in | 20 +++-
1 file changed, 19 insertions(+), 1 deletion(-)
diff --git a/bin/cygport.in b/bin/cygport.in
index 5fc89eaf..3fe8a52e 100755
--- a/bin/cygport.in
+++ b/bin/cygport.in
@@ -493,14 +493,32 @@ unset restrict
if [ "${SOURCE_DATE_EPOCH+y}" = "y" ]
then
- if [ -n "$(echo "${SOURCE_DATE_EPOCH}" | sed -e 's/^$/X/' -e
's/[0-9]//g')" ]
+ if ! [[ "${SOURCE_DATE_EPOCH}" =~ ^[0-9]+$ ]]
then
error "SOURCE_DATE_EPOCH must be an integer number (seconds
since the unix epoch)"
fi
+ if ! _d=$(date --iso-8601=seconds -d "@${SOURCE_DATE_EPOCH}"
2>/dev/null)
+ then
+ error "SOURCE_DATE_EPOCH is out of range"
+ fi
+ if [ "${SOURCE_DATE_EPOCH}" -le "$(printf '%(%s)T')" ]
+ then
+ inform "Reproducible build:
SOURCE_DATE_EPOCH='${SOURCE_DATE_EPOCH}' [$_d]"
+ else
+ warning "SOURCE_DATE_EPOCH='${SOURCE_DATE_EPOCH}' [$_d] lies in
the future"
+ fi
+ unset _d
+ if [ -z "$(printenv SOURCE_DATE_EPOCH 2>/dev/null)" ]
+ then
+ inform "SOURCE_DATE_EPOCH is not exported to the environment"
+ fi
+
case $(peflags --version 2>/dev/null | sed -n '1s/^.* //p') in
4.6.[6-9]|4.[7-9]*|[5-9]*) ;;
*) error "SOURCE_DATE_EPOCH requires peflags 4.6.6 or later"
esac
+else
+ inform "SOURCE_DATE_EPOCH is not set"
fi
--
2.43.0
Re: [PATCH cygport] Increase _FORTIFY_SOURCE level from 2 to 3 in CFLAGS
Jon Turney wrote: On 02/02/2024 16:13, Christian Franke via Cygwin-apps wrote: _FORTIFY_SOURCE=3 is supported by Cygwin 3.5.0 headers and Cygwin gcc 13.2.1 test release. Silently falls back to level 2 if level 3 is unsupported (older headers or gcc) or to level 0 if unsupported at all (C++, clang). Thanks. I applied this. I'm thinking I want to try to do another cygport release fairly soonish, so please feel free to remind me about any other patches by you (or others) which I need to look at before then. Possibly some initial SOURCE_DATE_EPOCH support: https://sourceware.org/pipermail/cygwin-apps/2023-August/043108.html Related: https://cygwin.com/git/?p=newlib-cygwin.git;a=commit;h=f5e37b9
[PATCH cygport] Increase _FORTIFY_SOURCE level from 2 to 3 in CFLAGS
_FORTIFY_SOURCE=3 is supported by Cygwin 3.5.0 headers and Cygwin gcc 13.2.1 test release. Silently falls back to level 2 if level 3 is unsupported (older headers or gcc) or to level 0 if unsupported at all (C++, clang). -- Regards, Christian From 1a32375682d0e5ff6b80a47de70d3d9cfe0f0780 Mon Sep 17 00:00:00 2001 From: Christian Franke Date: Fri, 2 Feb 2024 17:00:18 +0100 Subject: [PATCH] Increase _FORTIFY_SOURCE level from 2 to 3 in CFLAGS This enables buffer overflow checks if the buffer size is non-const but known during runtime and GCC 12.0 or later is used. --- lib/compilers.cygpart | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/compilers.cygpart b/lib/compilers.cygpart index 35e6fe28..52df5304 100644 --- a/lib/compilers.cygpart +++ b/lib/compilers.cygpart @@ -34,9 +34,9 @@ declare -x CC="gcc"; # Flags passed to CC when compiling C code. Individual packages may append # or override this value if they will not build correctly without it. # DEFAULT VALUE -# -ggdb -O2 -pipe -Wall -Werror=format-security -Wp,-D_FORTIFY_SOURCE=2 -fstack-protector-strong --param=ssp-buffer-size=4 +# -ggdb -O2 -pipe -Wall -Werror=format-security -Wp,-D_FORTIFY_SOURCE=3 -fstack-protector-strong --param=ssp-buffer-size=4 # -declare -x CFLAGS="-ggdb -O2 -pipe -Wall -Werror=format-security -Wp,-D_FORTIFY_SOURCE=2 -fstack-protector-strong --param=ssp-buffer-size=4"; +declare -x CFLAGS="-ggdb -O2 -pipe -Wall -Werror=format-security -Wp,-D_FORTIFY_SOURCE=3 -fstack-protector-strong --param=ssp-buffer-size=4"; #v* Compiling/CPPFLAGS # DESCRIPTION -- 2.43.0
Re: [PATCH cygport] Add initial support for SOURCE_DATE_EPOCH
Jon Turney wrote:
On 28/08/2023 16:12, Christian Franke via Cygwin-apps wrote:
Christian Franke wrote:
A small step towards reproducible packaging...
Thanks very much for this. Sorry for taking so long to look at it.
No problem.
A few questions and suggestions interspersed
[...]
--
Regards,
Christian
0001-Add-initial-support-for-SOURCE_DATE_EPOCH.patch
From 73dde4d2dabb74b7b9ee40655204f84e1d4086d6 Mon Sep 17 00:00:00 2001
From: Christian Franke
Date: Mon, 28 Aug 2023 16:24:36 +0200
Subject: [PATCH] Add initial support for SOURCE_DATE_EPOCH
If specified, set the header timestamps of executables and
patch files to SOURCE_DATE_EPOCH.
Suppress the header timestamp of .gz files.
Instruct tar to avoid more recent modification times and
to sort all entries by name.
---
bin/cygport.in | 17 +++--
lib/pkg_pkg.cygpart | 20 ++--
lib/src_postinst.cygpart | 22 +++---
3 files changed, 52 insertions(+), 7 deletions(-)
diff --git a/bin/cygport.in b/bin/cygport.in
index 3f89ac67..e2fe1785 100755
--- a/bin/cygport.in
+++ b/bin/cygport.in
@@ -231,8 +231,9 @@ source ${_privlibdir}/check_funcs.cygpart
###
# check now for all mandatory programs
-for _myprog in bzip2 cat chmod cp diff diffstat dos2unix file find
gawk grep gzip \
- install ln mkdir mv patch rm rsync sed sort tar xargs
which xz
+for _myprog in bzip2 cat chmod cp date diff diffstat dos2unix file
find gawk grep \
+ gzip install ln mkdir mv patch rm rsync sed sort tar
touch which \
+ xargs xz
do
if ! check_prog ${_myprog}
then
@@ -490,6 +491,18 @@ do
done
unset restrict
+if [ "${SOURCE_DATE_EPOCH+y}" = "y" ]
+then
+ if [ -n "$(echo "${SOURCE_DATE_EPOCH}" | sed -e 's/^$/X/' -e
's/[0-9]//g')" ]
+ then
+ error "Malformed SOURCE_DATE_EPOCH: '${SOURCE_DATE_EPOCH}'"
This error message should perhaps say what a well-formed
SOURCE_DATE_EPOCH looks like: an integer number of seconds since the
unix epoch?
error "SOURCE_DATE_EPOCH must be an integer number (seconds since the
unix epoch)" ?
+ fi
+ case $(peflags --version 2>/dev/null | sed -n '1s/^.* //p') in
+ 4.6.[6-9]|4.[7-9]*|[5-9]*) ;;
+ *) error "SOURCE_DATE_EPOCH requires peflags 4.6.6 or later"
+ esac
+fi
+
#
diff --git a/lib/pkg_pkg.cygpart b/lib/pkg_pkg.cygpart
index 2a2bb663..4e6a7cd2 100644
--- a/lib/pkg_pkg.cygpart
+++ b/lib/pkg_pkg.cygpart
@@ -42,7 +42,7 @@ TAR_COMPRESSION_EXT="${TAR_COMPRESSION_EXT:-xz}"
#
__tar() {
- local TAR_COMPRESSION_OPT;
+ local TAR_COMPRESSION_OPT TAR_SOURCE_DATE_OPTS;
# We could use --auto-compress, but this also constrains the
extension
# to the currently valid set. We could probe if tar supports the
@@ -65,7 +65,14 @@ __tar() {
error "tar option for
TAR_COMPRESSION_EXT='${TAR_COMPRESSION_EXT}' unknown"
;;
esac
- tar ${TAR_COMPRESSION_OPT} --owner=Guest:501 --group=None:513
-cvf "$@"
+
+ if [ -n "${SOURCE_DATE_EPOCH}" ]
+ then
+ # Ensure reproducible sort order and last modification times
<= SOURCE_DATE_EPOCH
+ TAR_SOURCE_DATE_OPTS="--sort=name
--mtime=@${SOURCE_DATE_EPOCH} --clamp-mtime"
I'm slightly concerned that maybe this is masking problems elsewhere,
at least when making the source archive: In 2eb7c0eb I started making
an effort so that if the "source inputs" have fixed, upstream
timestamps, we'll preserve those in the output the source package.
(Obviously that's not always going to be the case, e.g. where cygport
patches are from a local git checkout, so maybe that's not a real
problem...)
If the following condition holds, the timestamps of "source inputs" are
not affected:
newest_source_timestamp < SOURCE_DATE_EPOCH < build_time
+ fi
+
+ tar ${TAR_COMPRESSION_OPT} ${TAR_SOURCE_DATE_OPTS}
--owner=Guest:501 --group=None:513 -cvf "$@"
}
__pkg_binpkg() {
@@ -319,6 +326,7 @@ __pkg_diff() {
local difflevel;
local exclude;
local optional_patchfiles;
+ local source_date;
default_excludes="CYGWIN-PATCHES aclocal.m4~ aclocal.m4t
autom4te.cache
config.cache config.guess config.log config.status config.sub
@@ -498,6 +506,14 @@ __pkg_diff() {
sed -b -e '/^diff -u/d' -i ${optional_patchfiles}
${patchdir}/${src_patchfile};
+ if [ -n "${SOURCE_DATE_EPOCH}" ]
+ then
+ # Ensure that the timestamp comment of the new file is
reproducible
"timestamp comments in the generated patch files" or suchlike?
OK.
+ source_date=$(date -d @"${SOURCE_DATE_EPOCH}" -u +'%Y-%m-%d
%H:%M:%S.0 +')
+ se
Re: cygport may not create debug info if top directory contains a symlink
Jon Turney wrote:
On 20/09/2023 11:58, Christian Franke via Cygwin-apps wrote:
Brian Inglis wrote:
On 2023-09-18 04:41, Christian Franke via Cygwin-apps wrote:
Brian Inglis wrote:
On 2023-09-17 08:01, Jon Turney via Cygwin-apps wrote:
On 16/09/2023 15:17, Christian Franke via Cygwin wrote:
Found during tests of busybox package:
If the path of the top build directory contains a symlink and
the project's build scripts normalize pathnames, no debug info
is created by cygport.
This is because options like
-fdebug-prefix-map=${B}=/usr/src/debug/${PF}
have no effect because ${B} contains a symlink but the compiler
is run with the real source path.
I think that there was some historical bug with gcc where a
relative path for the old path in this mapping wasn't correctly
handled, which is why were using an absolute path here at all.
So changing it to something like [1] (if that works), might be
better.
[1]
https://github.com/jon-turney/cygport/commit/4175d456a9184c5cdebd8bfb4b5ba30583cedd66
Should bin/cygport.in:534: not have $B between the == as in line 531:
declare ${flags}+=" -fdebug-prefix-map=${B}=/usr/src/debug/${PF}"
...
declare ${flags}+=" -fdebug-prefix-map==/usr/src/debug/${PF}"
or be hoist above the condition if identical, unless that is some
undocumented default for cwd?
I guess the == without ${B} is intentional because it makes the debug
source path relative to ${B} as lines 535-536 also do.
Yeah, I think that "==" is shorthand for "=.=", i.e. relative to cwd.
Further tests show that "==" behaves strange and is not useful at all
(no problem as it doesn't actually appear in cygport.in).
The "=.=" only does a simple replacement of first "." in source name.
Testcases with file.c in /var/tmp/test:
$ gcc -g file.c && objdump -dl a.exe | grep -F file.c
/var/tmp/test/file.c:1
$ gcc -g ./file.c && objdump -dl a.exe | grep -F file.c
/var/tmp/test/./file.c:1
$ gcc -g ../test/file.c && objdump -dl a.exe | grep -F file.c
/var/tmp/test/../test/file.c:1
$ gcc -g -fdebug-prefix-map==/foo/bar file.c && ...
/foo/bar/foo/barfile.c:1
$ gcc -g -fdebug-prefix-map=.=/foo/bar file.c && ...
/var/tmp/test/file.c:1
$ gcc -g -fdebug-prefix-map=.=/foo/bar ./file.c && ...
/foo/bar/file.c:1
$ gcc -g -fdebug-prefix-map=/var/tmp/test=/foo/bar file.c && ...
/foo/bar/file.c:1
Source directories with symlinks are preserved in line number info like
with 'pwd -L'.
Does using relative paths in the prefix-map resolve the original issue
seen with busybox?
No, relative paths do not work. Adding this to top of src_compile() works:
CFLAGS+=" -fdebug-prefix-map=$(cd ${S} && pwd -P)=/usr/src/debug/${PF}"
Same for ${B} is not required for this project. This also works (B and S
are not set yet):
DEBUG_PREFIX_MAPS=( "$(pwd -P)/${PF}.${ARCH}/src/${PN}-${PV}" )
(It's unclear to me how gcc compares paths to apply this mapping. If
it's a literal string prefix, rather than on some (semi-)canonicalized
path, then we're maybe going to lose here sometimes, depending on the
vagaries of the build-system, unless we list all of relative,
absolute, and canonical absolute paths?)
(But then maybe we can push dealing with or indicating which of those
is correct off onto the individual cygport?)
Adding this if "$(cd ${S} && pwd -P)" != "${S}" should IMO be safe:
-fdebug-prefix-map=$(cd ${B} && pwd -P)=/usr/src/debug/${PF}
-fdebug-prefix-map=$(cd ${S} && pwd -P)=/usr/src/debug/${PF}
(or use realpath)
[PATCH rebase] Fix handling of unset PE checksums
Tests with non-Cygwin executables show that other toolchains (LLVM LLD,
MS link without /RELEASE option) leave the PE checksum field as zero.
Peflags+rebase 4.6.6 still work correctly then, but rebase prints a
misleading warning 'Checksum update failed' in this case. With the
attached patch, a zero checksum is left as is and no message is printed.
This currently does not affect typical Cygwin use cases, so there is IMO
no urgent need for a new package.
BTW, there was possibly no announcement for the rebase 4.6.6 package.
--
Regards,
Christian
From 855be8d72a1f96d0a3cfcb0a97fbe12a9a912748 Mon Sep 17 00:00:00 2001
From: Christian Franke
Date: Mon, 9 Oct 2023 09:56:05 +0200
Subject: [PATCH] Fix handling of unset PE checksums
Leave unset (zero) checksums as is unless checksum update is
explicitly requested. Remove misleading peflags warning
'Checksum update failed'.
Signed-off-by: Christian Franke
---
imagehelper/objectfile.cc | 9 ++--
peflags.c | 45 ++-
rebase.c | 7 +++---
3 files changed, 36 insertions(+), 25 deletions(-)
diff --git a/imagehelper/objectfile.cc b/imagehelper/objectfile.cc
index 651fcce..ce4b444 100755
--- a/imagehelper/objectfile.cc
+++ b/imagehelper/objectfile.cc
@@ -497,13 +497,18 @@ BOOL LinkedObjectFile::updateCheckSum()
p_checksum = ()->OptionalHeader.CheckSum;
else
p_checksum = ()->OptionalHeader.CheckSum;
+ uint32_t checksum = *p_checksum;
+
+ // Leave unset checksum as is
+ if (!checksum)
+return TRUE;
// Add changes from NT header to checksum updated by relocs
uint16_t checksum16;
if (relocs)
checksum16 = relocs->getCheckSum16();
else
-checksum16 = pe32_checksum_update_begin(*p_checksum, FileSize);
+checksum16 = pe32_checksum_update_begin(checksum, FileSize);
if (is64bit ())
checksum16 = pe32_checksum_update_add(checksum16, _ntheader64,
@@ -514,7 +519,7 @@ BOOL LinkedObjectFile::updateCheckSum()
getNTHeader32 (),
sizeof(old_ntheader32));
- uint32_t checksum = pe32_checksum_update_end(checksum16, FileSize);
+ checksum = pe32_checksum_update_end(checksum16, FileSize);
if (!checksum)
return FALSE;
diff --git a/peflags.c b/peflags.c
index 917088b..14ea78d 100644
--- a/peflags.c
+++ b/peflags.c
@@ -508,7 +508,7 @@ do_mark (const char *pathname)
}
- /* Update the checksum. */
+ /* Update the checksum if set. */
changed = FALSE;
{
const void *p_oldheader, *p_newheader;
@@ -529,20 +529,24 @@ do_mark (const char *pathname)
p_checksum = >ntheader32->OptionalHeader.CheckSum;
}
-hdr_checksum = pe32_checksum_update_end(
- pe32_checksum_update_add(
- pe32_checksum_update_begin(*p_checksum, pep->filesize),
- p_oldheader, p_newheader, headersize
- ),
- pep->filesize
-);
-
-if (!hdr_checksum)
- fprintf(stderr, "%s: Checksum update failed\n", pathname);
-else if (*p_checksum != hdr_checksum)
+hdr_checksum = *p_checksum;
+if (hdr_checksum)
{
- changed = TRUE;
- *p_checksum = hdr_checksum;
+ ULONG new_hdr_checksum = pe32_checksum_update_end (
+ pe32_checksum_update_add (
+ pe32_checksum_update_begin (hdr_checksum, pep->filesize),
+ p_oldheader, p_newheader, headersize
+ ),
+ pep->filesize
+ );
+
+ /* Update the checksum or set it to 0 if the old value is invalid. */
+ if (new_hdr_checksum != hdr_checksum
+ && (mark_any || handle_any_sizeof == DO_WRITE))
+ {
+ changed = TRUE;
+ *p_checksum = hdr_checksum = new_hdr_checksum;
+ }
}
}
@@ -650,7 +654,7 @@ static void
do_checksum (const char *pathname, int indent, int changed, ULONG hdr_checksum)
{
const char name[] = "PE file checksum";
- if (checksum_option > 1)
+ if (checksum_option > 1 && (checksum_option > 2 || hdr_checksum))
{
int fd;
unsigned new_checksum, old_checksum;
@@ -684,10 +688,11 @@ do_checksum (const char *pathname, int indent, int
changed, ULONG hdr_checksum)
printf ("%*s%-24s: 0x%08x (*needs update to 0x%08x*)\n", indent, "",
name,
old_checksum, new_checksum);
}
- else /* (checksum_option == 1) */
+ else /* (checksum_option == 1 || (checksum_option == 2 && !hdr_checksum)) */
{
- printf ("%*s%-24s: 0x%08x (%schanged, not verified)\n", indent, "", name,
- hdr_checksum, (changed ? "" : "un"));
+ printf ("%*s%-24s: 0x%08x (%schanged%s)\n", indent, "", name,
+ hdr_checksum, (changed ? "" : "un"),
+ (hdr_checksum ? ", not verified" : "
Re: cygport may not create debug info if top directory contains a symlink
Brian Inglis wrote:
On 2023-09-18 04:41, Christian Franke via Cygwin-apps wrote:
Brian Inglis wrote:
On 2023-09-17 08:01, Jon Turney via Cygwin-apps wrote:
On 16/09/2023 15:17, Christian Franke via Cygwin wrote:
Found during tests of busybox package:
If the path of the top build directory contains a symlink and the
project's build scripts normalize pathnames, no debug info is
created by cygport.
This is because options like
-fdebug-prefix-map=${B}=/usr/src/debug/${PF}
have no effect because ${B} contains a symlink but the compiler is
run with the real source path.
I think that there was some historical bug with gcc where a
relative path for the old path in this mapping wasn't correctly
handled, which is why were using an absolute path here at all.
So changing it to something like [1] (if that works), might be better.
[1]
https://github.com/jon-turney/cygport/commit/4175d456a9184c5cdebd8bfb4b5ba30583cedd66
Should bin/cygport.in:534: not have $B between the == as in line 531:
declare ${flags}+=" -fdebug-prefix-map=${B}=/usr/src/debug/${PF}"
...
declare ${flags}+=" -fdebug-prefix-map==/usr/src/debug/${PF}"
or be hoist above the condition if identical, unless that is some
undocumented default for cwd?
I guess the == without ${B} is intentional because it makes the debug
source path relative to ${B} as lines 535-536 also do.
...
An STC script which creates test dirs to demonstrate the issue and
show the alternative outputs would be nice so anyone can see.
$ ln -s /usr/src /tmp/source
$ cd /tmp/source
$ pwd
/tmp/source
$ /bin/pwd
/usr/src
$ pwd -P
/usr/src
$ /bin/pwd -L
/tmp/source
Thanks, looks good - care to submit a patch, including above
suggestions, to cover all bases?
Users may have a good reason to use a symlinked directory, e.g. fake the
original build path during a rebuild. So I'm still not sure how to
handle this.
- Simply warn the user:
declare -r top=$(cd ${_topdir}; pwd);
+if [ "${top}" != "$(cd ${_topdir}; pwd -P)" ]
+then
+ warning "symlinks in ${top} do not work with some build systems."
+fi
unset _topdir;
- or enforce the real path:
-declare -r top=$(cd ${_topdir}; pwd);
+declare -r top=$(cd ${_topdir}; pwd -P);
+if [ "${top}" != "$(cd ${_topdir}; pwd -L)" ]
+then
+ inform "using real path ${top} as top level directory."
+fi
unset _topdir;
Projects using GNU autotools and cyg{conf,make,install} in cygport are
usually not affected by symlinks in ${top}.
--
Regards,
Christian
Re: cygport may not create debug info if top directory contains a symlink
Brian Inglis wrote:
On 2023-09-17 08:01, Jon Turney via Cygwin-apps wrote:
On 16/09/2023 15:17, Christian Franke via Cygwin wrote:
Found during tests of busybox package:
If the path of the top build directory contains a symlink and the
project's build scripts normalize pathnames, no debug info is
created by cygport.
This is because options like
-fdebug-prefix-map=${B}=/usr/src/debug/${PF}
have no effect because ${B} contains a symlink but the compiler is
run with the real source path.
I think that there was some historical bug with gcc where a relative
path for the old path in this mapping wasn't correctly handled, which
is why were using an absolute path here at all.
So changing it to something like [1] (if that works), might be better.
[1]
https://github.com/jon-turney/cygport/commit/4175d456a9184c5cdebd8bfb4b5ba30583cedd66
Sidenote: we should probably also be using file-prefix-map, now we're
on a gcc which supports it.
Definitely. in particular useful in conjunction with reproducible builds
and this cygport patch:
https://sourceware.org/pipermail/cygwin-apps/2023-August/043108.html
The related newlib-cygwin patch has been pushed already:
https://cygwin.com/git/?p=newlib-cygwin.git;a=commit;h=f5e37b93
The postinstall code then does not find any line number info with
source path /usr/src/debug/${PF}/...
Could be fixed easily in line 414 of /bin/cygport:
-declare -r top=$(cd ${_topdir}; pwd);
+declare -r top=$(cd ${_topdir}; /bin/pwd);
Can you explain why this makes a difference?
In cygport, pwd is a bash builtin defaulting to -L; /bin/pwd defaults
to -P.
Both commands support both options and we might expect the same output.
It would be better to use builtin `pwd -P` if that produces the
correct result.
It does.
An STC script which creates test dirs to demonstrate the issue and
show the alternative outputs would be nice so anyone can see.
$ ln -s /usr/src /tmp/source
$ cd /tmp/source
$ pwd
/tmp/source
$ /bin/pwd
/usr/src
$ pwd -P
/usr/src
$ /bin/pwd -L
/tmp/source
--
Regards,
Christian
Re: [PATCH cygport] Add initial support for SOURCE_DATE_EPOCH
ASSI via Cygwin-apps wrote: Christian Franke via Cygwin-apps writes: If the build-path changes, more files differ (cygcheck.exe, ldd.exe, cygserver.exe) because __FILE__ is used and expands to an absolute path name. This could be fixed by adding -fmacro-prefix and/or -ffile-prefix arguments. Yes, -fmacro-prefix-map should do the trick. Meantime I found a workaround for the additional timestamp in the PE Export Tables header: The ld option --no-insert-timestamp also sets this timestamp to 0. This is apparently preserved by objcopy, regardless of --preserve-dates option. -- Regards, Christian
Re: [PATCH cygport] Add initial support for SOURCE_DATE_EPOCH
Christian Franke wrote:
A small step towards reproducible packaging...
Currently only tested with upcoming smartmontools package - contains
only exe, man, doc files (no dll, lib, ...). Multiple cygport runs
produce binary identical distribution tarballs if SOURCE_DATE_EPOCH
(from the past) is specified in the cygport file.
A slightly enhanced patch is attached. For .gz files, 'gzip -n' is now
used as suggested.
Result from a test with the cygwin-3.4.8-1 source package:
These two lines were added to cygwin.cygport:
VERSION="3.4.8"
RELEASE="1"
+export SOURCE_DATE_EPOCH=$(date -d '2023-08-18 UTC' +%s)
+export TZ=UTC
(TZ=UTC is a workaround for a minor bug in SOURCE_DATE_EPOCH handling of
gropdf)
These distribution files are always identical:
- cygwin-3.4.8-1-src.tar.xz
- cygwin-doc-3.4.8-1.tar.xz
These individual files differ if the build-path of multiple test-builds
is always the same:
- usr/bin/cygwin1.dll:
-- Contains a build timestamp not based on '__DATE__ __TIME__'. Could be
easily addressed in scripts/mkvers.sh.
-- The PE Export Tables header contains a timestamp independent from the
PE header. There is possibly no easy way to fix this without an extra
tool (or further enhancing peflags).
- usr/lib/lib*.a:
-- Object file time timestamps differ. Could be addressed by adding 'ar'
option 'D' (deterministic) in Makefiles and scripts/speclib.
- usr/lib/debug/usr/bin/cygwin1.dll.dbg: As expected, because
cygwin1.dll differ.
If the build-path changes, more files differ (cygcheck.exe, ldd.exe,
cygserver.exe) because __FILE__ is used and expands to an absolute path
name.
--
Regards,
Christian
From 73dde4d2dabb74b7b9ee40655204f84e1d4086d6 Mon Sep 17 00:00:00 2001
From: Christian Franke
Date: Mon, 28 Aug 2023 16:24:36 +0200
Subject: [PATCH] Add initial support for SOURCE_DATE_EPOCH
If specified, set the header timestamps of executables and
patch files to SOURCE_DATE_EPOCH.
Suppress the header timestamp of .gz files.
Instruct tar to avoid more recent modification times and
to sort all entries by name.
---
bin/cygport.in | 17 +++--
lib/pkg_pkg.cygpart | 20 ++--
lib/src_postinst.cygpart | 22 +++---
3 files changed, 52 insertions(+), 7 deletions(-)
diff --git a/bin/cygport.in b/bin/cygport.in
index 3f89ac67..e2fe1785 100755
--- a/bin/cygport.in
+++ b/bin/cygport.in
@@ -231,8 +231,9 @@ source ${_privlibdir}/check_funcs.cygpart
###
# check now for all mandatory programs
-for _myprog in bzip2 cat chmod cp diff diffstat dos2unix file find gawk grep
gzip \
- install ln mkdir mv patch rm rsync sed sort tar xargs which xz
+for _myprog in bzip2 cat chmod cp date diff diffstat dos2unix file find gawk
grep \
+ gzip install ln mkdir mv patch rm rsync sed sort tar touch
which \
+ xargs xz
do
if ! check_prog ${_myprog}
then
@@ -490,6 +491,18 @@ do
done
unset restrict
+if [ "${SOURCE_DATE_EPOCH+y}" = "y" ]
+then
+ if [ -n "$(echo "${SOURCE_DATE_EPOCH}" | sed -e 's/^$/X/' -e
's/[0-9]//g')" ]
+ then
+ error "Malformed SOURCE_DATE_EPOCH: '${SOURCE_DATE_EPOCH}'"
+ fi
+ case $(peflags --version 2>/dev/null | sed -n '1s/^.* //p') in
+ 4.6.[6-9]|4.[7-9]*|[5-9]*) ;;
+ *) error "SOURCE_DATE_EPOCH requires peflags 4.6.6 or later"
+ esac
+fi
+
#
diff --git a/lib/pkg_pkg.cygpart b/lib/pkg_pkg.cygpart
index 2a2bb663..4e6a7cd2 100644
--- a/lib/pkg_pkg.cygpart
+++ b/lib/pkg_pkg.cygpart
@@ -42,7 +42,7 @@ TAR_COMPRESSION_EXT="${TAR_COMPRESSION_EXT:-xz}"
#
__tar() {
- local TAR_COMPRESSION_OPT;
+ local TAR_COMPRESSION_OPT TAR_SOURCE_DATE_OPTS;
# We could use --auto-compress, but this also constrains the extension
# to the currently valid set. We could probe if tar supports the
@@ -65,7 +65,14 @@ __tar() {
error "tar option for
TAR_COMPRESSION_EXT='${TAR_COMPRESSION_EXT}' unknown"
;;
esac
- tar ${TAR_COMPRESSION_OPT} --owner=Guest:501 --group=None:513 -cvf "$@"
+
+ if [ -n "${SOURCE_DATE_EPOCH}" ]
+ then
+ # Ensure reproducible sort order and last modification times <=
SOURCE_DATE_EPOCH
+ TAR_SOURCE_DATE_OPTS="--sort=name --mtime=@${SOURCE_DATE_EPOCH}
--clamp-mtime"
+ fi
+
+ tar ${TAR_COMPRESSION_OPT} ${TAR_SOURCE_DATE_OPTS} --owner=Guest:501
--group=None:513 -cvf "$@"
}
__pkg_binpkg() {
@@ -319,6 +326,7 @@ __pkg_diff() {
local difflevel;
local exclude;
local optional_patchfiles;
+ local source_date;
default_excludes="CYGWIN-PATCHES aclocal.m4~ aclocal.m4t autom4te.cache
Re: [PATCH cygport] Add initial support for SOURCE_DATE_EPOCH
ASSI via Cygwin-apps wrote:
Christian Franke via Cygwin-apps writes:
If binutils, gzip and tar also would support this, the patch would be
empty :-)
GZip has -n though ...
For gzip I decided to keep the timestamp and use (the GNU version of)
touch instead.
and GNU tar --mtime and --clamp-mtime, so why not use
that?
Already done:
+ if [ -n "${SOURCE_DATE_EPOCH}" ]
+ then
+ # Ensure reproducible sort order and last modification times <=
SOURCE_DATE_EPOCH
+ TAR_SOURCE_DATE_OPTS="--sort=name --mtime=@${SOURCE_DATE_EPOCH}
--clamp-mtime"
+ fi
:-)
--
Regards,
Christian
Re: [PATCH cygport] Add initial support for SOURCE_DATE_EPOCH
Brian Inglis via Cygwin-apps wrote: On 2023-08-23 11:39, Christian Franke via Cygwin-apps wrote: A small step towards reproducible packaging... Currently only tested with upcoming smartmontools package - contains only exe, man, doc files (no dll, lib, ...). Multiple cygport runs produce binary identical distribution tarballs if SOURCE_DATE_EPOCH (from the past) is specified in the cygport file. What format is this variable? Unix epoch, see: https://reproducible-builds.org/docs/source-date-epoch/ Some tools like gcc and gropdf already support this: $ echo '__DATE__ __TIME__' | SOURCE_DATE_EPOCH=1672531200 gcc -E -xc - # 0 "" # 0 "" # 0 "" # 1 "" "Jan 1 2023" "00:00:00" $ TZ=UTC SOURCE_DATE_EPOCH=1672531200 man -Tpdf ls | TZ=UTC pdfinfo - Creator: groff version 1.23.0 Producer: gropdf version 1.23.0 CreationDate: Sun Jan 1 00:00:00 2023 UTC ModDate: Sun Jan 1 00:00:00 2023 UTC ... If binutils, gzip and tar also would support this, the patch would be empty :-) Would be best to use some ISO format e.g. %Y-%m-%dT%H:%M:%SZ or %Y-%m-%dT%H:%M:%S [local] that is human readable and can be used by all POSIX command options [e.g. see touch(1p) -d] that accept date-time stamps. I use this in smartmontools.cygport: SOURCE_DATE=2023-08-23 SOURCE_DATE_EPOCH=$(date -d "$SOURCE_DATE UTC" +%s) This relies on GNU date, BSD date is very different, a POSIX-only date would be useless in this context. Could do with docs and NEWS entries for new variables, otherwise packagers do not know it exists, and how to use it e.g. #f* Information/BUILD_REQUIRES # SYNOPSIS ... Of course this should be documented, but I left this for a later patch. -- Regards, Christian
[PATCH cygport] Add initial support for SOURCE_DATE_EPOCH
A small step towards reproducible packaging...
Currently only tested with upcoming smartmontools package - contains
only exe, man, doc files (no dll, lib, ...). Multiple cygport runs
produce binary identical distribution tarballs if SOURCE_DATE_EPOCH
(from the past) is specified in the cygport file.
--
Regards,
Christian
From 146b1df83a20ccd71e57d6123c7ee24b8390ca3a Mon Sep 17 00:00:00 2001
From: Christian Franke
Date: Wed, 23 Aug 2023 19:27:02 +0200
Subject: [PATCH] Add initial support for SOURCE_DATE_EPOCH
If specified, ensure that the header timestamps of executables
and of compressed man pages are set to this value.
Instruct tar to avoid more recent modification times and
sort all entries by name.
---
bin/cygport.in | 14 +-
lib/pkg_pkg.cygpart | 11 +--
lib/src_postinst.cygpart | 21 +
3 files changed, 43 insertions(+), 3 deletions(-)
diff --git a/bin/cygport.in b/bin/cygport.in
index 3f89ac67..f7c476b0 100755
--- a/bin/cygport.in
+++ b/bin/cygport.in
@@ -232,7 +232,7 @@ source ${_privlibdir}/check_funcs.cygpart
# check now for all mandatory programs
for _myprog in bzip2 cat chmod cp diff diffstat dos2unix file find gawk grep
gzip \
- install ln mkdir mv patch rm rsync sed sort tar xargs which xz
+ install ln mkdir mv patch rm rsync sed sort tar touch xargs
which xz
do
if ! check_prog ${_myprog}
then
@@ -490,6 +490,18 @@ do
done
unset restrict
+if [ "${SOURCE_DATE_EPOCH+y}" = "y" ]
+then
+ if [ -n "$(echo "${SOURCE_DATE_EPOCH}" | sed -e 's/^$/X/' -e
's/[0-9]//g')" ]
+ then
+ error "Malformed SOURCE_DATE_EPOCH: '${SOURCE_DATE_EPOCH}'"
+ fi
+ case $(peflags --version 2>/dev/null | sed -n '1s/^.* //p') in
+ 4.6.[6-9]|4.[7-9]*|[5-9]*) ;;
+ *) error "SOURCE_DATE_EPOCH requires peflags 4.6.6 or later"
+ esac
+fi
+
#
diff --git a/lib/pkg_pkg.cygpart b/lib/pkg_pkg.cygpart
index 2a2bb663..3869bdb7 100644
--- a/lib/pkg_pkg.cygpart
+++ b/lib/pkg_pkg.cygpart
@@ -42,7 +42,7 @@ TAR_COMPRESSION_EXT="${TAR_COMPRESSION_EXT:-xz}"
#
__tar() {
- local TAR_COMPRESSION_OPT;
+ local TAR_COMPRESSION_OPT TAR_SOURCE_DATE_OPTS;
# We could use --auto-compress, but this also constrains the extension
# to the currently valid set. We could probe if tar supports the
@@ -65,7 +65,14 @@ __tar() {
error "tar option for
TAR_COMPRESSION_EXT='${TAR_COMPRESSION_EXT}' unknown"
;;
esac
- tar ${TAR_COMPRESSION_OPT} --owner=Guest:501 --group=None:513 -cvf "$@"
+
+ if [ -n "${SOURCE_DATE_EPOCH}" ]
+ then
+ # Ensure reproducible sort order and last modification times <=
SOURCE_DATE_EPOCH
+ TAR_SOURCE_DATE_OPTS="--sort=name --mtime=@${SOURCE_DATE_EPOCH}
--clamp-mtime"
+ fi
+
+ tar ${TAR_COMPRESSION_OPT} ${TAR_SOURCE_DATE_OPTS} --owner=Guest:501
--group=None:513 -cvf "$@"
}
__pkg_binpkg() {
diff --git a/lib/src_postinst.cygpart b/lib/src_postinst.cygpart
index dd947311..f0ab0f8b 100644
--- a/lib/src_postinst.cygpart
+++ b/lib/src_postinst.cygpart
@@ -775,6 +775,11 @@ __prepman() {
while read -d $'\0' manpage
do
echo "${manpage##*/}";
+ if [ -n "${SOURCE_DATE_EPOCH}" ]
+ then
+ # Ensure that the timestamp in gzip header is
reproducible
+ touch -d @${SOURCE_DATE_EPOCH} "${manpage}"
+ fi
gzip -q "${manpage}";
done
fi
@@ -989,6 +994,12 @@ __prepstrip_one() {
objdump=${objcopy/copy/dump}
+ if [ -n "${SOURCE_DATE_EPOCH}" ]
+ then
+ # Let objcopy preserve the timestamps
+ objcopy+=" --enable-deterministic-archives --preserve-dates"
+ fi
+
# Static libraries should not be fully stripped, but we can
# still provide split debuginfo if desired
case "${exe}" in
@@ -1074,6 +1085,16 @@ __prepstrip_one() {
# keep sticky bit if present
chmod u+w,a+x "${exe}";
+ if [ -n "${SOURCE_DATE_EPOCH}" ]
+ then
+ case "${exe}" in
+ *.exe|*.dll|*.so|*.so.*|*.oct|*.mex|*.cmxs)
+ # Ensure PE header timestamp is reproducible and
checksum is correct
+ # objcopy later inherits the timestamp to debug info
and stripped file
+ peflags --checksum=1 --timestamp=${SOURCE_DATE_EPOCH}
${exe} >/dev/null ;;
+ esac
+ fi
+
if defined _CYGPORT_RESTRICT_debuginfo_
then
${objcopy} --strip-all "${exe}";
--
2.39.0
Re: [PATCH rebase 1/2] Always update the file checksum in the PE header
Corinna Vinschen wrote: On Aug 12 14:26, Christian Franke via Cygwin-apps wrote: This patch is still experimental, but tested with all /bin/cyg*.dll from my installation. Does that mean I shouldn't apply it for now, or do you want me to apply it but not create a new release yet? Meantime I did more tests, also a few with 32-bit DLLs, which all succeeded. So it should be safe to use. In conjunction with the "Don't update the PE header timestamp unless -t is used" patch, rebase results are reproducible. -- Regards, Christian
[PATCH rebase 2/2] rebase: Don't update the PE header timestamp unless -t is used
This changes existing behavior but a new option --keep-timestamp is IMO
not needed.
--
Regards,
Christian
From 1c13ccb047ebfbcd2f239bedcd50a128fec659e9 Mon Sep 17 00:00:00 2001
From: Christian Franke
Date: Sat, 12 Aug 2023 14:17:04 +0200
Subject: [PATCH] rebase: Don't update the PE header timestamp unless -t is
used
This enables reproducible rebase.
Signed-off-by: Christian Franke
---
imagehelper/rebaseimage.cc | 6 --
rebase.c | 3 ++-
2 files changed, 6 insertions(+), 3 deletions(-)
diff --git a/imagehelper/rebaseimage.cc b/imagehelper/rebaseimage.cc
index 8827a14..0dec93c 100755
--- a/imagehelper/rebaseimage.cc
+++ b/imagehelper/rebaseimage.cc
@@ -111,12 +111,14 @@ BOOL ReBaseImage64 (
if (dll.is64bit ())
{
ntheader64->OptionalHeader.ImageBase = *NewImageBase;
- ntheader64->FileHeader.TimeDateStamp = TimeStamp;
+ if (ReBaseChangeFileTime)
+ ntheader64->FileHeader.TimeDateStamp = TimeStamp;
}
else
{
ntheader32->OptionalHeader.ImageBase = *NewImageBase;
- ntheader32->FileHeader.TimeDateStamp = TimeStamp;
+ if (ReBaseChangeFileTime)
+ ntheader32->FileHeader.TimeDateStamp = TimeStamp;
}
int64_t difference = *NewImageBase - *OldImageBase;
diff --git a/rebase.c b/rebase.c
index 20a9902..6a531d0 100644
--- a/rebase.c
+++ b/rebase.c
@@ -1687,7 +1687,8 @@ Rebase PE files, usually DLLs, to a specified address or
address range.\n\
-o, --offset=OFFSET Specify an additional offset between adjacent DLLs\n\
when rebasing. Default is no offset.\n\
-t, --touch Use this option to make sure the file's
modification\n\
- time is bumped if it has been successfully
rebased.\n\
+ time and the timestamp in the PE header are bumped
if\n\
+ the file has been successfully rebased.\n\
Usually rebase does not change the file's time
unless\n\
the -c flag is also specified.\n\
-T, --filelist=FILE Also rebase the files specified in FILE. The
format\n\
--
2.39.0
[PATCH rebase 1/2] Always update the file checksum in the PE header
This patch is still experimental, but tested with all /bin/cyg*.dll from
my installation.
--
Regards,
Christian
From 53663d46c2c989e665143b33c0614b416fd1c666 Mon Sep 17 00:00:00 2001
From: Christian Franke
Date: Sat, 12 Aug 2023 14:13:43 +0200
Subject: [PATCH] Always update the file checksum in the PE header
Both peflags and rebase now update the checksum without reading
the whole file under the assumption that the original checksum
was correct. Calculation is done by new module
'imagehelper/pechecksum_update.c'.
Slightly change the peflags -k, --checksum option accordingly.
Signed-off-by: Christian Franke
---
Makefile.in | 6 +-
imagehelper/Makefile.in | 7 ++-
imagehelper/imagehelper.h | 4 ++
imagehelper/objectfile.cc | 62 +++---
imagehelper/objectfile.h| 7 +++
imagehelper/pechecksum_update.c | 56 +
imagehelper/pechecksum_update.h | 52
imagehelper/rebaseimage.cc | 4 ++
imagehelper/sections.cc | 20 +-
imagehelper/sections.h | 8 ++-
peflags.c | 107 +---
rebase.c| 39
12 files changed, 305 insertions(+), 67 deletions(-)
create mode 100644 imagehelper/pechecksum_update.c
create mode 100644 imagehelper/pechecksum_update.h
diff --git a/Makefile.in b/Makefile.in
index e7b7f6a..edc9eda 100644
--- a/Makefile.in
+++ b/Makefile.in
@@ -81,7 +81,7 @@ REBASE_DUMP_OBJS = rebase-dump.$(O) rebase-db.$(O) $(LIBOBJS)
REBASE_DUMP_LIBS =
PEFLAGS_OBJS = peflags.$(O) pechecksum.$(O) $(LIBOBJS)
-PEFLAGS_LIBS =
+PEFLAGS_LIBS = $(LIBIMAGEHELPER)
SRC_DISTFILES = configure.ac configure Makefile.in \
peflagsall.in rebaseall.in \
@@ -109,8 +109,8 @@ rebase-dump$(EXEEXT): $(REBASE_DUMP_OBJS)
rebase-dump.$(O):: rebase-dump.c rebase-db.h Makefile
-peflags$(EXEEXT): $(PEFLAGS_OBJS)
- $(CC) $(CFLAGS) $(LDFLAGS) -o $@ $(PEFLAGS_OBJS)
+peflags$(EXEEXT): $(PEFLAGS_OBJS) $(PEFLAGS_LIBS)
+ $(CXX) $(CXXFLAGS) $(LDFLAGS) $(CXX_LDFLAGS) -o $@ $(PEFLAGS_OBJS)
$(PEFLAGS_LIBS)
peflags.$(O):: peflags.c pechecksum.h Makefile
diff --git a/imagehelper/Makefile.in b/imagehelper/Makefile.in
index 0ed8e6c..a40cd8c 100755
--- a/imagehelper/Makefile.in
+++ b/imagehelper/Makefile.in
@@ -75,11 +75,12 @@ LIB_TARGET=imagehelper
LIB_TARGET_FILE=libimagehelper.a
LIB_OBJS = objectfile.$(O) objectfilelist.$(O) sections.$(O) debug.$(O) \
rebaseimage.$(O) checkimage.$(O) fiximage.$(O) getimageinfos.$(O) \
- bindimage.$(O)
+ bindimage.$(O) pechecksum_update.$(O)
LIB_SRCS = objectfile.cc objectfilelist.cc sections.cc debug.cc \
rebaseimage.cc checkimage.cc fiximage.cc getimageinfos.cc \
- bindimage.cc
-LIB_HDRS = objectfilelist.h imagehelper.h sections.h objectfile.h
+ bindimage.cc pechecksum_update.c
+LIB_HDRS = objectfilelist.h imagehelper.h sections.h objectfile.h \
+ pechecksum_update.h
#
# (obsolete) applications
diff --git a/imagehelper/imagehelper.h b/imagehelper/imagehelper.h
index bafc106..8535311 100755
--- a/imagehelper/imagehelper.h
+++ b/imagehelper/imagehelper.h
@@ -33,6 +33,10 @@ extern BOOL ReBaseChangeFileTime;
/* Set to TRUE, if rebasing should also drop the /DYNAMICBASE flag
from the PE flags. */
extern BOOL ReBaseDropDynamicbaseFlag;
+/* Set to TRUE if ReBaseImage{64} should update the PE checksum.
+ Cleared if the update failed due to inconsistent original
+ checksum. */
+extern BOOL ReBaseUpdateCheckSum;
BOOL ReBaseImage64(
LPCSTR CurrentImageName,
diff --git a/imagehelper/objectfile.cc b/imagehelper/objectfile.cc
index 4dafa6a..651fcce 100755
--- a/imagehelper/objectfile.cc
+++ b/imagehelper/objectfile.cc
@@ -24,6 +24,7 @@
#include
#include "objectfile.h"
+#include "pechecksum_update.h"
// read a dll into the cache
@@ -109,6 +110,15 @@ ObjectFile::ObjectFile(const char *aFileName, bool
writeable)
FileName = strdup(name);
}
+ LARGE_INTEGER fs;
+ if (!GetFileSizeEx(hfile, ))
+{
+ CloseHandle(hfile);
+ Error = 2;
+ return;
+}
+ FileSize = fs.QuadPart;
+
hfilemapping = CreateFileMapping(hfile, NULL, writeable ? PAGE_READWRITE :
PAGE_READONLY , 0, 0, NULL);
if (hfilemapping == 0)
{
@@ -139,8 +149,7 @@ ObjectFile::ObjectFile(const char *aFileName, bool
writeable)
return;
}
// filesize big enough to allow at least reading the NT header?
- if (GetFileSize (hfile, NULL)
- < (char *) ntheader - (char *) dosheader + sizeof *ntheader)
+ if (FileSize < (char *) ntheader - (char *) dosheader + sizeof *ntheader)
{
Error = 4;
return;
@@ -178,9 +187,6 @@ ObjectFile::~ObjectFile()
CloseHandle(hfile);
}
-
-
-
int LinkedObjectFile::level = 0;
LinkedObjectFile::LinkedObjectFile(const char *aFileName, bool writable) :
ObjectFile(aFileName,writable)
@@ -204,6 +210,12 @@ LinkedOb
Re: [PATCH rebase] rebase: Add -c, --checksum option
Jon Turney wrote: On 08/08/2023 11:10, Christian Franke via Cygwin-apps wrote: Last patch for now. Left for later: ReBaseImage64() unconditionally updates the timestamp in the file header. Just for clarity: does this mean that rebasing as it is currently implemented leaves the PE file with an invalid checksum, and the Windows loader is just fine this that? Or is there something else happening which makes the checksum wrong? Both rebase and peflags leave the PE file with an invalid checksum. This is no problem for Cygwin uses cases: "Checksums are required for kernel-mode drivers and some system DLLs." https://learn.microsoft.com/en-us/windows/win32/api/imagehlp/nf-imagehlp-mapfileandchecksuma -- Regards, Christian
[PATCH rebase] rebase: Add -c, --checksum option
Last patch for now.
Left for later: ReBaseImage64() unconditionally updates the timestamp in
the file header.
--
Regards,
Christian
From 3973a92cf11056521552d9d3f87efe8e721e8c31 Mon Sep 17 00:00:00 2001
From: Christian Franke
Date: Tue, 8 Aug 2023 12:04:25 +0200
Subject: [PATCH] rebase: Add -c, --checksum option
If specified, the file checksum in the PE header is updated after
rebasing.
Signed-off-by: Christian Franke
---
Makefile.in | 4 ++--
rebase.c| 50 ++
2 files changed, 48 insertions(+), 6 deletions(-)
diff --git a/Makefile.in b/Makefile.in
index adb7972..2f061ca 100644
--- a/Makefile.in
+++ b/Makefile.in
@@ -74,7 +74,7 @@ override CXX_LDFLAGS+=@EXTRA_CXX_LDFLAG_OVERRIDES@
LIBIMAGEHELPER = imagehelper/libimagehelper.a
-REBASE_OBJS = rebase.$(O) rebase-db.$(O) $(LIBOBJS)
+REBASE_OBJS = rebase.$(O) rebase-db.$(O) pechecksum.$(O) $(LIBOBJS)
REBASE_LIBS = $(LIBIMAGEHELPER)
REBASE_DUMP_OBJS = rebase-dump.$(O) rebase-db.$(O) $(LIBOBJS)
@@ -100,7 +100,7 @@ $(LIBIMAGEHELPER):
rebase$(EXEEXT): $(REBASE_LIBS) $(REBASE_OBJS)
$(CXX) $(CXXFLAGS) $(LDFLAGS) $(CXX_LDFLAGS) -o $@ $(REBASE_OBJS)
$(REBASE_LIBS)
-rebase.$(O):: rebase.c rebase-db.h Makefile
+rebase.$(O):: rebase.c pechecksum.h rebase-db.h Makefile
rebase-db.$(O):: rebase-db.c rebase-db.h Makefile
diff --git a/rebase.c b/rebase.c
index 7417d4d..50cc79f 100644
--- a/rebase.c
+++ b/rebase.c
@@ -39,6 +39,7 @@
#include
#include
#include "imagehelper.h"
+#include "pechecksum.h"
#include "rebase-db.h"
#include /* requires */
@@ -74,6 +75,7 @@ WORD machine = IMAGE_FILE_MACHINE_I386;
ULONG64 image_base = 0;
ULONG64 low_addr;
BOOL down_flag = FALSE;
+BOOL checksum_flag = FALSE;
BOOL image_info_flag = FALSE;
BOOL image_storage_flag = FALSE;
BOOL image_oblivious_flag = FALSE;
@@ -1188,6 +1190,32 @@ print_overlapped ()
}
}
+static BOOL
+update_checksum (const char *pathname)
+{
+ int fd, err;
+ unsigned old_checksum, new_checksum;
+
+ if ((fd = open (pathname, O_RDWR | O_BINARY)) == -1)
+{
+ fprintf (stderr, "%s: failed to reopen for checksum update: %s\n",
+ pathname, strerror (errno));
+ return FALSE;
+}
+ new_checksum = pe32_checksum (fd, 1, _checksum);
+ err = errno;
+ close(fd);
+ if (!new_checksum)
+{
+ fprintf (stderr, "%s: checksum update failed: %s\n", pathname,
+ strerror (err));
+ /* Assume file is unchanged. */
+ return FALSE;
+}
+
+ return (new_checksum != old_checksum);
+}
+
BOOL
rebase (const char *pathname, ULONG64 *new_image_base, BOOL down_flag)
{
@@ -1279,13 +1307,18 @@ retry:
}
#endif
+ /* Update checksum, if requested. */
+ status = (checksum_flag ? update_checksum (pathname) : FALSE);
+
/* Display rebase results, if verbose. */
if (verbose)
{
- printf ("%s: new base = %" PRIx64 ", new size = %x\n",
+ printf ("%s: new base = %" PRIx64 ", new size = %x%s\n",
pathname,
(uint64_t) ((down_flag) ? *new_image_base : prev_new_image_base),
- (uint32_t) (new_image_size + offset));
+ (uint32_t) (new_image_size + offset),
+ (checksum_flag ? (status ? ", checksum updated" :
+ ", checksum unchanged") : ""));
}
/* Calculate next base address, if rebasing up. */
@@ -1299,6 +1332,7 @@ static struct option long_options[] = {
{"32", no_argument, NULL, '4'},
{"64", no_argument, NULL, '8'},
{"base", required_argument, NULL, 'b'},
+ {"checksum", no_argument, NULL, 'c'},
{"down", no_argument, NULL, 'd'},
{"help", no_argument, NULL, 'h'},
{"usage",no_argument, NULL, 'h'},
@@ -1316,7 +1350,7 @@ static struct option long_options[] = {
{NULL, no_argument, NULL, 0 }
};
-static const char *short_options = "48b:dhiMno:OqstT:vV";
+static const char *short_options = "48b:cdhiMno:OqstT:vV";
void
parse_args (int argc, char *argv[])
@@ -1341,6 +1375,9 @@ parse_args (int argc, char *argv[])
image_base = string_to_ulonglong (optarg);
force_rebase_flag = TRUE;
break;
+ case 'c':
+ checksum_flag = TRUE;
+ break;
case 'd':
down_flag = TRUE;
break;
@@ -1625,6 +1662,10 @@ Rebase PE files, usually DLLs, to a specified address or
address range.\n\
One of the options -b, -s or -i is mandatory. If no rebase database
exists\n\
yet, -b is required together with -s.\n\
\n\
+ -c, --checksum Update the file's checksum in the PE header if the\n\
+ file has been successfully rebased. This also
bumps\n\
+ the file's modificatio
[PATCH rebase] Add missing pechecksum.* to SRC_DISTFILES
Missed yesterday, sorry. -- Regards, Christian From 84065da466d9501d0522af8860ea829ee51c12f5 Mon Sep 17 00:00:00 2001 From: Christian Franke Date: Tue, 8 Aug 2023 10:52:14 +0200 Subject: [PATCH] Add missing pechecksum.* to SRC_DISTFILES Signed-off-by: Christian Franke --- Makefile.in | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Makefile.in b/Makefile.in index 46df1d5..adb7972 100644 --- a/Makefile.in +++ b/Makefile.in @@ -84,7 +84,8 @@ PEFLAGS_OBJS = peflags.$(O) pechecksum.$(O) $(LIBOBJS) PEFLAGS_LIBS = SRC_DISTFILES = configure.ac configure Makefile.in \ - peflagsall.in rebaseall.in peflags.c rebase.c \ + peflagsall.in rebaseall.in \ + pechecksum.c pechecksum.h peflags.c rebase.c \ build.sh ChangeLog COPYING NEWS README setup.hint Todo \ build-aux/config.guess build-aux/config.sub \ build-aux/install-sh getopt.h_ getopt_long.c \ -- 2.39.0
Re: [PATCH rebase] peflags: Fix ULONG range checks
Corinna Vinschen wrote: On Aug 8 10:06, Christian Franke via Cygwin-apps wrote: Corinna Vinschen via Cygwin-apps wrote: Hi Christian, On Aug 7 16:07, Christian Franke via Cygwin-apps wrote: Minor issue found during tests of the upcoming 'peflags --timestamp' patch. -- Regards, Christian ... diff --git a/peflags.c b/peflags.c index 93eaa0b..d98b121 100644 --- a/peflags.c +++ b/peflags.c @@ -30,7 +30,6 @@ #include #include #include -#include #if defined (__CYGWIN__) || defined (__MSYS__) #include #endif @@ -598,7 +597,7 @@ handle_num_option (const char *option_name, || sizeof_vals[option_index].value > 0xULL /* Just a ULONG value */ || (sizeof_vals[option_index].is_ulong - && sizeof_vals[option_index].value > ULONG_MAX)) + && sizeof_vals[option_index].value > 0xULL)) What about using MAXDWORD or MAXULONG32 instead? Of course :-) Christian Pushed. I've started deploying a new release. I'm currently working on 'rebase -c, --checksum' and found one minor issue: pechecksum.* are missing in SRC_DISTFILES.
Re: [PATCH rebase] peflags: Fix ULONG range checks
Corinna Vinschen via Cygwin-apps wrote:
Hi Christian,
On Aug 7 16:07, Christian Franke via Cygwin-apps wrote:
Minor issue found during tests of the upcoming 'peflags --timestamp' patch.
--
Regards,
Christian
...
diff --git a/peflags.c b/peflags.c
index 93eaa0b..d98b121 100644
--- a/peflags.c
+++ b/peflags.c
@@ -30,7 +30,6 @@
#include
#include
#include
-#include
#if defined (__CYGWIN__) || defined (__MSYS__)
#include
#endif
@@ -598,7 +597,7 @@ handle_num_option (const char *option_name,
|| sizeof_vals[option_index].value > 0xULL
/* Just a ULONG value */
|| (sizeof_vals[option_index].is_ulong
- && sizeof_vals[option_index].value > ULONG_MAX))
+ && sizeof_vals[option_index].value > 0xULL))
What about using MAXDWORD or MAXULONG32 instead?
Of course :-)
Christian
From 8c8537fbc08d677651eee3055e5b0c6c9873804d Mon Sep 17 00:00:00 2001
From: Christian Franke
Date: Tue, 8 Aug 2023 09:58:39 +0200
Subject: [PATCH] peflags: Fix ULONG range checks
Don't use ULONG_MAX from because ULONG is not necessarily
'unsigned long'. Use MAXULONG32 instead.
Signed-off-by: Christian Franke
---
peflags.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/peflags.c b/peflags.c
index f215704..1a61da7 100644
--- a/peflags.c
+++ b/peflags.c
@@ -30,7 +30,6 @@
#include
#include
#include
-#include
#if defined (__CYGWIN__) || defined (__MSYS__)
#include
#endif
@@ -696,7 +695,7 @@ handle_num_option (const char *option_name,
|| sizeof_vals[option_index].value > 0xULL
/* Just a ULONG value */
|| (sizeof_vals[option_index].is_ulong
- && sizeof_vals[option_index].value > ULONG_MAX))
+ && sizeof_vals[option_index].value > MAXULONG32))
{
fprintf (stderr, "Invalid argument for %s: %s\n",
option_name, option_arg);
@@ -1092,7 +1091,7 @@ get_and_set_size (const pe_file *pep, sizeof_values_t
*val)
}
else if (val->handle == DO_WRITE)
{
- if ((!pep->is_64bit || val->is_ulong) && val->value >= ULONG_MAX)
+ if ((!pep->is_64bit || val->is_ulong) && val->value > MAXULONG32)
{
fprintf (stderr, "%s: Skip writing %s, value too big\n",
pep->pathname, val->name);
--
2.39.0
[PATCH rebase 2/2] peflags: Add -k, --checksum option
This patch is on top of the --timestamp patch. Could not be applied to
current HEAD without conflicts.
--
Regards,
Christian
From 9ecaf86bff5d229bf5b2a1ba1ff4674526fc1b68 Mon Sep 17 00:00:00 2001
From: Christian Franke
Date: Mon, 7 Aug 2023 15:52:14 +0200
Subject: [PATCH] peflags: Add -k, --checksum option
This allows to fix the file checksum in the PE header.
An invalid checksum may break reproducible builds or may
increase the risk of false positive malware detections.
The checksum calculation is done by a new self-contained module
'pechecksum.c' which could also be built as a stand-alone tool
or later added to rebase.
Signed-off-by: Christian Franke
---
Makefile.in | 6 +-
pechecksum.c | 195 +++
pechecksum.h | 25 +++
peflags.c| 129 --
4 files changed, 347 insertions(+), 8 deletions(-)
create mode 100644 pechecksum.c
create mode 100644 pechecksum.h
diff --git a/Makefile.in b/Makefile.in
index 34c4684..46df1d5 100644
--- a/Makefile.in
+++ b/Makefile.in
@@ -80,7 +80,7 @@ REBASE_LIBS = $(LIBIMAGEHELPER)
REBASE_DUMP_OBJS = rebase-dump.$(O) rebase-db.$(O) $(LIBOBJS)
REBASE_DUMP_LIBS =
-PEFLAGS_OBJS = peflags.$(O) $(LIBOBJS)
+PEFLAGS_OBJS = peflags.$(O) pechecksum.$(O) $(LIBOBJS)
PEFLAGS_LIBS =
SRC_DISTFILES = configure.ac configure Makefile.in \
@@ -111,7 +111,9 @@ rebase-dump.$(O):: rebase-dump.c rebase-db.h Makefile
peflags$(EXEEXT): $(PEFLAGS_OBJS)
$(CC) $(CFLAGS) $(LDFLAGS) -o $@ $(PEFLAGS_OBJS)
-peflags.$(O):: peflags.c Makefile
+peflags.$(O):: peflags.c pechecksum.h Makefile
+
+pechecksum.$(O):: pechecksum.c pechecksum.h Makefile
getopt.h: getopt.h_
cp $^ $@
diff --git a/pechecksum.c b/pechecksum.c
new file mode 100644
index 000..8695138
--- /dev/null
+++ b/pechecksum.c
@@ -0,0 +1,195 @@
+/*
+ * PE32 checksum
+ *
+ * Copyright (C) 2023 Christian Franke
+ *
+ * SPDX-License-Identifier: BSD-3-Clause
+ */
+
+#include "pechecksum.h"
+
+#include
+#include
+#include
+
+static inline unsigned get_word(const unsigned char * p)
+{
+ return p[0] | (p[1] << 8);
+}
+
+static inline unsigned get_dword(const unsigned char * p)
+{
+ return p[0] | (p[1] << 8) | (p[2] << 16) | (p[3] << 24);
+}
+
+static inline void put_dword(unsigned char * p, unsigned val)
+{
+ p[0] = (unsigned char) val;
+ p[1] = (unsigned char)(val >> 8);
+ p[2] = (unsigned char)(val >> 16);
+ p[3] = (unsigned char)(val >> 24);
+}
+
+unsigned pe32_checksum(int fd, int set, unsigned * old_checksum)
+{
+ // Read headers.
+ const unsigned bufsiz = 4096;
+ unsigned char buf[bufsiz];
+ if (lseek(fd, 0, SEEK_SET) != 0)
+return 0;
+ int nr = read(fd, buf, bufsiz);
+ if (nr < 0)
+return 0;
+ if (nr <= 0x200) {
+errno = EINVAL;
+return 0;
+ }
+
+ // IMAGE_DOS_HEADER.pe_magic == "MZ" ?
+ if (get_word(buf) != 0x5a4d) {
+errno = EINVAL;
+return 0;
+ }
+ // pehdr_offs = IMAGE_DOS_HEADER.lfa_new
+ unsigned pehdr_offs = get_dword(buf + 0x3c);
+ if (!(0x40 <= pehdr_offs && pehdr_offs <= nr - 0x100)) {
+errno = EINVAL;
+return 0;
+ }
+ // IMAGE_NT_HEADERS.Signature == "PE" ?
+ if (get_word(buf + pehdr_offs) != 0x4550) {
+errno = EINVAL;
+return 0;
+ }
+ // old_sum = IMAGE_OPTIONAL_HEADER(32|64).CheckSum
+ unsigned sum_offs = pehdr_offs + 0x58;
+ unsigned old_sum = get_dword(buf + sum_offs);
+ if (old_checksum)
+*old_checksum = old_sum;
+
+ // Clear old checksum because it is included below.
+ put_dword(buf + sum_offs, 0);
+
+ // Calc new checksum.
+ unsigned sum = 0, size = 0;
+ int i = 0;
+ for (;;) {
+sum += get_word(buf + i);
+sum = (sum + (sum >> 16)) & 0x;
+
+if ((size += 2) >= 0x4000) {
+ // 1GiB, assume something is wrong.
+ errno = EINVAL;
+ return 0;
+}
+if ((i += 2) < nr - 1)
+ continue; // ... with next 2 bytes.
+
+// Assume that there are no short reads.
+if (i < nr)
+ break; // Last byte.
+i = 0;
+if ((nr = read(fd, buf, bufsiz)) < 0)
+ return 0;
+if (nr < 2)
+ break; // Last byte or EOF.
+// Continue with next block.
+ }
+
+ // Handle last byte of file with uneven size.
+ if (i < nr) {
+sum += buf[i];
+sum = (sum + (sum >> 16)) & 0x;
+size++;
+ }
+
+ // Add filesize to use some of the upper 16 bits.
+ sum += size;
+
+ // Fix the checksum if requested and required.
+ if (set && old_sum != sum) {
+put_dword(buf, sum);
+if (lseek(fd, sum_offs, SEEK_SET) == -1)
+ return 0;
+if (write(fd, buf, 4) == -1)
+ return 0;
+ }
+
+ return sum;
+}
+
+#if STANDALONE
+//
+// Test program
+//
+#include
+#include
+
+// Optionally check result using native imagehlp.dll function.
+#if STANDALONE > 1
+#include
+#include
+#endif
+
+int main(int argc, char ** argv)
+{
+ int i = 1,
[PATCH rebase 1/2] peflags: Add -p, --timestamp option
Running 'peflags -p /bin/*.exe' with the patch applied suggests that
objcopy/strip recently changed behavior and no longer set timestamps to 0.
Related: https://sourceware.org/bugzilla/show_bug.cgi?id=30702
--
Regards,
Christian
From 68d42574e4b7bbc0659708ce801b6cd25b88dc11 Mon Sep 17 00:00:00 2001
From: Christian Franke
Date: Mon, 7 Aug 2023 14:02:12 +0200
Subject: [PATCH] peflags: Add -p, --timestamp option
This allows to set the header timestamp to 0 or some other fixed
value (SOURCE_DATE_EPOCH) to support reproducible builds.
Signed-off-by: Christian Franke
---
peflags.c | 29 +
1 file changed, 25 insertions(+), 4 deletions(-)
diff --git a/peflags.c b/peflags.c
index d98b121..f4b1812 100644
--- a/peflags.c
+++ b/peflags.c
@@ -131,6 +131,7 @@ enum {
SIZEOF_HEAP_RESERVE,
SIZEOF_HEAP_COMMIT,
SIZEOF_CYGWIN_HEAP,
+ SIZEOF_TIMESTAMP,
NUM_SIZEOF_VALUES/* Keep at the end */
};
@@ -152,7 +153,7 @@ typedef struct {
ULONG offset32;
} sizeof_values_t;
-sizeof_values_t sizeof_vals[5] = {
+static sizeof_values_t sizeof_vals[NUM_SIZEOF_VALUES] = {
{ 0, "stack reserve size" , "bytes", 0, FALSE,
offsetof (IMAGE_NT_HEADERS64, OptionalHeader.SizeOfStackReserve),
offsetof (IMAGE_NT_HEADERS32, OptionalHeader.SizeOfStackReserve),
@@ -172,6 +173,10 @@ sizeof_values_t sizeof_vals[5] = {
{ 0, "initial Cygwin heap size", "MB", 0, TRUE,
offsetof (IMAGE_NT_HEADERS64, OptionalHeader.LoaderFlags),
offsetof (IMAGE_NT_HEADERS32, OptionalHeader.LoaderFlags),
+ },
+ { 0, "file header timestamp", "seconds", 0, TRUE,
+offsetof (IMAGE_NT_HEADERS64, FileHeader.TimeDateStamp),
+offsetof (IMAGE_NT_HEADERS32, FileHeader.TimeDateStamp),
}
};
@@ -197,6 +202,7 @@ static struct option long_options[] = {
{"heap-reserve", optional_argument, NULL, 'y'},
{"heap-commit", optional_argument, NULL, 'Y'},
{"cygwin-heap", optional_argument, NULL, 'z'},
+ {"timestamp",optional_argument, NULL, 'p'},
{"filelist", no_argument, NULL, 'T'},
{"verbose", no_argument, NULL, 'v'},
{"help", no_argument, NULL, 'h'},
@@ -204,7 +210,7 @@ static struct option long_options[] = {
{NULL, no_argument, NULL, 0}
};
static const char *short_options
- = "d::e::c::f::n::i::s::b::W::t::w::l::S::x::X::y::Y::z::T:vhV";
+ = "d::e::c::f::n::i::s::b::W::t::w::l::S::x::X::y::Y::z::p::T:vhV";
static void short_usage (FILE *f);
static void help (FILE *f);
@@ -459,13 +465,22 @@ do_mark (const char *pathname)
{
if (sizeof_vals[i].handle != DONT_HANDLE)
{
- printf ("%*s%-24s: %" PRIu64 " (0x%" PRIx64 ") %s\n",
+ char extra[32] = "";
+ if (verbose && i == SIZEOF_TIMESTAMP)
+ {
+ time_t t = (time_t)sizeof_vals[i].value;
+ const struct tm * tp = gmtime ();
+ if (tp)
+ strftime (extra, sizeof(extra), " [%Y-%m-%d %H:%M:%S UTC]",
tp);
+ }
+
+ printf ("%*s%-24s: %" PRIu64 " (0x%" PRIx64 ") %s%s\n",
printed_characteristic ? (int) strlen (pathname) + 2
: 0, "",
sizeof_vals[i].name,
(uint64_t) sizeof_vals[i].value,
(uint64_t) sizeof_vals[i].value,
- sizeof_vals[i].unit);
+ sizeof_vals[i].unit, extra);
printed_characteristic = TRUE;
}
}
@@ -786,6 +801,11 @@ parse_args (int argc, char *argv[])
optarg,
SIZEOF_CYGWIN_HEAP);
break;
+ case 'p':
+ handle_num_option (long_options[option_index].name,
+optarg,
+SIZEOF_TIMESTAMP);
+ break;
case 'T':
file_list = optarg;
break;
@@ -1118,6 +1138,7 @@ help (FILE *f)
" Useful values are between 4 and 2048. If 0,\n"
" Cygwin uses the default heap size of 384
Megs.\n"
" Has no meaning for non-Cygwin applications.\n"
+" -p, --timestamp [NUM] Timestamp in file header (seconds since
epoch).\n"
" -T, --filelist FILE Indicate that FILE contains a list\n"
" of PE files to process\n"
" -v, --verbose Display diagnostic information\n"
--
2.39.0
[PATCH rebase] peflags: Fix ULONG range checks
Minor issue found during tests of the upcoming 'peflags --timestamp' patch.
--
Regards,
Christian
From 9da405da78e92dc8263239e25365bee3167f185e Mon Sep 17 00:00:00 2001
From: Christian Franke
Date: Mon, 7 Aug 2023 13:42:50 +0200
Subject: [PATCH] peflags: Fix ULONG range checks
Don't use ULONG_MAX from because ULONG is not necessarily
'unsigned long'.
Signed-off-by: Christian Franke
---
peflags.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/peflags.c b/peflags.c
index 93eaa0b..d98b121 100644
--- a/peflags.c
+++ b/peflags.c
@@ -30,7 +30,6 @@
#include
#include
#include
-#include
#if defined (__CYGWIN__) || defined (__MSYS__)
#include
#endif
@@ -598,7 +597,7 @@ handle_num_option (const char *option_name,
|| sizeof_vals[option_index].value > 0xULL
/* Just a ULONG value */
|| (sizeof_vals[option_index].is_ulong
- && sizeof_vals[option_index].value > ULONG_MAX))
+ && sizeof_vals[option_index].value > 0xULL))
{
fprintf (stderr, "Invalid argument for %s: %s\n",
option_name, option_arg);
@@ -960,7 +959,7 @@ get_and_set_size (const pe_file *pep, sizeof_values_t *val)
}
else if (val->handle == DO_WRITE)
{
- if ((!pep->is_64bit || val->is_ulong) && val->value >= ULONG_MAX)
+ if ((!pep->is_64bit || val->is_ulong) && val->value >
0xULL)
{
fprintf (stderr, "%s: Skip writing %s, value too big\n",
pep->pathname, val->name);
--
2.39.0
Re: [PATCH setup] Keyboard accelerator for keep or skip
Jon Turney wrote: On 25/04/2023 18:00, Christian Franke via Cygwin-apps wrote: Use case: Easily prevent update of multiple packages in the "Pending" view, in particular useful if "Test" is selected. Sorry for the delay in looking at this. Obviously needed. N.P. Please apply. Done.
Re: [PATCH setup 2/2] Detect filename collisions between packages
Jon Turney wrote: ... Using std::set_intersection() on values from std::map() here is probably a mistake. It's simple to write, but the performance is not good. A faster alternative which avoids set_intersection calls in a loop is possibly to use one large data structure which maps filenames to sets of packages. Using multimap instead of the straightforward map> needs possibly less memory (not tested). But for multimap it is required that file/package name pairs are not inserted twice. I attached a small standalone POC source file using multimap. It would also detect collisions in the already installed packages. Thanks for the ideas. It seems I really didn't think that carefully about this... It seems like maybe building a map from filename to the set of package names which contain it, and then finding all the filenames where that set has more than one member would be a possible better implementation. Of course this is the more obvious method and easier to implement. It is somewhat slower and needs more memory. Meantime I did a quick artificial test with 1 "packages" with 100 "files" each and 2 collisions. This resulted in a (multi)map size if 100(+2), total size of strings was ~80MB. Results: Data structure: execution time, memory (working set) multimap: 1.8 seconds, 238MB map>: 2.0 seconds, 318MB The execution time is needed for the map insertions. The final scan for collisions is very fast in both cases.
[PATCH setup] Keyboard accelerator for keep or skip
Use case: Easily prevent update of multiple packages in the "Pending"
view, in particular useful if "Test" is selected.
--
Regards,
Christian
From 05d9c72f07cf754dc6172a998b2e0991034d363f Mon Sep 17 00:00:00 2001
From: Christian Franke
Date: Tue, 25 Apr 2023 18:42:18 +0200
Subject: [PATCH] Keyboard accelerator for keep or skip
Ctrl+K selects keep or skip and then moves selection to next row.
---
PickPackageLine.cc | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/PickPackageLine.cc b/PickPackageLine.cc
index c1e2a15..d380965 100644
--- a/PickPackageLine.cc
+++ b/PickPackageLine.cc
@@ -155,6 +155,7 @@ PickPackageLine::map_key_to_action(WORD vkey, int modkeys,
int & col_num,
col_num = new_col;
return Action::PopUp;
case 'I': // Ctrl+I: select install default version and move to next row
+case 'K': // Ctrl+K: select keep or skip package and move to next row
case 'R': // Ctrl+R: select reinstall and move to next row
case 'U': // Ctrl+U: select uninstall and move to next row
if (modkeys != ModifierKeys::Control)
@@ -163,8 +164,9 @@ PickPackageLine::map_key_to_action(WORD vkey, int modkeys,
int & col_num,
switch (vkey)
{
case 'I': action_id = packagemeta::Install_action; break;
+default: action_id = packagemeta::NoChange_action; break;
case 'R': action_id = packagemeta::Reinstall_action; break;
-default: action_id = packagemeta::Uninstall_action; break;
+case 'U': action_id = packagemeta::Uninstall_action; break;
}
return Action::Direct | Action::NextRow;
}
--
2.39.0
Re: [PATCH setup 2/2] Detect filename collisions between packages
Jon Turney via Cygwin-apps wrote:
Detect filename collisions between packages
Don't check filenames under /etc/postinstall/ for collisions
Report when filename collisions exist
Add option '--collisions' to enable
IMO a useful enhancement.
Notes:
Reading file catalog from a package is moderately expensive in terms of
I/O: To extract all the filenames from a tar archive, we need to seek to
every file header, and to seek forward through a compressed file, we
must examine every intervening byte to decompress it.
This adds a fourth(!) pass through each archive (one to checksum it, one
to extract files, another one (I added in dbfd1a64 without thinking too
deeply about it) to extract symlinks), and now one to check for filename
collisions).
Using std::set_intersection() on values from std::map() here is probably
a mistake. It's simple to write, but the performance is not good.
A faster alternative which avoids set_intersection calls in a loop is
possibly to use one large data structure which maps filenames to sets of
packages. Using multimap instead of the straightforward
map> needs possibly less memory (not tested). But
for multimap it is required that file/package name pairs are not
inserted twice.
I attached a small standalone POC source file using multimap. It would
also detect collisions in the already installed packages.
...
diff --git a/filemanifest.h b/filemanifest.h
new file mode 100644
index 000..cc6fb81
--- /dev/null
+++ b/filemanifest.h
@@ -0,0 +1,29 @@
+/*
+ * Copyright (c) 2022 Jon Turney
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * A copy of the GNU General Public License can be found at
+ *http://www.gnu.org/
"SPDX-License-Identifier: GPL-2.0-or-later" is possibly a more brief and
modern way to say this in new code.
+ *
+ */
+
+/*
+ FileManifest
+
+ A serializable set of filenames
+ each filename is associated with the package name that owns it
+
+ Used when working with the install package file manifests in /etc/setup
+*/
+
+#include
+#include
This should be as std::unordered_map is not used in the code.
+#include
+
+class FileManifest: public std::map
+{
+};
Is the new file filemanifest.h required at all? It could be reduced to
the following in install.cc:
#include
...
typedef std::map FileManifest;
// or more modern (C++11):
// using FileManifest = std::map;
--
Regards,
Christian
#include
#include
#include
using FileManifest = std::multimap;
int main()
{
FileManifest manifest;
// read from /etc/setup/*.lst.gz
const FileManifest installed {
{"/usr/bin/file11", "package1"}, // #1 package2
{"/usr/bin/file12", "package1"}, // #2 package3
{"/usr/bin/file21", "package2"},
{"/usr/bin/file22", "package2"}, // #3 package3, package4
{"/usr/bin/file11", "package2"} // #1 package1
};
manifest.insert(installed.begin(), installed.end());
// read from tar files to be installed
const FileManifest install {
{"/usr/bin/file31", "package3"},
{"/usr/bin/file12", "package3"}, // #2 package1
{"/usr/bin/file22", "package3"}, // #3 package2, package4
{"/usr/bin/file41", "package4"},
{"/usr/bin/file42", "package4"},
{"/usr/bin/file22", "package4"} // #3 package2, package3
};
manifest.insert(install.begin(), install.end());
for (auto i = manifest.cbegin(), end = manifest.cend(); i != end; ) {
auto j = i;
if (++j != end && j->first == i->first) {
std::printf("Packages");
j = i;
do
std::printf(" %s", j->second.c_str());
while (++j != end && j->first == i->first);
std::printf(" contain file %s\n", i->first.c_str());
}
//else
//std::printf("Package %s contains file %s\n",
//i->second.c_str(), i->first.c_str());
i = j;
}
return 0;
}
Re: [PATCH setup] Add tooltip for Ctrl+I/R/U accelerator keys
Jon Turney wrote: On 22/11/2022 16:00, Christian Franke wrote: This is a first try to make these keys more obvious as requested on the Cygwin ML. A more complex approach would be to mention only the keys which actually would change the current package state. Thanks for this patch. So, I appreciate what this is trying to do, but I have great difficultly convincing myself that this is the right approach. I'm also not sure. An alternative would possibly be some help below the listbox, centered in the same line as "Hide obsolete packages". We don't need to resort to a tooltip for any other shortcut, because there is an established visual design language to indicate those. I guess we just need to find what's consistent with pre-existing ways of indicating this kind of behaviour in other applications. Cygwin setup is somewhat uncommon as there is no default menu bar with drop-down menus. Accelerator keys are traditionally mentioned there.
Re: [PATCH setup] Ignore reinstall requests if version is not accessible
Jon Turney wrote: On 11/12/2022 15:11, Christian Franke via Cygwin-apps wrote: This prevents accidental package loss if reinstall is run from local directory but the package is no longer cached. Thanks. This seems right, so please apply. Done. Could you please also review "[PATCH setup] Add tooltip for Ctrl+I/R/U accelerator keys" before next release? Possible further improvement: Do not offer "Reinstall" in the popup menu in this case. Indeed. That might be simply fixable, but really, we need to keep an index of the files in the package cache, so we can handle situations like this properly, without spending lots of time spinning rust Agree. "Install from Local Directory" is in particular slow on HDDs when directory tree is fragmented or even slower if the cache is on a network share. (we could then also do some expiry on the cache, which is a feature that has been needed for decades...) For example: Add checkbox(es) to LocalDirPage to enable/configure cache cleanup. In a later step remove all files that could no longer be downloaded (like Debian's "apt-get autoclean") and are no longer installed. The latter would need some heuristics for source packages as these do not appear in installed.db. BTW, I have a local bash-script 'cygcache' which allows to list package states ([auto]installed, cached, curr/prev/test), cleanup the cache using above rules (configurable) or merge cache directories into one. Still requires some polishing. If there is any interest, I could ITP it next year.
[PATCH setup] Ignore reinstall requests if version is not accessible
This prevents accidental package loss if reinstall is run from local
directory but the package is no longer cached.
Possible further improvement: Do not offer "Reinstall" in the popup menu
in this case.
--
Regards,
Christian
From 054697c297a3e21ee5e63f51b27431640cab5cde Mon Sep 17 00:00:00 2001
From: Christian Franke
Date: Sun, 11 Dec 2022 15:55:47 +0100
Subject: [PATCH] Ignore reinstall requests if version is not accessible
---
package_meta.cc | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/package_meta.cc b/package_meta.cc
index 2257b59..3daa970 100644
--- a/package_meta.cc
+++ b/package_meta.cc
@@ -672,7 +672,7 @@ packagemeta::set_action (_actions action, packageversion
const _version,
else if (action == Reinstall_action)
{
desired = installed;
- if (desired)
+ if (desired.accessible ())
{
pick (true);
srcpick (false);
--
2.38.1
Re: [Bug] setup regression #2
Jon Turney wrote:
On 20/11/2022 19:05, Achim Gratz wrote:
Jon Turney writes:
I believe that the intent of the code in setup is that there should
only be two modes:
USER: install "for me", with the users primary group
As I understand it, the intention here was that the user can have a
"single user installation" in a place that they have access to (say,
their home directory) while they have no permission in one of the usual
places. In a setup where that place is a certain type of share the user
will not be able to change the group the files are owned by anyway
(standard NetApp CIFS shares are set up this way) and it may not be the
users primary group.
SYSTEM: install "for everyone", with the administrators primary group
(only permitted if you are an administrator)
I don't see why the fact the installation is meant to be used by
multiple users means that the install must be owned by group
Administrators. I'm not sure this is a good idea on Windows anyway, at
least when you don't put extra (inheritable) DACL on the install
folder.
Christian,
Maybe you can offer your opinion here, since you seem to have the
opposite, or at least a different, point of view.
Anything installed with "All Users" option should IMO be protected
against modifications by any regular non-elevated user.
This is not the case if the RID=513 group ("HOST\None",
"DOMAIN\Domain-Users") is used. Many upstream projects install
directories and files with permissions like 0664, 0775, 0660 or 0770.
This is safe when the group is "root". On current Cygwin, all users have
R/W access regardless of the "other" permission bits.
Try for example: find /usr/share ! -type l -perm -020 -ls (~4000 hits on
my installation)
Using the administrators group as discussed here would solve this but
apparently introduces interesting new permission problems with some
packages. Could these possibly be solved by the maintainers of the
affected packages?
An alternative may be: Keep the group as is, but prevent using above
permissions as far as possible. For new packages, this could possibly be
done with an enhancement of cygport. But I'm sure that there will also
be subtle cases where these modified permissions break things. Cygport
could allow to opt-out then.
Ideally the protection should also be effective for the non-elevated
user who runs setup elevated. This is automatically the case for typical
installers because Windows changes TokenOwner to administrators group if
run elevated. That's why I provided
https://sourceware.org/pipermail/cygwin-apps/2022-August/042221.html
Re: [PATCH setup] Add new option --chown-admin
Jon Turney wrote: On 04/10/2022 13:05, Christian Franke wrote: Jon Turney wrote: Corinna had some concerns about making the owner a group, rather than a user, which I believe historically caused some difficulties in Cygwin, so I think I'll need to understand that better before making a decision about this change. I see. Do you have any info about these difficulties? Are these still relevant? If yes, let's forget this patch. After a bit of research, I think the issue was that if you make user owner and group owner map onto the same Windows SID, certain unix access permissions cannot be reversibly mapped onto a Windows ACL. (e.g you can't set the mode to 0600, because when you read that back, it's mode is 0660. Some programs e.g ssh check for and require 0600 permission on some files) No and yes. No, a quick test shows that stat() returns what chmod() sets even in this case: # for p in 600 640 660 644 664; do f=perm-$p && touch $f && chown Administrators.Administrators $f && chmod $p $f && ls -l $f done -rw--- 1 Administrators Administrators 0 Nov 30 18:39 perm-600 -rw-r- 1 Administrators Administrators 0 Nov 30 18:39 perm-640 -rw-rw 1 Administrators Administrators 0 Nov 30 18:39 perm-660 -rw-r--r-- 1 Administrators Administrators 0 Nov 30 18:39 perm-644 -rw-rw-r-- 1 Administrators Administrators 0 Nov 30 18:39 perm-664 The above likely works due to some heuristic based on ACE order. Yes, the effective permissions of 0600 are always the same as 0660 because the first ACE is already for the group: # icacls perm-\* perm-600 BUILTIN\Administrators:(R,W,D,WDAC,WO) BUILTIN\Administrators:(Rc,S,RA) Everyone:(Rc,S,RA) perm-640 BUILTIN\Administrators:(R,W,D,WDAC,WO) BUILTIN\Administrators:(R) Everyone:(Rc,S,RA) perm-644 BUILTIN\Administrators:(R,W,D,WDAC,WO) BUILTIN\Administrators:(R) Everyone:(R) perm-660 BUILTIN\Administrators:(R,W,D,WDAC,WO) BUILTIN\Administrators:(R,W) Everyone:(Rc,S,RA) perm-664 BUILTIN\Administrators:(R,W,D,WDAC,WO) BUILTIN\Administrators:(R,W) Everyone:(R) (Tests done on German Windows and localized names renamed afterwards). This perhaps isn't terribly relevant to files created by setup It may depend on how access checks are done by ssh etc.. (mode bits or effective permissions).
[PATCH setup] Add tooltip for Ctrl+I/R/U accelerator keys
This is a first try to make these keys more obvious as requested on the
Cygwin ML.
A more complex approach would be to mention only the keys which actually
would change the current package state.
--
Regards,
Christian
From 6bfb1f07c772fd6298c63b3b2ae53000e26a6237 Mon Sep 17 00:00:00 2001
From: Christian Franke
Date: Tue, 22 Nov 2022 16:38:39 +0100
Subject: [PATCH] Add tooltip for Ctrl+I/R/U accelerator keys
Tooltip is shown for "New" column of selected row.
---
ListView.cc | 3 ++-
ListView.h | 2 +-
PickCategoryLine.cc | 2 +-
PickCategoryLine.h | 2 +-
PickPackageLine.cc | 9 +++--
PickPackageLine.h | 2 +-
res/en/res.rc | 1 +
res/fr/res.rc | 1 +
resource.h | 1 +
9 files changed, 16 insertions(+), 7 deletions(-)
diff --git a/ListView.cc b/ListView.cc
index 7cc7f0f..40addaa 100644
--- a/ListView.cc
+++ b/ListView.cc
@@ -643,7 +643,8 @@ ListView::OnNotify (NMHDR *pNmHdr, LRESULT *pResult)
static StringCache tooltip;
tooltip = "";
if (contents)
-tooltip = (*contents)[iRow]->get_tooltip(iCol);
+tooltip = (*contents)[iRow]->get_tooltip(iCol, GetFocus() ==
hWndListView
+ && ListView_GetSelectionMark(hWndListView) == iRow);
// set the tooltip text
NMTTDISPINFO *pNmTTDispInfo = (NMTTDISPINFO *)pNmHdr;
diff --git a/ListView.h b/ListView.h
index 6a1be0b..2703d1f 100644
--- a/ListView.h
+++ b/ListView.h
@@ -40,7 +40,7 @@ class ListViewLine
virtual ~ListViewLine() {};
virtual const std::wstring get_text(int col) const = 0;
virtual State get_state() const = 0;
- virtual const std::string get_tooltip(int col) const = 0;
+ virtual const std::string get_tooltip(int col, bool selected) const = 0;
virtual int get_indent() const = 0;
virtual ActionList *get_actions(int col) const = 0;
virtual int do_action(int col, int id) = 0;
diff --git a/PickCategoryLine.cc b/PickCategoryLine.cc
index b13dbe4..d9f9535 100644
--- a/PickCategoryLine.cc
+++ b/PickCategoryLine.cc
@@ -91,7 +91,7 @@ PickCategoryLine::get_indent() const
}
const std::string
-PickCategoryLine::get_tooltip(int col_num) const
+PickCategoryLine::get_tooltip(int col_num, bool selected) const
{
return "";
}
diff --git a/PickCategoryLine.h b/PickCategoryLine.h
index 7616b15..8352c36 100644
--- a/PickCategoryLine.h
+++ b/PickCategoryLine.h
@@ -36,7 +36,7 @@ public:
const std::wstring get_text(int col) const;
State get_state() const;
- const std::string get_tooltip(int col) const;
+ const std::string get_tooltip(int col, bool selected) const;
int get_indent() const;
ActionList *get_actions(int col) const;
int do_action(int col, int action_id);
diff --git a/PickPackageLine.cc b/PickPackageLine.cc
index c1e2a15..fd033e2 100644
--- a/PickPackageLine.cc
+++ b/PickPackageLine.cc
@@ -16,6 +16,7 @@
#include "PickPackageLine.h"
#include "PickView.h"
#include "package_db.h"
+#include "resource.h"
const std::wstring
PickPackageLine::get_text(int col_num) const
@@ -86,9 +87,13 @@ PickPackageLine::get_text(int col_num) const
}
const std::string
-PickPackageLine::get_tooltip(int col_num) const
+PickPackageLine::get_tooltip(int col_num, bool selected) const
{
- if (col_num == pkg_col)
+ if (col_num == new_col && selected)
+{
+ return wstring_to_string(LoadStringW(IDS_CTRL_IRU_TOOLTIP));
+}
+ else if (col_num == pkg_col)
{
return pkg.LDesc();
}
diff --git a/PickPackageLine.h b/PickPackageLine.h
index 0bf4ae6..f882ac8 100644
--- a/PickPackageLine.h
+++ b/PickPackageLine.h
@@ -32,7 +32,7 @@ public:
};
const std::wstring get_text(int col) const;
State get_state() const { return State::nothing; }
- const std::string get_tooltip(int col) const;
+ const std::string get_tooltip(int col, bool selected) const;
int get_indent() const;
ActionList *get_actions(int col_num) const;
int do_action(int col, int action_id);
diff --git a/res/en/res.rc b/res/en/res.rc
index ef5e8b1..9b3288d 100644
--- a/res/en/res.rc
+++ b/res/en/res.rc
@@ -653,6 +653,7 @@ BEGIN
IDS_CONFIRM_SOURCE "(source)"
IDS_FILE_INUSE_KILL " Processes"
IDS_FILE_INUSE_MSG "Unable to extract"
+IDS_CTRL_IRU_TOOLTIP "Ctrl+I: Install, Ctrl+R: Reinstall, Ctrl+U:
Uninstall"
END
//
diff --git a/res/fr/res.rc b/res/fr/res.rc
index 747e1dd..b6d2f6d 100644
--- a/res/fr/res.rc
+++ b/res/fr/res.rc
@@ -633,6 +633,7 @@ BEGIN
IDS_CONFIRM_SOURCE "(source)"
IDS_FILE_INUSE_KILL " les processus"
IDS_FILE_INUSE_MSG "Incapable d'extraire"
+// IDS_CTRL_IRU_TOOLTIP "XXX: missing translation"
END
//
diff --git a/resource.h b/resource.h
index cfe860b..4b636d2 100644
--- a/resource.h
+++ b/resource.h
@@ -107,6 +107,7 @@
#define IDS_DEPRECATED_WINDOWS_ARCH 1210
#define IDS_VIEW_REMOVABL
Re: [PATCH setup] Add new option --chown-admin
Jon Turney wrote: On 02/09/2022 16:17, Christian Franke wrote: Jon Turney wrote: On 28/08/2022 18:33, Christian Franke wrote: As the 'root_scope' issues are now fixed, here a reworked and enhanced (checkbox, setup.rc entry) version of the original patch from this thread. With the new setting enabled, setup behaves like other install tools when run elevated: The installation is then also protected against accidental modifications by the current user. owner:group assignments of newly installed dirs/files: adm:adm -- "All Users", "[X] Change owner of newly installed files to local Administrator" usr:adm -- "All Users" usr:def -- "Just Me" (usr = user running setup, adm = S-1-5-32-544, def = S-1-5-21-*-513) Thanks. When writing the change summary for the last RC, I wondered what the file owner should be. I guess my question is, if adm:adm ownership is correct, and expected for consistency with other Windows installers, why not make that the default? and then do we really need to provide the current behaviour as an option, if it's "wrong". Two good questions. I'm not sure. Well, perhaps we can explore that by asking what is the motivation for this change? Does the current situation cause you a problem? Is is it just motivated by the concern that the user running setup could accidentally modify the installation, or something else? If "All Users" is selected, the installation should IMO be protected against the same user if not elevated. This is automatically the case for other installers because Windows sets TokenOwner=Administrator if elevated. Corinna had some concerns about making the owner a group, rather than a user, which I believe historically caused some difficulties in Cygwin, so I think I'll need to understand that better before making a decision about this change. I see. Do you have any info about these difficulties? Are these still relevant? If yes, let's forget this patch.
Re: [Bug] setup regression
Jon Turney wrote:
On 27/09/2022 14:51, Christian Franke wrote:
Christian Franke wrote:
...
I made the false assumption that default_version=empty in
set_action() always implies that the default version is not
accessible. This is not the case for packages selected for
installation before chooser is visible.
I'm working on a new fix for the "Ctrl+I pressed but current version
is not accessible" case. ...
See attached patch. It also fixes the same problem for the "Category"
view. Testing shows that the problem only affects the display of the
version number as the solver later silently removes such install
requests.
The correct logic is already in toggle_action(): Install the most
recent accessible non-test ('naively_preferred') version.
I dropped this idea and aligned Ctrl+I behavior with "Install" from
"Category" view instead. Toggle_action() behaves different in such
corner cases as it always installs something.
Thanks, I applied both patches.
Do you have a simple way of causing a non-accessible package for
testing with?
Unfortunately there is only a simple one if the current installation and
cache directory matches some conditions. The "Ignore install requests if
version is not accessible" case appears if:
- "Install from Local Directory" is selected.
- A previous version of a no longer installed package is still in the
cache directory. Otherwise the package would not appear in the chooser.
- The current version of this package is not in the cache directory. The
"Size" column shows "?" for such packages.
Without commit 63a2c90, Ctrl+I or "Install" from "Category" view shows
an empty version in "New" column. The "Install" from "Category" case
could also be reproduced with setup 2.919.
With this commit, the Install request does nothing.
Re: [Bug] setup regression
Christian Franke wrote:
...
I made the false assumption that default_version=empty in set_action()
always implies that the default version is not accessible. This is not
the case for packages selected for installation before chooser is
visible.
I'm working on a new fix for the "Ctrl+I pressed but current version
is not accessible" case. ...
See attached patch. It also fixes the same problem for the "Category"
view. Testing shows that the problem only affects the display of the
version number as the solver later silently removes such install requests.
The correct logic is already in toggle_action(): Install the most
recent accessible non-test ('naively_preferred') version.
I dropped this idea and aligned Ctrl+I behavior with "Install" from
"Category" view instead. Toggle_action() behaves different in such
corner cases as it always installs something.
From 7e1efd346e35898e3486fb84cf25b3885a2a16ec Mon Sep 17 00:00:00 2001
From: Christian Franke
Date: Tue, 27 Sep 2022 15:41:06 +0200
Subject: [PATCH] Ignore install requests if version is not accessible
This avoids that an empty "New" version is shown when install is
requested via "Install" from "Category" view or Ctrl+I and the
version is not accessible.
---
package_meta.cc | 12
1 file changed, 12 insertions(+)
diff --git a/package_meta.cc b/package_meta.cc
index 05b8946..ebfc947 100644
--- a/package_meta.cc
+++ b/package_meta.cc
@@ -539,6 +539,17 @@ packagemeta::select_action (int id, trusts const deftrust)
{
if (id == packagemeta::NoChange_action)
set_action((packagemeta::_actions)id, installed);
+ else if (id == Install_action)
+ {
+ // Ignore install request if the default version is not accessible.
+ // This assumes that all available versions are already known.
+ // This is not always the case when set_action is called directly.
+ packageversion v = trustp (true, deftrust);
+ if (v.accessible ())
+ set_action(Install_action, v, true);
+ else
+ set_action(NoChange_action, installed);
+ }
else
set_action((packagemeta::_actions)id, trustp (true, deftrust), true);
}
@@ -627,6 +638,7 @@ packagemeta::set_action (_actions action, packageversion
const _version,
else if (action == Install_action)
{
desired = default_version;
+ // If desired is empty, it will be set to the solver's preferred version
later.
if (desired)
{
if (desired != installed)
--
2.37.2
Re: [Bug] setup regression
Jon Turney wrote:
On 22/09/2022 17:56, Achim Gratz wrote:
Achim Gratz writes:
Achim Gratz writes:
I had updated setup to 2.921 recently, so I rolled it back to 2.920
and
this version does the package selection correctly. I haven't yet
looked
what commit is responsible, but whatever the cause of the
regression is
still in 2.922 as well.
The most likely change responsible for this is the additions in
package_meta.cc in commit c99e4c14911181636892355a4f1855024051ea1d. I
might not be able to check this tomorrow, though I'll try to free up
some time for that.
That was indeed the culprit. I've reverted just these two hunks on top
of release_2.922 and things worked again.
Yes, looking again at that change, the first hunk in package_meta.cc,
changing Install_action doesn't look right.
Indeed and this should be removed ASAP - patch attached.
Thanks for catching and sorry for the regression.
If I remember correctly action=Install_action, desired=empty package
version (evaluating as a boolean is false) means "install the solver's
preferred version", so converting that to NoChange_action seems wrong.
I'm kind of confused how to reproduce this, or why it decided to
install only some things, rather than nothing.
Christian,
From your reply to
https://cygwin.com/pipermail/cygwin-apps/2022-August/042212.html, it
seems this change is meant to handle the case where 'I' is pressed but
the package isn't accessible? Although I don't seem quite how.
I made the false assumption that default_version=empty in set_action()
always implies that the default version is not accessible. This is not
the case for packages selected for installation before chooser is visible.
I'm working on a new fix for the "Ctrl+I pressed but current version is
not accessible" case. The correct logic is already in toggle_action():
Install the most recent accessible non-test ('naively_preferred') version.
From 54665f0596f8ca50eff99ec8ec35970dc5fd439d Mon Sep 17 00:00:00 2001
From: Christian Franke
Date: Mon, 26 Sep 2022 16:47:42 +0200
Subject: [PATCH] Fix ignored install requests added before run of solver
Regression was introduced by commit c99e4c1.
---
package_meta.cc | 7 ---
1 file changed, 7 deletions(-)
diff --git a/package_meta.cc b/package_meta.cc
index a5dc436..05b8946 100644
--- a/package_meta.cc
+++ b/package_meta.cc
@@ -651,13 +651,6 @@ packagemeta::set_action (_actions action, packageversion
const _version,
srcpick (false);
}
}
- else
- {
- action = NoChange_action;
- desired = installed;
- pick (false);
- srcpick (false);
- }
}
else if (action == Reinstall_action)
{
--
2.37.2
Re: [PATCH setup] Add new option --chown-admin
Jon Turney wrote:
On 28/08/2022 18:33, Christian Franke wrote:
As the 'root_scope' issues are now fixed, here a reworked and
enhanced (checkbox, setup.rc entry) version of the original patch
from this thread.
With the new setting enabled, setup behaves like other install tools
when run elevated: The installation is then also protected against
accidental modifications by the current user.
owner:group assignments of newly installed dirs/files:
adm:adm -- "All Users", "[X] Change owner of newly installed files to
local Administrator"
usr:adm -- "All Users"
usr:def -- "Just Me"
(usr = user running setup, adm = S-1-5-32-544, def = S-1-5-21-*-513)
Thanks. When writing the change summary for the last RC, I wondered
what the file owner should be.
I guess my question is, if adm:adm ownership is correct, and expected
for consistency with other Windows installers, why not make that the
default? and then do we really need to provide the current behaviour
as an option, if it's "wrong".
Two good questions. I'm not sure.
An alternative for the UI would be a 3rd radio button ("All Users -
change owner of newly installed files to local Administrator"), but
the checkbox makes this addition IMO more obvious.
The new setup.rc setting 'root-scope' is only used to read the
chown_admin setting but this could be enhanced, e.g. warn user if
root_scope selection differs from previous setup run.
The drawback that files generated by postinstall scripts are still
owned by current user could be fixed with a perpetual postinstall
script. I could provide one for base-files package if desired.
Doesn't this mean that we are using the wrong user-context to run
those scripts?
The correct user context for running the script would be an equivalent
to 'sudo administrator' which is not possible.
A change or addition (environment CYGWIN=chown_admin) in the Cygwin DLL
would help: If launched with TokenOwner = Administrator, make sure that
all newly created dirs/files are owned by TokenOwner instead of current
user.
Re: [PATCH setup] Add new option --chown-admin
As the 'root_scope' issues are now fixed, here a reworked and enhanced
(checkbox, setup.rc entry) version of the original patch from this thread.
With the new setting enabled, setup behaves like other install tools
when run elevated: The installation is then also protected against
accidental modifications by the current user.
owner:group assignments of newly installed dirs/files:
adm:adm -- "All Users", "[X] Change owner of newly installed files to
local Administrator"
usr:adm -- "All Users"
usr:def -- "Just Me"
(usr = user running setup, adm = S-1-5-32-544, def = S-1-5-21-*-513)
An alternative for the UI would be a 3rd radio button ("All Users -
change owner of newly installed files to local Administrator"), but the
checkbox makes this addition IMO more obvious.
The new setup.rc setting 'root-scope' is only used to read the
chown_admin setting but this could be enhanced, e.g. warn user if
root_scope selection differs from previous setup run.
The drawback that files generated by postinstall scripts are still owned
by current user could be fixed with a perpetual postinstall script. I
could provide one for base-files package if desired.
--
Regards,
Christian
From 6d996b377b4a6a908fbb4c217bda24249b4b58c1 Mon Sep 17 00:00:00 2001
From: Christian Franke
Date: Sun, 28 Aug 2022 19:23:44 +0200
Subject: [PATCH] Optionally change owner of installed files to local
administrator
Could be selected via new checkbox in Root dialog or via new option
--chown-admin. This choice and the root_scope are stored in new
setup.rc entry 'root-scope'.
---
res/en/res.rc | 6 +-
res/fr/res.rc | 6 +-
resource.h| 2 ++
root.cc | 45 -
win32.cc | 33 -
win32.h | 2 ++
6 files changed, 78 insertions(+), 16 deletions(-)
diff --git a/res/en/res.rc b/res/en/res.rc
index ef5e8b1..74d3d0c 100644
--- a/res/en/res.rc
+++ b/res/en/res.rc
@@ -108,7 +108,10 @@ BEGIN
CONTROL "Just ",IDC_ROOT_USER,"Button",BS_AUTORADIOBUTTON |
WS_TABSTOP,13,130,130,8
LTEXT "Cygwin will be available to all users of the system.",
-IDC_ALLUSERS_TEXT,25,101,300,28
+IDC_ALLUSERS_TEXT,25,100,300,28
+CONTROL " owner of newly installed files to local
Administrator",
+IDC_ROOT_CHOWN_ADMIN,"Button",BS_AUTOCHECKBOX | WS_TABSTOP,
+25,111,300,16
LTEXT "Cygwin will still be available to all users, but "
"Desktop Icons, Cygwin Menu Entries, and important "
"Installer information are only available to the current "
@@ -668,6 +671,7 @@ BEGIN
IDS_HELPTEXT_ALLOW_UNSUPPORTED_WINDOWS "Allow old, unsupported Windows
versions"
IDS_HELPTEXT_ARCH "Architecture to install (x86_64 or x86)"
IDS_HELPTEXT_CATEGORIES "Specify categories to install"
+IDS_HELPTEXT_CHOWN_ADMIN "Change owner of newly installed files to local
Administrator"
IDS_HELPTEXT_COMPACTOS "Compress installed files with Compact OS
(xpress4k, xpress8k, xpress16k, lzx)"
IDS_HELPTEXT_DELETE_ORPHANS "Remove orphaned packages"
IDS_HELPTEXT_DISABLE_ANTIVIRUS "Disable known or suspected buggy anti
virus software packages during execution"
diff --git a/res/fr/res.rc b/res/fr/res.rc
index d081bb2..21ba8f9 100644
--- a/res/fr/res.rc
+++ b/res/fr/res.rc
@@ -102,7 +102,10 @@ BEGIN
CONTROL "Juste ",IDC_ROOT_USER,"Button",BS_AUTORADIOBUTTON |
WS_TABSTOP,13,130,130,8
LTEXT "Cygwin sera disponible pour tous les utilisateurs.",
-IDC_ALLUSERS_TEXT,25,101,300,28
+IDC_ALLUSERS_TEXT,25,100,300,28
+CONTROL " owner of newly installed files to local
Administrator", // XXX: missing translation
+IDC_ROOT_CHOWN_ADMIN,"Button",BS_AUTOCHECKBOX | WS_TABSTOP,
+25,111,300,16
LTEXT "Cygwin sera disponible pour tous les utilisateurs "
"mais les icônes et les menus uniquement pour
l'utilisateur "
"en cours. Ne sélectionner que si vous n'avez pas les
droits "
@@ -648,6 +651,7 @@ BEGIN
IDS_HELPTEXT_ALLOW_UNSUPPORTED_WINDOWS "Autoriser les vieilles versions de
Windows"
IDS_HELPTEXT_ARCH "Architecture à installer (x86_64 ou x86)"
IDS_HELPTEXT_CATEGORIES "Spécifie les catégories à installer"
+// IDS_HELPTEXT_CHOWN_ADMIN "XXX: missing translation"
// IDS_HELPTEXT_COMPACTOS "XXX: missing translation"
IDS_HELPTEXT_DELETE_ORPHANS "Su
Re: [PATCH setup] Add new option --chown-admin
Jon Turney wrote:
On 23/08/2022 18:27, Christian Franke wrote:
Jon Turney wrote:
On 12/07/2022 13:50, Jon Turney wrote:
[Replying to the right list this time...]
On 09/07/2022 13:21, Christian Franke wrote:
[...]
The UserSettings ctor has a somewhat hidden side effect which sets
root_scope correctly:
UserSettings::UserSettings(...);
open_settings("setup.rc", ...);
io_stream::open("cygfile:///etc/setup/setup.rc", ...);
io_stream_cygfile::io_stream_cygfile("/etc/setup/setup.rc", ...);
get_root_dir_now();
read_mounts("");
read_mounts_nt("");
root_scope = isuser ? IDC_ROOT_USER : IDC_ROOT_SYSTEM;
Conclusion: Regression introduced Feb 24, 2012 (befc9dd).
Thanks for tracking this down.
That just seems... fractally wrong.
I kind of lost track of this. Is there anything else needed to fix
the original problem here? Or is it solved by the change to defer
setting the group until after root_scope is known?
The group seems to be correctly set now.
An old problem still remains: root_scope always ends up as
IDC_ROOT_SYSTEM if setup is run elevated, regardless off GUI setting.
Apply the temporary patch from here to see what happens:
https://sourceware.org/pipermail/cygwin-apps/2022-July/042151.html
Ah, right, I remember now.
I think having read_mounts() have a side effect of setting root_scope
is just nonsense now (it might have made some sense back in the day
when the mount table was also stored in the registry).
So, how about the attached?
(We should perhaps also set the installation root directory to
something other than C:\cygwin64 if a non-admin user, since they are
unlikely to be able to write there in current windows versions, but
that's difficult from a sequencing point of view in that dialog, and
for backwards compatibility)
Possibly difficult to fix, in particular in conjunction with later
changes via [< Back] button. An easier approach: Remove the GUI
setting and connect root_scope to -B option.
Looking for that option is not correct, because we might be running
without -B, but from an already elevated shell. I think checking
nt_sec.isRunAsAdmin() would be the correct test.
Did some quick tests with patch applied. LGTM.
Re: [PATCH setup] Add new option --chown-admin
Jon Turney wrote:
On 12/07/2022 13:50, Jon Turney wrote:
[Replying to the right list this time...]
On 09/07/2022 13:21, Christian Franke wrote:
[...]
The UserSettings ctor has a somewhat hidden side effect which sets
root_scope correctly:
UserSettings::UserSettings(...);
open_settings("setup.rc", ...);
io_stream::open("cygfile:///etc/setup/setup.rc", ...);
io_stream_cygfile::io_stream_cygfile("/etc/setup/setup.rc", ...);
get_root_dir_now();
read_mounts("");
read_mounts_nt("");
root_scope = isuser ? IDC_ROOT_USER : IDC_ROOT_SYSTEM;
Conclusion: Regression introduced Feb 24, 2012 (befc9dd).
Thanks for tracking this down.
That just seems... fractally wrong.
I kind of lost track of this. Is there anything else needed to fix
the original problem here? Or is it solved by the change to defer
setting the group until after root_scope is known?
The group seems to be correctly set now.
An old problem still remains: root_scope always ends up as
IDC_ROOT_SYSTEM if setup is run elevated, regardless off GUI setting.
Apply the temporary patch from here to see what happens:
https://sourceware.org/pipermail/cygwin-apps/2022-July/042151.html
Possibly difficult to fix, in particular in conjunction with later
changes via [< Back] button. An easier approach: Remove the GUI setting
and connect root_scope to -B option.
Re: [PATCH setup] Keyboard accelerators for install/reinstall/uninstall
Jon Turney wrote:
On 22/08/2022 16:29, Christian Franke wrote:
Jon Turney wrote:
On 14/08/2022 12:57, Christian Franke wrote:
This eases state changes of a selected sequence of packages.
Nice! The keyboard control of the package chooser was a bit of an
after-thought, which it really shouldn't be.
Thanks - revised patch is attached.
Thanks. This looks good. Please apply.
Thanks. Applied.
Ctrl+U is in particular useful to cleanup installations in
conjunction with "unneeded" view:
https://sourceware.org/pipermail/cygwin-apps/2022-August/042185.html
Open issue: Add some visual clue (tooltip?) to make this
functionality more obvious.
Yeah. These shortcuts should also be accelerators for the package
action selection popup menu, which would make them more discoverable?
Handling these in the popup menu is possibly tricky. According to
documentation of TrackPopupMenu(), hWndListView "receives all
messages from the menu" which is apparently not the case.
Confused. I don't think that matters (we could probably be using
TPM_NONOTIFY), because we use TPM_RETURNCMD.
If we mark accelerators in the menu:
--- a/res/en/res.rc
+++ b/res/en/res.rc
@@ -573,11 +573,11 @@ BEGIN
IDS_PROGRESS_POSTINSTALL "Running..."
IDS_PROGRESS_SOLVING "Solving dependencies..."
IDS_ACTION_DEFAULT "Default"
- IDS_ACTION_INSTALL "Install"
- IDS_ACTION_UNINSTALL "Uninstall"
+ IDS_ACTION_INSTALL ""
+ IDS_ACTION_UNINSTALL ""
IDS_ACTION_SKIP "Skip"
IDS_ACTION_KEEP "Keep"
- IDS_ACTION_REINSTALL "Reinstall"
+ IDS_ACTION_REINSTALL ""
IDS_ACTION_RETRIEVE "Retrieve"
IDS_ACTION_UNKNOWN "Unknown"
IDS_ACTION_SOURCE "Source"
They appear when the menu is opened by pressing the menu key (or
always, if "Underline access keys when available" is on in
ease-of-access settings), and menu items can be chosen using them.
It's not quite that straightforward because we need to remove the '&'
when those strings are used elsewhere (e.g. in the action column), but
I think it can be done...
Using plain letters (without Ctrl) or first digit of version number
already work in the popup menu. But it is possibly tricky to also
interpret Ctrl+I/R/U in the popup menu.
@@ -670,6 +670,10 @@ packagemeta::set_action (_actions action,
packageversion const _version,
else if (action == Uninstall_action)
{
desired = packageversion ();
+ pick (false);
+ srcpick (false);
+ if (!installed)
+ action = NoChange_action;
Hmm... why is adding this needed?
Otherwise a strange state change would occur at least in the GUI when
an install request is undone:
"Skip" == Ctrl+I ==> "3.2-1" == Ctrl+U ==> "Uninstall"
The new patch includes another addition which prevents this on
installs from local directory when the current default version is not
yet downloaded:
"Skip" == Ctrl+I ==> "" (empty)
I see. But these work correctly when chosen via the action menu dropdown?
These state changes are not offered by the popup menu. For example for
not installed packages the popup menu does not contain a selection which
returns Uninstall_action. Undoing an install request is done via
re-selection of "Skip" which returns NoChange_action.
The design alternatives were either to emulate this in
PickPackageLine::map_key_to_action() or to complete the (hidden) state
machine in packagemeta::set_action(). I decided to use the latter.
Re: [PATCH setup] Keyboard accelerators for install/reinstall/uninstall
Jon Turney wrote:
On 14/08/2022 12:57, Christian Franke wrote:
This eases state changes of a selected sequence of packages.
Nice! The keyboard control of the package chooser was a bit of an
after-thought, which it really shouldn't be.
Thanks - revised patch is attached.
Ctrl+U is in particular useful to cleanup installations in
conjunction with "unneeded" view:
https://sourceware.org/pipermail/cygwin-apps/2022-August/042185.html
Open issue: Add some visual clue (tooltip?) to make this
functionality more obvious.
Yeah. These shortcuts should also be accelerators for the package
action selection popup menu, which would make them more discoverable?
Handling these in the popup menu is possibly tricky. According to
documentation of TrackPopupMenu(), hWndListView "receives all messages
from the menu" which is apparently not the case.
...
+ bool down = false;
+ if
((*contents)[iRow]->map_key_to_action(pNmLvKeyDown->wVKey, ctrl, alt,
_num,
+ _id, ))
{
int update;
if (action_id >= 0)
@@ -591,6 +597,13 @@ ListView::OnNotify (NMHDR *pNmHdr, LRESULT
*pResult)
if (update > 0)
ListView_RedrawItems(hWndListView, iRow, iRow +
update -1);
}
+ if (down && iRow + 1 < ListView_GetItemCount(hWndListView)) {
Again as a stylistic thing, I'd suggest perhaps changing
map_key_to_action() to return a set of flags which could include
"ACTION", "POPUP" and "DOWN", rather than adding another flag
parameter to it...
Done, see "enum Action".
+ ListView_SetItemState(hWndListView, -1, 0, LVIS_SELECTED |
LVIS_FOCUSED);
+ ListView_SetItemState(hWndListView, iRow + 1,
LVIS_SELECTED | LVIS_FOCUSED,
+ LVIS_SELECTED | LVIS_FOCUSED);
+ ListView_SetSelectionMark(hWndListView, iRow + 1);
+ ListView_EnsureVisible(hWndListView, iRow + 1, false);
A comment here saying "and move selection to next row".
Done.
...
{
switch (vkey)
{
@@ -154,6 +155,20 @@ PickPackageLine::map_key_to_action(WORD vkey,
int *col_num, int *action_id) cons
*col_num = new_col;
*action_id = -1;
return true;
+ case 'I':
+ case 'R':
+ case 'U':
+ if (!(ctrl && !alt))
As a stylistic thing, I'd perhaps rather combine ctrl and alt flags as
a modifier bitmap.
Done, see 'struct ModifierKeys'.
What is the reasoning for selecting the "ctrl but not alt" modifier
state here?
Why should Ctrl+Alt+I have the same effect as Ctrl+I ?
The new patch also checks for shift.
...
@@ -670,6 +670,10 @@ packagemeta::set_action (_actions action,
packageversion const _version,
else if (action == Uninstall_action)
{
desired = packageversion ();
+ pick (false);
+ srcpick (false);
+ if (!installed)
+ action = NoChange_action;
Hmm... why is adding this needed?
Otherwise a strange state change would occur at least in the GUI when an
install request is undone:
"Skip" == Ctrl+I ==> "3.2-1" == Ctrl+U ==> "Uninstall"
The new patch includes another addition which prevents this on installs
from local directory when the current default version is not yet downloaded:
"Skip" == Ctrl+I ==> "" (empty)
BTW, I didn't understand this line:
void
packagemeta::set_action (...)
{
...
else if (action == Install_action)
{
desired = default_version;
if (desired)
{
if (desired != installed)
if (desired.accessible ())
{
...
pick (true);
srcpick (false);
}
else
{
pick (false);
srcpick (true); <== why true? ==
}
...
From 895f5510731bec0b161ba9b651b5a77dd8cb96a4 Mon Sep 17 00:00:00 2001
From: Christian Franke
Date: Mon, 22 Aug 2022 16:39:21 +0200
Subject: [PATCH] Keyboard accelerators for install/reinstall/uninstall
Ctrl+I/R/U select install/reinstall/uninstall and then move selection
to next row.
---
ListView.cc | 64 +++--
ListView.h | 10 ++-
PickCategoryLine.cc | 21 +++
PickCategoryLine.h | 2 +-
PickPackageLine.cc | 27 ++-
PickPackageLine.h | 2 +-
package_meta.cc | 11
7 files changed, 97 insertions(+), 40 deletions(-)
diff --git a/ListView.cc b/ListView.cc
index 62a37ab..7cc7f0f 100644
--- a/ListView.cc
+++ b/ListView.cc
@@ -24,6 +24,18 @@
// ListView Common Control
// ---
+int ModifierKeys::get()
+{
+ int keys = 0;
+ if (GetKeyState(VK_SHIFT) & 0x8000)
+
Re: [PATCH setup] Add view mode "Unneeded"
Jon Turney wrote:
On 02/08/2022 13:17, Christian Franke wrote:
In long standing cygwin installations, many no longer needed
automatically installed packages (e.g. libicuNN) accumulate. This
patch adds a new view which is possibly helpful to cleanup packages
manually.
Some possible later enhancements:
- automatically refresh this view (a few seconds) after the user
changed a package status as this may add or remove entries.
- add a keyboard shortcut (^U) to the list view for "Uninstall this
package and then select next package"
Thanks. This looks good.
I think perhaps a better approach would be a view showing all packages
which aren't user_picked, or a dependency of a user_picked package.
This would drop the ability to easily clean up user_picked packages
without later conflicts. The attached new patch splits this into two
views "Removable" (not "Uninstallable" due to possible ambiguity with
"cannot be installed") and "Unneeded" (or "Stale" ?).
(If I've read the code correctly your implementation has the weakness
that if e.g. appA -> libbB -> libC, which is then changed to appA ->
libD -> libE, it will only show libC as unneeded, then libB on the
next run?)
I'm not sure for this case. It may be correct again after the view is
refreshed during the same run. In general, this ad-hoc algorithm does
not handle all corner cases. It should be sufficient if installation
cleanup is done in a separate run.
+// Scan installed or desired packages and collect the names of packages
+// which provide the dependencies of other packages or are member of
+// category "Base".
+static void FindNeededPackages (const packagedb & db,
std::set & needed)
+{
+ std::map providedBy;
+ for (const auto & p : db.packages)
+ {
+ const packagemeta & pkg = *p.second;
+ if (!pkg.isBinary ())
+ continue;
+ if (!(pkg.desired && (pkg.installed || pkg.picked (
+ continue;
This seems redundant. Why can't this be just !pkg.desired?
Yes, fixed. I originally wanted to handle the "install source package
without the binary" case here. During development of "Ctrl+I/R/U" patch,
I learned that this could not happen.
This should also update the tooltip for the view dropdown
(IDS_VIEWBUTTON_TOOLTIP) to describe the new view.
Done.
Open issue: An easy way to refresh the views after Uninstall requests
(Ctrl+L ?).
From b31674d809a71bf17bb621c74e5ba7b3df3cd80a Mon Sep 17 00:00:00 2001
From: Christian Franke
Date: Mon, 15 Aug 2022 14:21:36 +0200
Subject: [PATCH] Add view modes "Removable" and "Unneeded"
These views show user picked or automatically installed packages which
could be safely removed.
---
PickView.cc | 53 ++-
PickView.h| 2 ++
res/en/res.rc | 12 +++-
res/fr/res.rc | 4
resource.h| 2 ++
5 files changed, 71 insertions(+), 2 deletions(-)
diff --git a/PickView.cc b/PickView.cc
index c961b9f..7d60d8a 100644
--- a/PickView.cc
+++ b/PickView.cc
@@ -17,6 +17,7 @@
#include "PickPackageLine.h"
#include "PickCategoryLine.h"
#include
+#include
#include
#include
@@ -28,6 +29,39 @@
#include "LogSingleton.h"
#include "Exception.h"
+// Scan desired packages and collect the names of packages which provide the
+// dependencies of other desired packages or are member of category "Base".
+static void FindNeededPackages (const packagedb & db, std::set &
needed)
+{
+ std::map providedBy;
+ for (const auto & p : db.packages)
+{
+ const packagemeta & pkg = *p.second;
+ if (!pkg.isBinary ())
+continue;
+ if (!pkg.desired)
+continue;
+ for (const PackageSpecification *s : pkg.desired.provides ())
+providedBy.insert ({s->packageName (), pkg.name});
+}
+ for (const auto & p : db.packages)
+{
+ const packagemeta & pkg = *p.second;
+ if (!pkg.isBinary ())
+continue;
+ if (pkg.categories.count ("Base"))
+needed.insert (pkg.name);
+ if (!pkg.desired)
+continue;
+ for (const PackageSpecification *s : pkg.desired.depends ()) {
+const auto i = providedBy.find (s->packageName ());
+if (i == providedBy.end ())
+ continue;
+needed.insert (i->second);
+ }
+}
+}
+
void
PickView::setViewMode (views mode)
{
@@ -47,6 +81,11 @@ PickView::setViewMode (views mode)
}
else
{
+ std::set needed;
+ if (view_mode == PickView::views::PackageRemovable
+ || view_mode == PickView::views::PackageUnneeded)
+FindNeededPackages (db, needed);
+
// iterate through every package
for (packagedb::packagecollection::iterator i = db.packages.begin ();
[PATCH setup] Keyboard accelerators for install/reinstall/uninstall
This eases state changes of a selected sequence of packages.
Ctrl+U is in particular useful to cleanup installations in conjunction
with "unneeded" view:
https://sourceware.org/pipermail/cygwin-apps/2022-August/042185.html
Open issue: Add some visual clue (tooltip?) to make this functionality
more obvious.
--
Regards,
Christian
From 71da4fbc68022b5083eba0cbdf2c0a4a541ddf1c Mon Sep 17 00:00:00 2001
From: Christian Franke
Date: Sun, 14 Aug 2022 13:43:48 +0200
Subject: [PATCH] Keyboard accelerators for install/reinstall/uninstall
Ctrl+I/R/U select install/reinstall/uninstall and then move selection
to next list item.
---
ListView.cc | 17 +++--
ListView.h | 3 ++-
PickCategoryLine.cc | 3 ++-
PickCategoryLine.h | 3 ++-
PickPackageLine.cc | 17 -
PickPackageLine.h | 3 ++-
package_meta.cc | 4
7 files changed, 43 insertions(+), 7 deletions(-)
diff --git a/ListView.cc b/ListView.cc
index 62a37ab..22009ba 100644
--- a/ListView.cc
+++ b/ListView.cc
@@ -564,14 +564,20 @@ ListView::OnNotify (NMHDR *pNmHdr, LRESULT *pResult)
NMLVKEYDOWN *pNmLvKeyDown = (NMLVKEYDOWN *)pNmHdr;
int iRow = ListView_GetSelectionMark(hWndListView);
#if DEBUG
- Log (LOG_PLAIN) << "LVN_KEYDOWN vkey " << pNmLvKeyDown->wVKey << " on
row " << iRow << endLog;
+ Log (LOG_PLAIN) << "LVN_KEYDOWN vkey " << pNmLvKeyDown->wVKey << " on
row " << iRow
+ << " Ctrl:" << !!(GetKeyState(VK_CONTROL) & 0x8000)
+ << " Alt:" << !!(GetKeyState(VK_MENU) & 0x8000) <<
endLog;
#endif
if (contents && iRow >= 0)
{
+ bool ctrl = !!(GetKeyState(VK_CONTROL) & 0x8000);
+ bool alt = !!(GetKeyState(VK_MENU) & 0x8000);
int col_num;
int action_id;
- if ((*contents)[iRow]->map_key_to_action(pNmLvKeyDown->wVKey,
_num, _id))
+ bool down = false;
+ if ((*contents)[iRow]->map_key_to_action(pNmLvKeyDown->wVKey, ctrl,
alt, _num,
+ _id, ))
{
int update;
if (action_id >= 0)
@@ -591,6 +597,13 @@ ListView::OnNotify (NMHDR *pNmHdr, LRESULT *pResult)
if (update > 0)
ListView_RedrawItems(hWndListView, iRow, iRow + update -1);
}
+ if (down && iRow + 1 < ListView_GetItemCount(hWndListView)) {
+ListView_SetItemState(hWndListView, -1, 0, LVIS_SELECTED |
LVIS_FOCUSED);
+ListView_SetItemState(hWndListView, iRow + 1, LVIS_SELECTED |
LVIS_FOCUSED,
+ LVIS_SELECTED | LVIS_FOCUSED);
+ListView_SetSelectionMark(hWndListView, iRow + 1);
+ListView_EnsureVisible(hWndListView, iRow + 1, false);
+ }
}
}
break;
diff --git a/ListView.h b/ListView.h
index 95dd9ee..a2ecdef 100644
--- a/ListView.h
+++ b/ListView.h
@@ -38,7 +38,8 @@ class ListViewLine
virtual ActionList *get_actions(int col) const = 0;
virtual int do_action(int col, int id) = 0;
virtual int do_default_action(int col) = 0;
- virtual bool map_key_to_action(WORD vkey, int *col_num, int *action_id)
const = 0;
+ virtual bool map_key_to_action(WORD vkey, bool ctrl, bool alt, int *col_num,
+ int *action_id, bool *down) const = 0;
};
typedef std::vector ListViewContents;
diff --git a/PickCategoryLine.cc b/PickCategoryLine.cc
index d2ac899..9872a2e 100644
--- a/PickCategoryLine.cc
+++ b/PickCategoryLine.cc
@@ -97,7 +97,8 @@ PickCategoryLine::get_tooltip(int col_num) const
}
bool
-PickCategoryLine::map_key_to_action(WORD vkey, int *col_num, int *action_id)
const
+PickCategoryLine::map_key_to_action(WORD vkey, bool ctrl, bool alt, int
*col_num,
+int *action_id, bool *down) const
{
switch (vkey)
{
diff --git a/PickCategoryLine.h b/PickCategoryLine.h
index 6a7321d..0bfba33 100644
--- a/PickCategoryLine.h
+++ b/PickCategoryLine.h
@@ -41,7 +41,8 @@ public:
ActionList *get_actions(int col) const;
int do_action(int col, int action_id);
int do_default_action(int col);
- bool map_key_to_action(WORD vkey, int *col_num, int *action_id) const;
+ bool map_key_to_action(WORD vkey, bool ctrl, bool alt, int *col_num,
+ int *action_id, bool *down) const;
private:
CategoryTree * cat_tree;
diff --git a/PickPackageLine.cc b/PickPackageLine.cc
index ae1e520..ba47e1f 100644
--- a/PickPackageLine.cc
+++ b/PickPackageLine.cc
@@ -145,7 +145,8 @@ PickPackageLine::get_indent() const
}
bool
-PickPackageLine::map_key_to_action(WORD vkey, int *col_num, int *action_id)
const
+PickPackageLine::map_key_to_action(WORD vkey, bool ctrl, bool alt
[PATCH setup] Add view mode "Unneeded"
In long standing cygwin installations, many no longer needed
automatically installed packages (e.g. libicuNN) accumulate. This patch
adds a new view which is possibly helpful to cleanup packages manually.
Some possible later enhancements:
- automatically refresh this view (a few seconds) after the user changed
a package status as this may add or remove entries.
- add a keyboard shortcut (^U) to the list view for "Uninstall this
package and then select next package"
--
Regards,
Christian
From 07a2e561b7ba19817a0bebb2cbf542a9b18a6d4e Mon Sep 17 00:00:00 2001
From: Christian Franke
Date: Tue, 2 Aug 2022 13:54:30 +0200
Subject: [PATCH] Add view mode "Unneeded"
This view shows installed packages which could be safely removed.
---
PickView.cc | 47 ++-
PickView.h| 1 +
res/en/res.rc | 1 +
res/fr/res.rc | 1 +
resource.h| 1 +
5 files changed, 50 insertions(+), 1 deletion(-)
diff --git a/PickView.cc b/PickView.cc
index c961b9f..b7a37b5 100644
--- a/PickView.cc
+++ b/PickView.cc
@@ -17,6 +17,7 @@
#include "PickPackageLine.h"
#include "PickCategoryLine.h"
#include
+#include
#include
#include
@@ -28,6 +29,40 @@
#include "LogSingleton.h"
#include "Exception.h"
+// Scan installed or desired packages and collect the names of packages
+// which provide the dependencies of other packages or are member of
+// category "Base".
+static void FindNeededPackages (const packagedb & db, std::set &
needed)
+{
+ std::map providedBy;
+ for (const auto & p : db.packages)
+{
+ const packagemeta & pkg = *p.second;
+ if (!pkg.isBinary ())
+continue;
+ if (!(pkg.desired && (pkg.installed || pkg.picked (
+continue;
+ for (const PackageSpecification *s : pkg.desired.provides ())
+providedBy.insert ({s->packageName (), pkg.name});
+}
+ for (const auto & p : db.packages)
+{
+ const packagemeta & pkg = *p.second;
+ if (!pkg.isBinary ())
+continue;
+ if (pkg.categories.count ("Base"))
+needed.insert (pkg.name);
+ if (!(pkg.desired && (pkg.installed || pkg.picked (
+continue;
+ for (const PackageSpecification *s : pkg.desired.depends ()) {
+const auto i = providedBy.find (s->packageName ());
+if (i == providedBy.end ())
+ continue;
+needed.insert (i->second);
+ }
+}
+}
+
void
PickView::setViewMode (views mode)
{
@@ -47,6 +82,10 @@ PickView::setViewMode (views mode)
}
else
{
+ std::set needed;
+ if (view_mode == PickView::views::PackageUnneeded)
+FindNeededPackages (db, needed);
+
// iterate through every package
for (packagedb::packagecollection::iterator i = db.packages.begin ();
i != db.packages.end (); ++i)
@@ -77,7 +116,11 @@ PickView::setViewMode (views mode)
// "UserPick" : installed packages that were picked by user
|| (view_mode == PickView::views::PackageUserPicked &&
- (pkg.installed && pkg.user_picked)))
+ (pkg.installed && pkg.user_picked))
+
+ // "Unneeded" : installed packages which could be safely removed
+ || (view_mode == PickView::views::PackageUnneeded &&
+ (pkg.installed && !needed.count (pkg.name
{
// Filter by package name
if (packageFilterString.empty ()
@@ -111,6 +154,8 @@ PickView::mode_caption (views mode)
return IDS_VIEW_NOTINSTALLED;
case views::PackageUserPicked:
return IDS_VIEW_PICKED;
+case views::PackageUnneeded:
+ return IDS_VIEW_UNNEEDED;
case views::Category:
return IDS_VIEW_CATEGORY;
default:
diff --git a/PickView.h b/PickView.h
index 1e14a74..f2dbfa9 100644
--- a/PickView.h
+++ b/PickView.h
@@ -36,6 +36,7 @@ public:
PackageKeeps,
PackageSkips,
PackageUserPicked,
+PackageUnneeded,
Category,
};
diff --git a/res/en/res.rc b/res/en/res.rc
index 644b252..9517bb2 100644
--- a/res/en/res.rc
+++ b/res/en/res.rc
@@ -578,6 +578,7 @@ BEGIN
IDS_VIEW_UPTODATE "Up To Date"
IDS_VIEW_NOTINSTALLED "Not Installed"
IDS_VIEW_PICKED "Picked"
+IDS_VIEW_UNNEEDED "Unneeded"
IDS_VIEW_CATEGORY "Category"
IDS_COLUMN_PACKAGE "Package"
IDS_COLUMN_CURRENT "Current"
diff --git a/res/fr/res.rc b/res/fr/res.rc
index a0a7909..d787fdb 100644
--- a/res/fr/res.rc
+++ b/res/fr/res.rc
@@ -564,6 +564,7 @@ BEGIN
IDS_VIEW_UPTODATE "À jour"
IDS_VIEW_NOTINSTALLED "Non installé"
IDS_VIEW_PICKED "Sélectionné"
+// IDS_VIEW_UNNEEDED "XXX: missing translation"
IDS_VIEW_CAT
[PATCH rebase] Print list of DLLs which still overlap after rebasing + Fix index ...
Corinna Vinschen wrote:
> ...
> Pushed. Do you have a few more changes in the loop? When you're
> finished, I'll release a new rebase.
The attached 0001-*.patch is the last one in the loop - I guessed. But
during testing this I found the long standing bug fixed in 0002-*.patch.
That's all for now.
Thanks,
Christian
From c6e050fa69552e023d18df2cf1255a2a827f1bcc Mon Sep 17 00:00:00 2001
From: Christian Franke
Date: Tue, 19 Jul 2022 10:36:20 +0200
Subject: [PATCH 1/2] Print list of DLLs which still overlap after rebasing
---
rebase.c | 38 ++
1 file changed, 38 insertions(+)
diff --git a/rebase.c b/rebase.c
index 1f9f74b..8ca65cc 100644
--- a/rebase.c
+++ b/rebase.c
@@ -47,6 +47,7 @@ BOOL load_image_info ();
BOOL merge_image_info ();
BOOL collect_image_info (const char *pathname);
void print_image_info ();
+static void print_overlapped ();
BOOL rebase (const char *pathname, ULONG64 *new_image_base, BOOL down_flag);
void parse_args (int argc, char *argv[]);
unsigned long long string_to_ulonglong (const char *string);
@@ -323,6 +324,10 @@ main (int argc, char *argv[])
}
fprintf (stderr, " %s\n", img_info_list[i].name);
}
+ /* Print list of DLLs which still overlap. This could occur if DLLs are
+not rebaseable or if --merge-files is used incorrectly. */
+ if (img_info_size)
+ print_overlapped ();
if (save_image_info () < 0)
return 2;
}
@@ -1149,6 +1154,39 @@ print_image_info ()
}
}
+static void
+print_overlapped ()
+{
+ BOOL header;
+ int i;
+ char overlaps[img_info_size];
+ memset (overlaps, 0, img_info_size);
+ qsort (img_info_list, img_info_size, sizeof (img_info_t), img_info_cmp);
+ for (header = FALSE, i = 0; i < img_info_size; ++i)
+{
+ int j;
+ if (img_info_list[i].flag.needs_rebasing)
+ continue; /* Rebase failed. */
+ for (j = i + 1; j < img_info_size; ++j)
+ {
+ if (img_info_list[j].flag.needs_rebasing)
+ continue; /* Rebase failed. */
+ if (img_info_list[i].base + img_info_list[i].slot_size + offset
+ <= img_info_list[j].base)
+ break;
+ overlaps[i] = overlaps[j] = 1;
+ }
+ if (!overlaps[i])
+ continue;
+ if (!header)
+ {
+ fputs ("\nThe following DLLs still overlap:\n", stderr);
+ header = TRUE;
+ }
+ fprintf (stderr, " %s\n", img_info_list[i].name);
+}
+}
+
BOOL
rebase (const char *pathname, ULONG64 *new_image_base, BOOL down_flag)
{
--
2.37.1
From d618ee13551b861699446f1eb0242f85a71006c6 Mon Sep 17 00:00:00 2001
From: Christian Franke
Date: Tue, 19 Jul 2022 10:40:55 +0200
Subject: [PATCH 2/2] Fix index after removing missing DLL from list
---
rebase.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/rebase.c b/rebase.c
index 8ca65cc..7417d4d 100644
--- a/rebase.c
+++ b/rebase.c
@@ -799,6 +799,7 @@ merge_image_info ()
memmove (overlaps + i, overlaps + i + 1, img_info_size - i - 1);
--img_info_rebase_start;
--img_info_size;
+ --i;
continue;
}
slot_size = roundup2 (cur_size, ALLOCATION_SLOT);
--
2.37.1
Re: [PATCH rebase] Fix handling of newly added non-rebaseable DLLs
Revised version which also handles the --oblivious case.
--
Regards,
Christian
From 19139e1b984eb3f4d11f83e6951c66064a2f2dd3 Mon Sep 17 00:00:00 2001
From: Christian Franke
Date: Mon, 18 Jul 2022 17:06:05 +0200
Subject: [PATCH] Fix handling of newly added non-rebaseable DLLs
Reset needs_rebasing flag to avoid that such a DLL is later removed
from the list due to rebase failure. Add related verbose messages.
---
rebase.c | 18 +-
1 file changed, 17 insertions(+), 1 deletion(-)
diff --git a/rebase.c b/rebase.c
index 39759a9..1f9f74b 100644
--- a/rebase.c
+++ b/rebase.c
@@ -705,6 +705,17 @@ merge_image_info ()
if (verbose)
fprintf (stderr, "rebasing %s because not in database yet\n",
img_info_list[i].name);
}
+ else if (img_info_list[i].flag.needs_rebasing)
+ {
+ /* Not in database yet and not rebaseable. Add without rebasing
or
+skip if --oblivious is active. */
+ img_info_list[i].flag.needs_rebasing = 0;
+ if (verbose)
+ fprintf (stderr, "not rebasing %s because file is not
writable\n",
+img_info_list[i].name);
+ /* FIXME: Overlaps of DLLs in the database with this DLL will
+not be detected below. */
+ }
}
}
if (!img_info_rebase_start || force_rebase_flag)
@@ -715,7 +726,12 @@ merge_image_info ()
{
/* Test DLLs already in database for writability. */
if (i < img_info_rebase_start)
- set_cannot_rebase (_info_list[i]);
+ {
+ set_cannot_rebase (_info_list[i]);
+ if (img_info_list[i].flag.cannot_rebase == 1 && verbose)
+ fprintf (stderr, "not rebasing %s because file is not
writable\n",
+img_info_list[i].name);
+ }
if (!img_info_list[i].flag.cannot_rebase)
{
img_info_list[i].base = 0;
--
2.37.1
[PATCH rebase] Fix handling of newly added non-rebaseable DLLs
The current behavior in this (possibly rare) case is misleading.
--
Regards,
Christian
From 6a68800512ce80689efbf43e639e7c1f38371636 Mon Sep 17 00:00:00 2001
From: Christian Franke
Date: Mon, 18 Jul 2022 16:07:26 +0200
Subject: [PATCH] Fix handling of newly added non-rebaseable DLLs
Reset needs_rebasing flag to avoid that such a DLL is later removed
from the list due to rebase failure. Add related verbose messages.
---
rebase.c | 17 -
1 file changed, 16 insertions(+), 1 deletion(-)
diff --git a/rebase.c b/rebase.c
index 39759a9..56d743d 100644
--- a/rebase.c
+++ b/rebase.c
@@ -705,6 +705,16 @@ merge_image_info ()
if (verbose)
fprintf (stderr, "rebasing %s because not in database yet\n",
img_info_list[i].name);
}
+ else if (img_info_list[i].flag.cannot_rebase == 1)
+ {
+ /* Not in database yet and not rebaseable. Add without rebasing.
*/
+ img_info_list[i].flag.needs_rebasing = 0;
+ if (verbose)
+ fprintf (stderr, "adding %s without rebasing because file is
not writable\n",
+img_info_list[i].name);
+ /* FIXME: Overlaps of DLLs in the database with this DLL will
+not be detected below. */
+ }
}
}
if (!img_info_rebase_start || force_rebase_flag)
@@ -715,7 +725,12 @@ merge_image_info ()
{
/* Test DLLs already in database for writability. */
if (i < img_info_rebase_start)
- set_cannot_rebase (_info_list[i]);
+ {
+ set_cannot_rebase (_info_list[i]);
+ if (img_info_list[i].flag.cannot_rebase == 1 && verbose)
+ fprintf (stderr, "not rebasing %s because file is not
writable\n",
+img_info_list[i].name);
+ }
if (!img_info_list[i].flag.cannot_rebase)
{
img_info_list[i].base = 0;
--
2.37.1
[PATCH rebase] Ensure that no rebaseable DLL overlaps a non-rebaseable DLL
The current check for overlapping non-rebaseable DLLs uses a possible
outdated base address. It also could not detect multiple overlaps by one
non-rebaseable DLL.
Patch size may be misleading due to indentation change.
--
Regards,
Christian
From ee8a966cb6da48b45dfb6de3e732dade81ce7fb9 Mon Sep 17 00:00:00 2001
From: Christian Franke
Date: Sat, 16 Jul 2022 17:55:58 +0200
Subject: [PATCH] Ensure that no rebaseable DLL overlaps a non-rebaseable DLL
Restart rebase decision loop with newly sorted list if a DLL is not
rebaseable. Search for all DLLs which overlap such a DLL.
---
rebase.c | 203 +--
1 file changed, 121 insertions(+), 82 deletions(-)
diff --git a/rebase.c b/rebase.c
index 5cda123..39759a9 100644
--- a/rebase.c
+++ b/rebase.c
@@ -625,6 +625,7 @@ merge_image_info ()
{
int i, end;
img_info_t *match;
+ BOOL sorted;
ULONG64 floating_image_base;
/* Sort new files from command line by name. */
@@ -725,91 +726,129 @@ merge_image_info ()
img_info_rebase_start = 0;
}
- /* Now sort the old part of the list by base address. */
- if (img_info_rebase_start)
-qsort (img_info_list, img_info_rebase_start, sizeof (img_info_t),
- img_info_cmp);
- /* Perform several tests on the information fetched from the database
- to match with reality. */
- for (i = 0; i < img_info_rebase_start; ++i)
+ for (sorted = FALSE; img_info_rebase_start && !sorted; )
{
- ULONG64 cur_base, cur_base_orig;
- ULONG cur_size, slot_size;
-
- /* Files with the needs_rebasing or cannot_rebase flags set have been
-checked already. */
- if (img_info_list[i].flag.needs_rebasing
- || img_info_list[i].flag.cannot_rebase)
- continue;
- /* Check if the files in the old list still exist. Drop non-existant
-or unaccessible files. */
- if (access (img_info_list[i].name, F_OK) == -1
- || !GetImageInfos64 (img_info_list[i].name, NULL,
- _base, _size))
- {
- free (img_info_list[i].name);
- memmove (img_info_list + i, img_info_list + i + 1,
- (img_info_size - i - 1) * sizeof (img_info_t));
- --img_info_rebase_start;
- --img_info_size;
- continue;
- }
- slot_size = roundup2 (cur_size, ALLOCATION_SLOT);
- cur_base_orig = cur_base;
- /* If the file has been reinstalled, try to rebase to the same address
-in the first place. */
- if (cur_base != img_info_list[i].base)
+ char overlaps[img_info_rebase_start];
+ memset(overlaps, 0, img_info_rebase_start);
+ /* Now sort the old part of the list by base address. */
+ qsort (img_info_list, img_info_rebase_start, sizeof (img_info_t),
+img_info_cmp);
+ /* Perform several tests on the information fetched from the database
+to match with reality. */
+ for (sorted = TRUE, i = 0; sorted && i < img_info_rebase_start; ++i)
{
- img_info_list[i].flag.needs_rebasing = 1;
- if (verbose)
- fprintf (stderr, "rebasing %s because it's base has changed (due to
being reinstalled?)\n", img_info_list[i].name);
- /* Set cur_base to the old base to simplify subsequent tests. */
- cur_base = img_info_list[i].base;
- }
- /* However, if the DLL got bigger and doesn't fit into its slot
-anymore, rebase this DLL from scratch. */
- if (i + 1 < img_info_rebase_start
- && cur_base + slot_size + offset > img_info_list[i + 1].base)
- {
- img_info_list[i].base = 0;
- if (verbose)
- fprintf (stderr, "rebasing %s because it won't fit in it's old slot
without overlapping next DLL\n", img_info_list[i].name);
- }
- /* Does the previous DLL reach into the address space of this
-DLL? This happens if the previous DLL is not rebaseable. */
- else if (i > 0 && cur_base < img_info_list[i - 1].base
- + img_info_list[i - 1].slot_size)
- {
- img_info_list[i].base = 0;
- if (verbose)
- fprintf (stderr, "rebasing %s because previous DLL now overlaps\n",
img_info_list[i].name);
- }
- /* Does the file match the base address requirements? If not,
-rebase from scratch. */
- else if ((down_flag && cur_base + slot_size + offset > image_base)
- || (!down_flag && cur_base < image_base))
- {
- img_info_list[i].base = 0;
- if (verbose)
- fprintf (stderr, "rebasing %s because it's base address is outside
the expected area\n", img_info_list[i].name);
- }
- /* Make sure all DLLs with base address 0 have the needs_rebasing
-flag set. */
- if (img_info_list[i].base == 0)
- img_info_list[i].flag.needs_r
Re: [PATCH setup 0/2] Simplify setting group ownership of installed files
Jon Turney wrote:
Jon Turney (2):
Drop group change while running postinstall scripts
Defer setting group until after All Users/Just For Me is chosen
main.cc| 2 +-
postinstall.cc | 13 -
root.cc| 5 +
win32.cc | 13 ++---
win32.h| 2 +-
5 files changed, 9 insertions(+), 26 deletions(-)
Works for me as expected. Primary group of dirs/files of "All Users"
installed files are now local administrator (like before Feb 2012),
including those generated by postinstall scripts (new behavior).
As already mentioned in the '--no-write-registry' thread, "Just me"
installs only work with non-elevated user and '-B' option. Attached is a
temporary patch I used to log changes of root_scope variable.
diff --git a/state.cc b/state.cc
index 111b890..b4e3410 100644
--- a/state.cc
+++ b/state.cc
@@ -24,7 +24,7 @@ int source;
std::string local_dir;
-int root_scope;
+root_scope_holder root_scope;
int root_menu;
int root_desktop;
diff --git a/state.h b/state.h
index b211de3..c561ea5 100644
--- a/state.h
+++ b/state.h
@@ -32,6 +32,7 @@
*/
#include
+#include "LogSingleton.h"
enum attend_mode { attended = 0, unattended, chooseronly };
extern enum attend_mode unattended_mode;
@@ -42,7 +43,22 @@ extern int source;
extern std::string local_dir;
extern int root_text;
-extern int root_scope;
+
+class root_scope_holder
+{
+public:
+ void operator=(int v) {
+int old = val;
+val = v;
+Log(LOG_TIMESTAMP) << "root_scope: " << old << " -> " << v << endLog;
+ }
+ operator int() const { return val; }
+private:
+ int val = 0;
+};
+
+extern root_scope_holder root_scope;
+
extern int root_menu;
extern int root_desktop;
Re: [PATCH setup] Add new option --no-write-registry
Jon Turney wrote:
On 09/07/2022 16:59, Christian Franke wrote:
IMO useful for temporary test installs or "portable" installs to USB
devices.
The 0002-patch adds a related log message.
These patches are ok.
If you can provide a help-text for the new option, I will apply them.
Attached new version of 0001-... patch which includes a help text.
Does not include the *.po* changes, sorry ('pip install
translate-toolkit' failed for some reason),
From 8a93babe4ef963de9fe1d2f5ba77ea9c89afa23c Mon Sep 17 00:00:00 2001
From: Christian Franke
Date: Tue, 12 Jul 2022 18:04:25 +0200
Subject: [PATCH] Add new option --no-write-registry
If specified, the rootdir is not written to the registry and no
registry key is created.
---
install.cc| 6 --
res/en/res.rc | 1 +
res/fr/res.rc | 1 +
resource.h| 1 +
4 files changed, 7 insertions(+), 2 deletions(-)
diff --git a/install.cc b/install.cc
index 1fdc699..fbd28b1 100644
--- a/install.cc
+++ b/install.cc
@@ -70,6 +70,7 @@ static long long int total_bytes_sofar = 0;
static int package_bytes = 0;
static BoolOption NoReplaceOnReboot (false, 'r', "no-replaceonreboot",
IDS_HELPTEXT_NO_REPLACEONREBOOT);
+static BoolOption NoWriteRegistry (false, '\0', "no-write-registry",
IDS_HELPTEXT_NO_WRITE_REGISTRY);
struct std_dirs_t {
const char *name;
@@ -833,8 +834,9 @@ do_install_thread (HINSTANCE h, HWND owner)
int df = diskfull (get_root_dir ().c_str());
Progress.SetBar3 (df);
- /* Writes Cygwin/setup/rootdir registry value */
- create_install_root ();
+ if (!NoWriteRegistry)
+/* Writes Cygwin/setup/rootdir registry value */
+create_install_root ();
std::vector install_q, uninstall_q, sourceinstall_q;
diff --git a/res/en/res.rc b/res/en/res.rc
index 9683ab5..644b252 100644
--- a/res/en/res.rc
+++ b/res/en/res.rc
@@ -679,6 +679,7 @@ BEGIN
IDS_HELPTEXT_NO_VERIFY "Don't verify setup.ini signatures"
IDS_HELPTEXT_NO_VERSION_CHECK "Suppress checking if a newer version of
setup is available"
IDS_HELPTEXT_NO_WARN_DEPRECATED_WINDOWS "Don't warn about deprecated
Windows versions"
+IDS_HELPTEXT_NO_WRITE_REGISTRY "Don't write root installation directory to
registry"
IDS_HELPTEXT_OLD_KEYS "Enable old cygwin.com keys"
IDS_HELPTEXT_ONLY_SITE "Do not download mirror list. Only use sites
specified with -s."
IDS_HELPTEXT_PACKAGES "Specify packages to install"
diff --git a/res/fr/res.rc b/res/fr/res.rc
index 79f2371..a0a7909 100644
--- a/res/fr/res.rc
+++ b/res/fr/res.rc
@@ -665,6 +665,7 @@ BEGIN
IDS_HELPTEXT_NO_VERIFY "Ne pas vérifier les signatures de setup.ini"
IDS_HELPTEXT_NO_VERSION_CHECK "Ne pas vérifier si une version plus récente
de l'assistant est disponible"
IDS_HELPTEXT_NO_WARN_DEPRECATED_WINDOWS "Ne pas avertir pour les vieilles
versions de Windows"
+// IDS_HELPTEXT_NO_WRITE_REGISTRY "XXX: missing translation"
IDS_HELPTEXT_OLD_KEYS "Utiliser les anciennes clés de cygwin.com"
IDS_HELPTEXT_ONLY_SITE "Ignorer tous les sites sauf ceux spécifiés par -s"
IDS_HELPTEXT_PACKAGES "Spécifie les paquets à installer"
diff --git a/resource.h b/resource.h
index e8ed0fa..2668dd9 100644
--- a/resource.h
+++ b/resource.h
@@ -154,6 +154,7 @@
#define IDS_HELPTEXT_ERROR 1545
#define IDS_HELPTEXT_HEADER 1546
#define IDS_HELPTEXT_FOOTER 1547
+#define IDS_HELPTEXT_NO_WRITE_REGISTRY 1548
// Dialogs
--
2.37.0
[PATCH setup] Add new option --no-write-registry
IMO useful for temporary test installs or "portable" installs to USB
devices.
The 0002-patch adds a related log message.
BTW: During testing I found that the "All Users" <> "Just Me" GUI
setting has no effect at all. If setup is run elevated, "All Users" is
always implied.
This is likely because read_mounts() is called again after the "Root"
dialog. It resets root_scope and does not re-read the registry.
read_mounts (const std::string val)
{
...
root_scope = (nt_sec.isRunAsAdmin ())? IDC_ROOT_SYSTEM : IDC_ROOT_USER;
...
}
--
Regards,
Christian
From ab3c94ebf0e78606c3660bec8e0c04c6e6b8ddd4 Mon Sep 17 00:00:00 2001
From: Christian Franke
Date: Sat, 9 Jul 2022 16:52:47 +0200
Subject: [PATCH 1/2] Add new option --no-write-registry
If specified, the rootdir is not written to the registry and no
registry key is created.
---
install.cc | 6 --
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/install.cc b/install.cc
index 1fdc699..eb7b1b2 100644
--- a/install.cc
+++ b/install.cc
@@ -70,6 +70,7 @@ static long long int total_bytes_sofar = 0;
static int package_bytes = 0;
static BoolOption NoReplaceOnReboot (false, 'r', "no-replaceonreboot",
IDS_HELPTEXT_NO_REPLACEONREBOOT);
+static BoolOption NoWriteRegistry (false, '\0', "no-write-registry" /*, TODO:
IDS_HELPTEXT_... */);
struct std_dirs_t {
const char *name;
@@ -833,8 +834,9 @@ do_install_thread (HINSTANCE h, HWND owner)
int df = diskfull (get_root_dir ().c_str());
Progress.SetBar3 (df);
- /* Writes Cygwin/setup/rootdir registry value */
- create_install_root ();
+ if (!NoWriteRegistry)
+/* Writes Cygwin/setup/rootdir registry value */
+create_install_root ();
std::vector install_q, uninstall_q, sourceinstall_q;
--
2.36.1
From 467b30e19e506ecb4b1e9ed9c4d7528d77db0228 Mon Sep 17 00:00:00 2001
From: Christian Franke
Date: Sat, 9 Jul 2022 16:57:31 +0200
Subject: [PATCH 2/2] Log writes to rootdir registry entry
---
mount.cc | 6 ++
1 file changed, 6 insertions(+)
diff --git a/mount.cc b/mount.cc
index f63edd4..0136396 100644
--- a/mount.cc
+++ b/mount.cc
@@ -135,6 +135,12 @@ create_install_root ()
mbox (NULL, IDS_MOUNT_REGISTRY_KEY_FAILED, MB_OK | MB_ICONWARNING);
RegCloseKey (key);
+ Log (LOG_TIMESTAMP) << "Registry value set: HKEY_"
+ << (root_scope == IDC_ROOT_USER ? "CURRENT_USER\\"
+ : "LOCAL_MACHINE\\")
+ << buf << "\\rootdir = \"" << get_root_dir () << "\""
+ << endLog;
+
// The mount table is already in the right shape at this point.
// Reading it again is not necessary.
//read_mounts (std::string ());
--
2.36.1
Re: [PATCH setup] Add new option --chown-admin
Christian Franke wrote: It does fix the regression, it adds a new installation flavor which fixes it as a side effect. It does *not* fix the regression, of course
Re: [PATCH setup] Add new option --chown-admin
Jon Turney wrote: On 06/07/2022 17:34, Christian Franke wrote: Jon Turney wrote: On 06/07/2022 08:14, Christian Franke wrote: If an installer is run elevated, the installed files will be typically owned by the local administrator (or in some cases SYSTEM or TrustedInstaller) instead of the current user. This is not the case for a Cygwin "All Users" installation. The files are then not protected from ... instead the files are owned by the user running setup? Yes, because the TokenUser is unconditionally copied to TokenOwner. Some research with 'git blame' shows that many parts of the related code are from the early days of UAC and elevated processes (Vista 2006, Win7 2009). Things may have changed since then, I don't know (or remember). If a process is run elevated, Windows keeps TokenUser unchanged but sets the TokenOwner to local administrator. The --chown-admin option simply keeps this as is, so no actual chown() is needed later. The TokenPrimaryGroup is not changed by Windows, therefore --chown-admin calls setAdminGroup() to mimic the usual "root root" ownership. Typical simple installers leave everything as is, so the installed files are owned by local adminstrator and group "None" (S-1-5-21-*-513), see 'ls -l "$PROGRAMFILES"'. accidental changes by this user. The attached patch adds an experimental --chown-admin option which allows (new) installations owned by local administrator user and group. Thanks for the patch, but... A drawback is that files generated by postinstall scripts are still owned by current user + "None" group. It should be possible to fix this with some perpetual preremove+postinstall scripts. I also don't know whether this may break some postinstall scripts. BTW: 'nt_sec.setDefaultSecurity (isAdmin)' is never called with 'isAdmin==true' as 'root_scope' is always 0. root_scope is set later, by the "Install For" option on the "Select Root Install Directory" page. To me, this looks like a (very long standing) bug that we shouldn't be calling setAdminGroup() here, but after root_scope has been set. If this bug is very old, I'm not sure whether this should be fixed. Setting admin group to files which are owned "only" by current user is possibly not very effective. It's true that some people might be relying on that buggy behaviour. I have one very old Cygwin installation from Win7 times. Very old installed files still have group="Administrator", newer files have group="None". The timestamps suggest that the regression was introduced early in 2012. The first file with group="None" is from March 2 2012. But, please help me understand how your patch differs from adding an option which fixes that misbehaviour when supplied? It does fix the regression, it adds a new installation flavor which fixes it as a side effect. Possible owner:group assignments of newly installed dirs/files: "All Users": adm:adm -- With this patch and --chown-admin set usr:def -- Current behavior, regardless of this patch usr:adm -- Before 'isAdmin always false' regression was introduced "Just Me": adm:adm -- With this patch, --chown-admin set and setup has admin rights usr:def -- Otherwise (usr = user running setup, adm = S-1-5-32-544, def = S-1-5-21-*-513) (As an aside, looking at how setAdminGroup()/resetPrimaryGroup() are used, changing the token for the postinstall scripts seems unnecessary now, since we don't run mkpasswd there any more...) Agree.
Re: [PATCH setup] Add new option --chown-admin
Jon Turney wrote: On 06/07/2022 08:14, Christian Franke wrote: If an installer is run elevated, the installed files will be typically owned by the local administrator (or in some cases SYSTEM or TrustedInstaller) instead of the current user. This is not the case for a Cygwin "All Users" installation. The files are then not protected from ... instead the files are owned by the user running setup? Yes, because the TokenUser is unconditionally copied to TokenOwner. Some research with 'git blame' shows that many parts of the related code are from the early days of UAC and elevated processes (Vista 2006, Win7 2009). Things may have changed since then, I don't know (or remember). If a process is run elevated, Windows keeps TokenUser unchanged but sets the TokenOwner to local administrator. The --chown-admin option simply keeps this as is, so no actual chown() is needed later. The TokenPrimaryGroup is not changed by Windows, therefore --chown-admin calls setAdminGroup() to mimic the usual "root root" ownership. Typical simple installers leave everything as is, so the installed files are owned by local adminstrator and group "None" (S-1-5-21-*-513), see 'ls -l "$PROGRAMFILES"'. accidental changes by this user. The attached patch adds an experimental --chown-admin option which allows (new) installations owned by local administrator user and group. Thanks for the patch, but... A drawback is that files generated by postinstall scripts are still owned by current user + "None" group. It should be possible to fix this with some perpetual preremove+postinstall scripts. I also don't know whether this may break some postinstall scripts. BTW: 'nt_sec.setDefaultSecurity (isAdmin)' is never called with 'isAdmin==true' as 'root_scope' is always 0. root_scope is set later, by the "Install For" option on the "Select Root Install Directory" page. To me, this looks like a (very long standing) bug that we shouldn't be calling setAdminGroup() here, but after root_scope has been set. If this bug is very old, I'm not sure whether this should be fixed. Setting admin group to files which are owned "only" by current user is possibly not very effective.
Re: New version of GNU ddrescue available (1.26)
Hamish McIntyre-Bhatty wrote: On 05/07/2022 10:22, Christian Franke wrote: Hamish McIntyre-Bhatty wrote: I was wondering if the maintainer for GNU ddrescue could update to 1.26 please? 1.27-pre2 is already released, so I wait for 1.27 final. Fair enough. Do you know if there's a mailing list or similar to be notified of new versions? I was on the GNU mailing list for a bit, but I couldn't configure it to not just constantly spam me. Perhaps you want to try this low volume list: https://lists.gnu.org/mailman/listinfo/bug-ddrescue
[PATCH setup] Add new option --chown-admin
If an installer is run elevated, the installed files will be typically
owned by the local administrator (or in some cases SYSTEM or
TrustedInstaller) instead of the current user. This is not the case for
a Cygwin "All Users" installation. The files are then not protected from
accidental changes by this user.
The attached patch adds an experimental --chown-admin option which
allows (new) installations owned by local administrator user and group.
A drawback is that files generated by postinstall scripts are still
owned by current user + "None" group. It should be possible to fix this
with some perpetual preremove+postinstall scripts.
I also don't know whether this may break some postinstall scripts.
BTW: 'nt_sec.setDefaultSecurity (isAdmin)' is never called with
'isAdmin==true' as 'root_scope' is always 0.
--
Regards,
Christian
From 1dfc8d63a8438e42544b06cfdf225f222107eed2 Mon Sep 17 00:00:00 2001
From: Christian Franke
Date: Wed, 6 Jul 2022 07:41:18 +0200
Subject: [PATCH] Add new option --chown-admin
If specified and the process token owner is the local administrator,
the owner is preserved and the primary group is set to the local
administrator group.
---
main.cc | 3 ++-
win32.cc | 41 ++---
win32.h | 2 +-
3 files changed, 33 insertions(+), 13 deletions(-)
diff --git a/main.cc b/main.cc
index 3a8c5ea..81c4a65 100644
--- a/main.cc
+++ b/main.cc
@@ -99,6 +99,7 @@ static StringOption Arch ("", 'a', "arch", IDS_HELPTEXT_ARCH,
false);
static BoolOption UnattendedOption (false, 'q', "quiet-mode",
IDS_HELPTEXT_QUIET_MODE);
static BoolOption PackageManagerOption (false, 'M', "package-manager",
IDS_HELPTEXT_PACKAGE_MANAGER);
static BoolOption NoAdminOption (false, 'B', "no-admin",
IDS_HELPTEXT_NO_ADMIN);
+static BoolOption ChownAdminOption (false, '\0', "chown-admin" /*, TODO:
IDS_HELPTEXT_... */);
static BoolOption WaitOption (false, 'W', "wait", IDS_HELPTEXT_WAIT);
static BoolOption HelpOption (false, 'h', "help", IDS_HELPTEXT_HELP);
static BoolOption VersionOption (false, 'V', "version", IDS_HELPTEXT_VERSION);
@@ -359,7 +360,7 @@ WinMain (HINSTANCE h,
}
/* Set default DACL and Group. */
-nt_sec.setDefaultSecurity ((root_scope == IDC_ROOT_SYSTEM));
+nt_sec.setDefaultSecurity ((root_scope == IDC_ROOT_SYSTEM),
ChownAdminOption);
/*
If --symlink-type option isn't given, look for winsymlinks in CYGWIN
diff --git a/win32.cc b/win32.cc
index 55072a9..5dc9616 100644
--- a/win32.cc
+++ b/win32.cc
@@ -308,7 +308,7 @@ NTSecurity::setAdminGroup ()
}
void
-NTSecurity::setDefaultSecurity (bool isAdmin)
+NTSecurity::setDefaultSecurity (bool isAdmin, bool keepAdmin)
{
/* Get the processes access token. */
if (!OpenProcessToken (GetCurrentProcess (),
@@ -335,21 +335,40 @@ NTSecurity::setDefaultSecurity (bool isAdmin)
/* Set the default DACL to all permissions for everyone as a fallback. */
setDefaultDACL ();
- /* Get the user */
- if (!GetTokenInformation (token.theHANDLE (), TokenUser, ,
+ /* Get the owner */
+ if (!GetTokenInformation (token.theHANDLE (), TokenOwner, ,
sizeof ownerSID, ))
{
- NoteFailedAPI ("GetTokenInformation(user)");
+ NoteFailedAPI ("GetTokenInformation(owner)");
return;
}
- /* Make it the owner */
- TOKEN_OWNER owner = { ownerSID.user.User.Sid };
- if (!SetTokenInformation (token.theHANDLE (), TokenOwner, ,
- sizeof owner))
+
+ bool ownerIsAdmin = !!EqualSid (ownerSID.user.User.Sid,
administratorsSID.theSID ());
+
+ if (keepAdmin && ownerIsAdmin)
+Log (LOG_TIMESTAMP) << "Default owner is Administrator" << endLog;
+ else
{
- NoteFailedAPI ("SetTokenInformation(owner)");
- return;
+ /* Get the user */
+ if (!GetTokenInformation (token.theHANDLE (), TokenUser, ,
+ sizeof ownerSID, ))
+ {
+ NoteFailedAPI ("GetTokenInformation(user)");
+ return;
+ }
+ /* Make it the owner */
+ TOKEN_OWNER owner = { ownerSID.user.User.Sid };
+ if (!SetTokenInformation (token.theHANDLE (), TokenOwner, ,
+ sizeof owner))
+ {
+ NoteFailedAPI ("SetTokenInformation(owner)");
+ return;
+ }
+ Log (LOG_TIMESTAMP) << "Default owner changed "
+ << (ownerIsAdmin ? "from Administrator " : "")
+ << "to current user" << endLog;
}
+
/* Get original primary group. The token's primary group will be reset
to the original group right before we call the postinstall scripts.
This is necessary, otherwise, if the installing user is a domain user,
Re: New version of GNU ddrescue available (1.26)
Hamish McIntyre-Bhatty wrote: I was wondering if the maintainer for GNU ddrescue could update to 1.26 please? 1.27-pre2 is already released, so I wait for 1.27 final. -- Regards, Christian
Re: SFTP release directories missing
Christian Franke wrote:
Adam Dinwoodie wrote:
I'm currently seeing attempts to run `cygport stage` fail with
an error "cd: Access failed: No such file (/x86_64/release)". And
logging in manually over sftp, that looks to be accurate; the only
file I can see is my !mail file.
```
$ echo $'ls\npwd\n' | sftp cyg...@cygwin.com
Connected to cygwin.com.
sftp> ls
!mail
sftp> pwd
Remote working directory: /
sftp>
```
Clearly something has gone wrong with the upload space for my
packages; can someone fix it / tell me what I need to do to fix it?
I got the same problem 2x today during upload of pass and etckeeper.
Could be fixed by creating the directories manually with mkdir.
Seems to be fixed now. Empty directories {noarch,x86,x86_64}/release
reappeared in my lftp account.
Re: SFTP release directories missing
Adam Dinwoodie wrote: I'm currently seeing attempts to run `cygport stage` fail with an error "cd: Access failed: No such file (/x86_64/release)". And logging in manually over sftp, that looks to be accurate; the only file I can see is my !mail file. ``` $ echo $'ls\npwd\n' | sftp cyg...@cygwin.com Connected to cygwin.com. sftp> ls !mail sftp> pwd Remote working directory: / sftp> ``` Clearly something has gone wrong with the upload space for my packages; can someone fix it / tell me what I need to do to fix it? I got the same problem 2x today during upload of pass and etckeeper. Could be fixed by creating the directories manually with mkdir. -- Regards, Christian
Re: [ITP] etckeeper 1.18.17-1
Adam Dinwoodie wrote: On Wed, Jun 29, 2022 at 09:55:10AM +0200, Christian Franke wrote: Christian Franke wrote: ... A possible simple extension which would allow the user to choose between manual or automatic installation+initialization: Provide an optional package, for example "etckeeper-git-init", which depends on etckeeper+git and only contains /etc/postinstall/etckeeper-git-init.sh which triggers new initialization code in /etc/postinstall/zp_zzz_etckeeper-postinstall.sh via some file in /var/cache/etckeeper. This code performs 'etckeeper init && etckeeper commit' if and only if VCS=git is selected and /etc/.git does not exist. Honestly, I suspect it's not worth the effort of doing things like that. As you say, 99.8% of users who might be interested in using etckeeper are going to be people who already have a good idea what they're doing and will be able to work it out for themselves. Thinking about it some more, I'm also mildly concerned about the small but non-trivial proportion of users who blithely install every package available on Cygwin, which I don't think is going to be an issue for more-or-less any other *nix distribution. I don't normally think it's worth doing much to actively catering for those users -- I'm generally of the opinion that they're making their own misery -- but in this case automatically starting etckeeper would be a potentially significant impact, and for the sake of both their lives and yours, I suspect it's best to just leave etckeeper as something that requires manual initiation. Good point. That said, if you're keen to set up that optional package, I definitely don't think it's a bad idea; "it wouldn't be worth the effort to me" doesn't mean you shouldn't do it! I now decided to leave this for later (or never). The package is new and if there are still issues during initialization, the messages should be visible on console.
Re: [PATCH setup] Add perpetual support for preremove scripts
Christian Franke wrote:
Jon Turney wrote:
...
Can you please also write a patch for [1] (source in [2]) to document
this?
[1] https://cygwin.com/packaging-package-files.html#postinstall
[2]
https://cygwin.com/git/?p=cygwin-htdocs.git;a=blob;f=packaging-package-files.html
Of course. I will possibly wait until my ITP of etckeeper is accepted
to have a real world example for the doc.
Patch attached. Written under the assumption that "[PATCH] Also run
stratum 'z' perpetual preremove scripts" and "[ITP] etckeeper 1.18.17-1"
will eventually be accepted :-)
From b8225603a5d66760445c04ec14861764deb1489f Mon Sep 17 00:00:00 2001
From: Christian Franke
Date: Fri, 1 Jul 2022 18:52:17 +0200
Subject: [PATCH] Add perpetual pre-remove scripts
---
packaging-package-files.html | 40 +---
1 file changed, 24 insertions(+), 16 deletions(-)
diff --git a/packaging-package-files.html b/packaging-package-files.html
index 446e62db..5b214f53 100755
--- a/packaging-package-files.html
+++ b/packaging-package-files.html
@@ -223,27 +223,35 @@ etc...
after it is installed.
-Perpetual post-install scripts
+Perpetual post-install and pre-remove scripts
- In addition to the ordinary ("run-once") post-install scripts described
above,
- the setup program supports "perpetual" post-install scripts. These are run
on
- every invocation of setup, as long as the package is still installed.
- Perpetual post-install scripts are distinguished from run-once scripts by
+ In addition to the ordinary ("run-once") scripts described above,
+ the setup program supports "perpetual" post-install and pre-remove scripts.
+ These are run on every invocation of setup, as long as the package is still
+ installed. Perpetual scripts are distinguished from run-once scripts by
having names that start with "0p_" or "zp_". Those that start with "0p_" are
- run before the run-once scripts, and those that start with "zp_" are run
after
- the run-once scripts. Examples include
- 0p_000_autorebase.dash (provided by the
_autorebase package)
- and 0p_update-info-dir.dash (provided by the info
package).
+ run before the run-once scripts, and those that start with "zp_" are run
+ after the run-once scripts. Examples include
+ postinstall/0p_000_autorebase.dash (provided by the
+ _autorebase package),
+ postinstall/0p_update-info-dir.dash (provided by the
+ info package),
+ postinstall/zp_zzz_etckeeper_post-install.sh and
+ preremove/0p_000_etckeeper_pre-install.sh (provided by the
+ etckeeper package).
For those package maintainers wanting to employ perpetual scripts, the first
- thing to keep in mind is to only use this feature for things that really
can't
- be done with run-once scripting. Any perpetual script should minimize the
- resources used (use dash instead of bash for instance) and exit at the
- earliest possible moment if no action is required. Scripts of type "0p_"
must
- be able to run with the Base packages installed but the post-install scripts
- not yet executed; in practical terms that rules out using bash scripts. This
- limitation does not apply to scripts of type "zp_".
+ thing to keep in mind is to only use this feature for things that really
+ can't be done with run-once scripting. Any perpetual script should minimize
+ the resources used (use dash instead of bash for instance) and exit at the
+ earliest possible moment if no action is required. Post-install scripts of
+ type "0p_" must be able to run with the Base packages installed but the
+ remaining post-install scripts not yet executed; in practical terms that
+ rules out using bash scripts. Pre-remove scripts of type "zp_" must be able
+ to run with the other pre-remove scripts already executed. These limitations
+ do not apply to post-install scripts of type "zp_" and pre-remove scripts of
+ type "0p_".
See https://cygwin.com/ml/cygwin-apps/2014-12/msg00148.html;>this
--
2.36.1
[ITP] pass 1.7.4-1 (password-store)
I would like to contribute pass (password-store): https://www.passwordstore.org/ https://repology.org/project/password-store/versions pass-1.7.4-1.hint: category: Utils requires: bash coreutils grep sed tree sdesc: "Lightweight CLI directory-based password manager" ldesc: "Pass is a command line tool which stores, retrieves and generates passwords securely using gpg. Optionally password changes could be tracked and synchronized with git. Passwords could be temporarily pasted to /dev/clipboard or displayed as QR codes (requires package qrencode). Auto completion modules for bash, fish and zsh are included. A vim plugin disables insecure options during password editing." Package for review: wget -r -nH --cut-dirs=2 \ https://chrfranke.de/cygwin/noarch/pass-1.7.4-1.hint \ https://chrfranke.de/cygwin/noarch/pass-1.7.4-1-src.hint \ https://chrfranke.de/cygwin/noarch/pass-1.7.4-1-src.tar.xz \ https://chrfranke.de/cygwin/noarch/pass-1.7.4-1.tar.xz \ https://chrfranke.de/cygwin/noarch/pass-1.7.4-1.sha256 gpg or gpg2 are also required. Non-Cygwin versions of gpg.exe or gpg2.exe are also supported if neither gnupg nor gnupg2 is installed. Package naming differs in practice: - "pass": Fedora, ... - I decided to use this. - "pass" but "password-store" for src package: Debian, Ubuntu, ... - "password-store": ... -- Regards, Christian
Re: [PATCH setup] Add perpetual support for preremove scripts
Christian Franke wrote:
Jon Turney wrote:
On 26/06/2022 17:33, Christian Franke wrote:
...
This patch adds the missing functionality to run the pre-install
hook. It is limited to /etc/preremove/0p_* because there is possibly
no use case for /etc/preremove/zp_*.
Thanks.
I'm not sure what you mean by 'there is possibly no use case': That
you don't have one currently, or that you've reasoned that there
can't be one?
I don't have one currently and found none which is useful in practice,
but cannot prove that there is none. If desired, I could provide a
patch which adds 'zp_*' support.
Meantime I realized that this is one of these cases where discussion may
take longer than implementation. Attached is a patch ...
...
I applied this patch.
Thanks. I found a minor GUI issue during testing: Script filename
display persists during package remove phase. Fixed with attached patch.
... which should be applied on top of this last patch.
From 7e3350f633f18e5639a109e0d779473e949ebe57 Mon Sep 17 00:00:00 2001
From: Christian Franke
Date: Wed, 29 Jun 2022 19:57:26 +0200
Subject: [PATCH] Also run stratum 'z' perpetual preremove scripts
---
install.cc | 29 +
1 file changed, 17 insertions(+), 12 deletions(-)
diff --git a/install.cc b/install.cc
index 0ceb05f..1fdc699 100644
--- a/install.cc
+++ b/install.cc
@@ -76,26 +76,28 @@ struct std_dirs_t {
mode_t mode;
};
-class Perpetual0RemoveFindVisitor : public FindVisitor
+class PerpetualRemoveFindVisitor : public FindVisitor
{
public:
- explicit Perpetual0RemoveFindVisitor (std::vector
Re: [PATCH setup] Add perpetual support for preremove scripts
Jon Turney wrote:
On 26/06/2022 17:33, Christian Franke wrote:
Use case: I ITP etckeeper (https://etckeeper.branchable.com/) which I
frequently use on Debian. For fully automatic operation, it requires
pre-install and post-install hooks, e.g:
/etc/preremove/0p_000_etckeeper_pre-install.sh
/etc/postinstall/zp_zzz_etckeeper_post-install.sh
This patch adds the missing functionality to run the pre-install
hook. It is limited to /etc/preremove/0p_* because there is possibly
no use case for /etc/preremove/zp_*.
Thanks.
I'm not sure what you mean by 'there is possibly no use case': That
you don't have one currently, or that you've reasoned that there can't
be one?
I don't have one currently and found none which is useful in practice,
but cannot prove that there is none. If desired, I could provide a patch
which adds 'zp_*' support.
'class Perpetual0RemoveFindVisitor' is borrowed from postinstall.cc
and modified. It still uses the ugly pre-C++11 hack to disable
copy-ctor and operator=. Possible refactoring like merging all 3
mostly similar visitors into one (or if C++11 is now allowed, use
lambda functions instead) are left for later.
Yeah, some refactoring would make this simpler and easier to
understand. :)
I applied this patch.
Thanks. I found a minor GUI issue during testing: Script filename
display persists during package remove phase. Fixed with attached patch.
Can you please also write a patch for [1] (source in [2]) to document
this?
[1] https://cygwin.com/packaging-package-files.html#postinstall
[2]
https://cygwin.com/git/?p=cygwin-htdocs.git;a=blob;f=packaging-package-files.html
Of course. I will possibly wait until my ITP of etckeeper is accepted to
have a real world example for the doc.
--
Regards,
Christian
From dd31653b99c3a970fd145d2ffbad5809de7e0273 Mon Sep 17 00:00:00 2001
From: Christian Franke
Date: Wed, 29 Jun 2022 16:51:47 +0200
Subject: [PATCH] Clear filename on GUI after running perpetual preremove
scripts
---
install.cc | 1 +
1 file changed, 1 insertion(+)
diff --git a/install.cc b/install.cc
index 6689a08..0ceb05f 100644
--- a/install.cc
+++ b/install.cc
@@ -192,6 +192,7 @@ Installer::preremovePerpetual0 ()
Progress.SetText3 (i->fullName ().c_str());
i->run();
}
+ Progress.SetText3 ("");
}
void
--
2.36.1
