Re: [PATCH cygport] Add more checks of SOURCE_DATE_EPOCH

2024-02-26 Thread Christian Franke via Cygwin-apps

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



Re: [PATCH cygport] Add more checks of SOURCE_DATE_EPOCH

2024-02-26 Thread Jon Turney via Cygwin-apps

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?

Would it not make more sense to just re-export it if set? (so that 
commands like "SOURCE_DATE_EPOCH=something cygport foo" work as expected?)




Re: [cygport] enabling a replacement for "objdump -d -l"

2024-02-26 Thread Jon Turney via Cygwin-apps



Thanks, this is great!

On 18/02/2024 19:51, ASSI via Cygwin-apps wrote:



[...]

dwarf-parse.-pl


There should be some build system changes which install this file, 
probably in /usr/share/cygport/tools/, which it can then be run from.



--8<---cut here---start->8---


Please, please make a patch with git format-patch, which I can then just 
apply.



#!perl -w


Fifty lines of perl with no comments! This is just line noise to me 
unless I spend lots of time staring at it :)


Seriously, this should at least say "I'm running objdump -Wl to dump out 
the .debug_line section containing DWARF XYZ information.


Then maybe some comments about what assumptions it's making about the 
human-readable output it's parsing.



use common::sense;
use List::Util qw( sum );

my $filter = shift @ARGV
 or die "not enough arguments";
my $obj = shift @ARGV
 or die "not enough arguments";
my @objdump = qw( /usr/bin/objdump -WNl );


cygport goes to some lengths to identify the correct objdump to use when 
cross-building, so it should probably should be used here (passed in as 
an arg?), rather than assuming it's /usr/bin/objdump.



open my $DWARF, "-|", @objdump, $obj
 or die "can't invoke objdump\n$!";

my ( @dirs, @files, %fn, %rn );
while (<$DWARF>) {
 if (/^ The Directory Table/../^$/) {
if (/^  \d+/) {

my ( $entry, $dir ) = m/^  (\d+)\t.+: (.+)$/;
$dir = "$dirs[0]/$dir" if ($dir =~ m:\A[^/]:);
push @dirs, $dir;
}
 }
 if (/^ The File Name Table/../^$/) {
if (/^  \d+/) {
my ( $idx, $fn, undef ) = m/^  \d+\t(\d+)\t.+: (.+)$/;
$rn{"$dirs[$idx]/$fn"}++;
push @files, "$dirs[$idx]/$fn";
}
 }
 if (my $rc = /^ Line Number Statements/../^  Offset:/) {
$fn{"$files[0]"}++ if ($rc == 1);
$fn{"$files[$1]"}++ if m/ Set File Name to entry (\d+) in the File Name 
Table/;


What this line is doing is obvious, the rest of this block, not so much.

You might also like to touch on why we bother looking at the line number 
information at all, rather than just producing a (filtered) list of all 
the pathnames mentioned?



@files = () if ($rc =~ m/E0$/);
@dirs  = () if ($rc =~ m/E0$/);
 }
 if (/^ No Line Number Statements./../^$/) {
@files = ();
@dirs  = ();
 }
}
foreach my $fn (grep m:^$filter:, sort keys %fn) {
 say sprintf "%s", $fn;
}
say STDERR sprintf "\tLNS: %6d (%6d locations) <=> FNT: %6d ( %6d locations)",
 0+grep( m:^$filter:, keys %fn ), sum( values %fn ),
 0+grep( m:^$filter:, keys %rn ), sum( values %rn )
 if (0);


If you're going to keep this (which you probably should), perhaps it 
should be under some 'if (DEBUG)' conditional.




close $DWARF
 or die "failed to close objdump\n$!";
--8<---cut here---end--->8---

Integration into cygport is made configurable via a variable to be set
in .cygportrc for instance in order to easily revert back to the
original objdump invocation if necessary.  I've been producing packages


DWARF_PARSE should be mentioned in the documentation for cygport.conf

Since the helper script will be installed, it could be made a boolean.


with that setup for a while now and have not noticed any errors.  In
principle the new parser actually produces more complete output as there
can be multiple line number statements and hence source files per
location, but objdump only lists one of them in the disassembly (at
least sometimes).  In practise I haven't found a package until now where
the final list (after filtering) is different.

--8<---cut here---start->8---
lib/src_postinst.cygpart: use DWARF_PARSE optionally instead of objdump -dl
---

diff --git a/lib/src_postinst.cygpart b/lib/src_postinst.cygpart
index f06004e4..3dd6e893 100644
--- a/lib/src_postinst.cygpart
+++ b/lib/src_postinst.cygpart
@@ -1096,7 +1096,12 @@ __prepstrip_one() {
else
dbg="/usr/lib/debug/${exe}.dbg";
  
-		lines=$(${objdump} -d -l "${exe}" 2>/dev/null | sed -ne "s|.*\(/usr/src/debug/${PF}/.*\):[0-9]*$|\1|gp" | sort -u | tee -a ${T}/.dbgsrc.out.${oxt} | wc -l);

+   if defined DWARF_PARSE
+   then
+   lines=$(${DWARF_PARSE} /usr/src/debug/${PF}/ "${exe}" | 
tee -a ${T}/.dbgsrc.out.${oxt} | wc -l);
+   else
+   lines=$(${objdump} -d -l "${exe}" 2>/dev/null | sed -ne 
"s|.*\(/usr/src/debug/${PF}/.*\):[0-9]*$|\1|gp" | sort -u | tee -a ${T}/.dbgsrc.out.${oxt} | 
wc -l);
+   fi
  
  		# we expect --add-gnu-debuglink to fail if a

# .gnu_debuglink section already exists (e.g. binutils,
--8<---cut here---end--->8---