chattr makes cygport slow

chattr makes cygport slow

[Andrew Schulman]

Recently I noticed that `cygport finish` has become really slow on some of my
package source trees. After I run for example

cygport libargp.cygport finish

it waits for about 5 minutes without any message to the console, before the
first "Removing work directory" message appears.

pstree shows that during this time cygport is waiting for chattr. In
/usr/bin/cygport I see:

if [ $OSTYPE = "cygwin" ]
then
    chattr -fR +C ${workdir} >/dev/null 2>&1 || true
fi

which is trying to make the workdir case-sensitive.

Whatever the advantages of that are, it can take a long time. Would it be
possible to skip it at least in the case of "finish"? It seems silly to spend
all that time fixing up a directory tree that we then turn around and remove
with rm -rf.

Thanks,
Andrew

Re: chattr makes cygport slow

[Brian Inglis]

On 2023-07-06 06:19, Andrew Schulman via Cygwin-apps wrote:
> Recently I noticed that `cygport finish` has become really slow on some of my
> package source trees. After I run for example
> 
> cygport libargp.cygport finish
> 
> it waits for about 5 minutes without any message to the console, before the
> first "Removing work directory" message appears.
> 
> pstree shows that during this time cygport is waiting for chattr. In
> /usr/bin/cygport I see:
> 
> if [ $OSTYPE = "cygwin" ]
> then
>      chattr -fR +C ${workdir} >/dev/null 2>&1 || true
> fi
> 
> which is trying to make the workdir case-sensitive.
> 
> Whatever the advantages of that are, it can take a long time. Would it be
> possible to skip it at least in the case of "finish"? It seems silly to spend
> all that time fixing up a directory tree that we then turn around and remove
> with rm -rf.

The attribute does not appear to be inheritable, so will not be applied to 
subdirectories created by make, or in cygport xargs commands, unless supported 
in cygwin1.dll, perhaps why it is open coded in cygport?

Perhaps it could be moved below case prep, into src_prep `__mkdirs`, or globally 
define `mkdir_p()` which applies it to each directory in those paths?
Could this also be defined in MKDIR_P for make or are those limited to binaries?

All cygport scripts appear to consistently use `mkdir -p` including after `xargs 
-r` (`--no-run-if-empty`) except:

/usr/share/cygport/cygclass/fossil.cygclass:    mkdir ${FOSSIL_MODULE}

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

La perfection est atteinte                   Perfection is achieved
non pas lorsqu'il n'y a plus rien à ajouter  not when there is no more to add
mais lorsqu'il n'y a plus rien à retirer     but when there is no more to cut
                                 -- Antoine de Saint-Exupéry

Re: chattr makes cygport slow

[Andrew Schulman]

> Recently I noticed that `cygport finish` has become really slow on some of my
> package source trees. After I run for example
> 
> cygport libargp.cygport finish
> 
> it waits for about 5 minutes without any message to the console, before the
> first "Removing work directory" message appears.
> 
> pstree shows that during this time cygport is waiting for chattr. In
> /usr/bin/cygport I see:
> 
> if [ $OSTYPE = "cygwin" ]
> then
>     chattr -fR +C ${workdir} >/dev/null 2>&1 || true
> fi
> 
> which is trying to make the workdir case-sensitive.
> 
> Whatever the advantages of that are, it can take a long time. Would it be
> possible to skip it at least in the case of "finish"? It seems silly to spend
> all that time fixing up a directory tree that we then turn around and remove
> with rm -rf.

And/or, maybe add a switch to suppress it. I've commented it out of my
/usr/bin/cygport for now, because it makes me wait an extra 5 minutes before
every cygport command, at least on this particular source tree with 12K files in
it.

Re: chattr makes cygport slow

[Jon Turney]

On 06/07/2023 18:36, Andrew Schulman via Cygwin-apps wrote:
>> Recently I noticed that `cygport finish` has become really slow on some of my
>> package source trees. After I run for example
>>
>> cygport libargp.cygport finish
>>
>> it waits for about 5 minutes without any message to the console, before the
>> first "Removing work directory" message appears.
>>
>> pstree shows that during this time cygport is waiting for chattr. In
>> /usr/bin/cygport I see:
>>
>> if [ $OSTYPE = "cygwin" ]
>> then
>>      chattr -fR +C ${workdir} >/dev/null 2>&1 || true
>> fi
>>
>> which is trying to make the workdir case-sensitive.
>>
>> Whatever the advantages of that are, it can take a long time. Would it be
>> possible to skip it at least in the case of "finish"? It seems silly to spend
>> all that time fixing up a directory tree that we then turn around and remove
>> with rm -rf.

For a long time, we've just been assuming that anyone using cygport has 
case-sensitivity turned on somehow (as the build underling a cygport 
might assume that it's present, as is normal on a unix)

So, the idea here is that we try to ensure it's on, at least for the 
working directory.

Skipping it when 'finish' is used isn't right, because then 'finish all' 
wouldn't work as desired.  But yeah, it seems that this is in the wrong 
place.  I'll look into moving it.

I think there's possibly something else going wrong if it's taking 5 
minutes, as that seems excessive.

Re: chattr makes cygport slow

[Jon Turney]

On 06/07/2023 17:18, Brian Inglis via Cygwin-apps wrote:
> On 2023-07-06 06:19, Andrew Schulman via Cygwin-apps wrote:
>> Recently I noticed that `cygport finish` has become really slow on 
>> some of my
>> package source trees. After I run for example
>>
>> cygport libargp.cygport finish
>>
>> it waits for about 5 minutes without any message to the console, 
>> before the
>> first "Removing work directory" message appears.
>>
>> pstree shows that during this time cygport is waiting for chattr. In
>> /usr/bin/cygport I see:
>>
>> if [ $OSTYPE = "cygwin" ]
>> then
>>      chattr -fR +C ${workdir} >/dev/null 2>&1 || true
>> fi
>>
>> which is trying to make the workdir case-sensitive.
>>
>> Whatever the advantages of that are, it can take a long time. Would it be
>> possible to skip it at least in the case of "finish"? It seems silly 
>> to spend
>> all that time fixing up a directory tree that we then turn around and 
>> remove
>> with rm -rf.
> 
> The attribute does not appear to be inheritable, so will not be applied 
> to subdirectories created by make, or in cygport xargs commands, unless 
> supported in cygwin1.dll, perhaps why it is open coded in cygport?

I believe that might have been the case in some previews, but it does 
inherit now. Otherwise it's almost completely, unusably useless.

Re: chattr makes cygport slow

[Andrew Schulman]

> On 06/07/2023 18:36, Andrew Schulman via Cygwin-apps wrote:
> >> Recently I noticed that `cygport finish` has become really slow on some of my
> >> package source trees. After I run for example
> >>
> >> cygport libargp.cygport finish
> >>
> >> it waits for about 5 minutes without any message to the console, before the
> >> first "Removing work directory" message appears.
> >>
> >> pstree shows that during this time cygport is waiting for chattr. In
> >> /usr/bin/cygport I see:
> >>
> >> if [ $OSTYPE = "cygwin" ]
> >> then
> >>      chattr -fR +C ${workdir} >/dev/null 2>&1 || true
> >> fi
> >>
> >> which is trying to make the workdir case-sensitive.
> >>
> >> Whatever the advantages of that are, it can take a long time. Would it be
> >> possible to skip it at least in the case of "finish"? It seems silly to spend
> >> all that time fixing up a directory tree that we then turn around and remove
> >> with rm -rf.
> 
> For a long time, we've just been assuming that anyone using cygport has 
> case-sensitivity turned on somehow (as the build underling a cygport 
> might assume that it's present, as is normal on a unix)
> 
> So, the idea here is that we try to ensure it's on, at least for the 
> working directory.
> 
> Skipping it when 'finish' is used isn't right, because then 'finish all' 
> wouldn't work as desired.  But yeah, it seems that this is in the wrong 
> place.  I'll look into moving it.

Thanks. Annoying, I realize. Unfortunately doing it separately for each command
isn't right either, if it runs 3 times instead of once when I run 'cygport prep
make inst'.

Also, just fixing it for 'finish' would still leave the delay for the other
commands. Maybe a switch that would allow turning it off, either host-wide in
~/.cygport.conf, or for a project that didn't need it in *.cygport.
 
> I think there's possibly something else going wrong if it's taking 5 
> minutes, as that seems excessive.

Could be BLODA. My host has MS antivirus.

Re: chattr makes cygport slow

[ASSI]

Jon Turney via Cygwin-apps writes:
> So, the idea here is that we try to ensure it's on, at least for the
> working directory.

That IMHO should only be done when the working directory is created, but
not retroactively applied to an existing workdir.

> Skipping it when 'finish' is used isn't right, because then 'finish
> all' wouldn't work as desired.  But yeah, it seems that this is in the
> wrong place.  I'll look into moving it.

Meanwhile I've done this:

--8<---------------cut here---------------start------------->8---
 if [ $OSTYPE = "cygwin" ]
 then
+	[ -n "$(find ${workdir} -maxdepth 0 -empty)" ] &&
 	chattr -fR +C ${workdir} >/dev/null 2>&1 || true
 fi
--8<---------------cut here---------------end--------------->8---

I think it should be moved into a function that can be called before the
prep command, but I haven't actually tried it for a longer time or
checked what tests need changing due to the extra output:

--8<---------------cut here---------------start------------->8---
From 4be00357be8cb14493413b7c8c80777671c21e5d Mon Sep 17 00:00:00 2001
Subject: [PATCH] cygport.in: move case-sensitivity testing / enabling into
 functions

---
 bin/cygport.in | 46 ++++++++++++++++++++++++++++++----------------
 1 file changed, 30 insertions(+), 16 deletions(-)

diff --git a/bin/cygport.in b/bin/cygport.in
index 02e9b7bf..a6dabc91 100755
--- a/bin/cygport.in
+++ b/bin/cygport.in
@@ -577,25 +577,36 @@ declare -r  pkg_count=${#pkg_name[*]};
 # this requires workdir to be already defined
 mkdir -p ${workdir}
 
-if [ $OSTYPE = "cygwin" ]
-then
-	chattr -fR +C ${workdir} >/dev/null 2>&1 || true
-fi
-
-rm -f ${workdir}/.probe_case_sensitivity
-touch ${workdir}/.PROBE_CASE_SENSITIVITY
-probe=$( [ -f ${workdir}/.probe_case_sensitivity ] && echo "failed" || echo "" )
-rm -f ${workdir}/.PROBE_CASE_SENSITIVITY
-if [ -n "$probe" ]
-then
+__enable_case_sensitivity() {
+    inform "Trying to enable case sensitivity on ${workdir}"
+    if [ $OSTYPE = "cygwin" ]
+    then
+	[ -n "$(find ${workdir} -maxdepth 0 -empty)" ] &&
+	    chattr -fR +C ${workdir} >/dev/null 2>&1 || true
+	__probe_case_sensitivity
+    fi
+}
+
+__probe_case_sensitivity() {
+    mkdir -p ${workdir}
+    rm -f ${workdir}/.probe_case_sensitivity
+    touch ${workdir}/.PROBE_CASE_SENSITIVITY
+    local probe=$( [ -f ${workdir}/.probe_case_sensitivity ] && echo "failed" || echo "" )
+    rm -f ${workdir}/.PROBE_CASE_SENSITIVITY
+    if [ -n "$probe" ]
+    then
 	if defined _CYGPORT_RESTRICT_case_insensitive_
 	then
-		error "Package ${NAME} cannot be built on a case-insensitive filesystem";
+	    error "Package ${NAME} cannot be built on a case-insensitive filesystem";
 	else
-		warning "Building on a case-insensitive filesystem";
+	    warning "Building on a case-insensitive filesystem";
 	fi
-fi
-unset probe
+	false
+    else
+	inform "Building on a case-sensitive filesystem";
+	true
+    fi
+}
 
 # this requires S and B to be already defined
 if ! defined _CYGPORT_RESTRICT_debuginfo_
@@ -638,15 +649,18 @@ while (( arg_n < argc ))
 do
 	case ${argv[${arg_n}]} in
 		downloadall|fetchall|wgetall|getall)
+		        __probe_case_sensitivity || __enable_case_sensitivity;
 			__src_fetch;
 			_status=$?;
 			;;
 		download|fetch|wget|get)
+		        __probe_case_sensitivity || __enable_case_sensitivity;
 			__DL_ONLY_MISSING=1 __src_fetch;
 			_status=$?;
 			;;
 		prepare|unpack|prep)
-			__stage Preparing;
+		        __probe_case_sensitivity || __enable_case_sensitivity;
+		        __stage Preparing;
 			__src_prep;
 			_status=$?;
 			;;
-- 
2.41.0


--8<---------------cut here---------------end--------------->8---

> I think there's possibly something else going wrong if it's taking 5
> minutes, as that seems excessive.

Try it on the gcc build dir…


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

Factory and User Sound Singles for Waldorf Q+, Q and microQ:
http://Synth.Stromeko.net/Downloads.html#WaldorfSounds

Re: chattr makes cygport slow

[Jon Turney]

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

On 08/07/2023 15:22, ASSI via Cygwin-apps wrote:
> Jon Turney via Cygwin-apps writes:
>> So, the idea here is that we try to ensure it's on, at least for the
>> working directory.
> 
> That IMHO should only be done when the working directory is created, but
> not retroactively applied to an existing workdir.

Yeah, that's just a side effect of where I put it.

>> Skipping it when 'finish' is used isn't right, because then 'finish
>> all' wouldn't work as desired.  But yeah, it seems that this is in the
>> wrong place.  I'll look into moving it.
> 
> Meanwhile I've done this:
> 
> --8<---------------cut here---------------start------------->8---
>   if [ $OSTYPE = "cygwin" ]
>   then
> +	[ -n "$(find ${workdir} -maxdepth 0 -empty)" ] &&
>   	chattr -fR +C ${workdir} >/dev/null 2>&1 || true

This seems a bit nuts as we've just done the mkdir a few lines above, we 
could just check if the directory doesn't exist and apply 
case-sensitivity if we are creating it.

>   fi
> --8<---------------cut here---------------end--------------->8---
> 
> I think it should be moved into a function that can be called before the
> prep command, but I haven't actually tried it for a longer time or
> checked what tests need changing due to the extra output:

The warning (error if RESTRICT=case_insensitive) should occur for all 
commands, not just prep.

How about the attached.

>> I think there's possibly something else going wrong if it's taking 5
>> minutes, as that seems excessive.
> 
> Try it on the gcc build dir…

Even then, it should only be modifying every directory, not every file.

[-- Attachment #2: 0001-cygport.in-only-try-to-turn-on-case-sensitivity-when.patch --]
[-- Type: text/plain, Size: 2238 bytes --]

From 0e4c4261122d3b2373734816c9620c5c44eb72a5 Mon Sep 17 00:00:00 2001
From: Jon Turney <jon.turney@dronecode.org.uk>
Date: Sun, 9 Jul 2023 18:35:51 +0100
Subject: [PATCH cygport] cygport.in: only try to turn on case-sensitivity when
 we create workdir

Only try to turn on case-sensitivity when we create workdir: this avoids
grovelling over all the files on subsequent runs.

Make sure to check for case-sensitivity required on subsequent runs,
though.
---
 bin/cygport.in | 33 ++++++++++++++++++++++++---------
 1 file changed, 24 insertions(+), 9 deletions(-)

diff --git a/bin/cygport.in b/bin/cygport.in
index 1791e5b3..3f89ac67 100755
--- a/bin/cygport.in
+++ b/bin/cygport.in
@@ -573,18 +573,34 @@ declare -ar pkg_name=(${PKG_NAMES:-${PN}});
 declare -r  pkg_count=${#pkg_name[*]};
 
 # this requires workdir to be already defined
-mkdir -p ${workdir}
 
-if [ $OSTYPE = "cygwin" ]
+__probe_case_sensitivity() {
+	mkdir -p ${workdir}
+	rm -f ${workdir}/.probe_case_sensitivity
+	touch ${workdir}/.PROBE_CASE_SENSITIVITY
+	local probe=$( [ -f ${workdir}/.probe_case_sensitivity ] && echo 1 || echo 0 )
+	rm -f ${workdir}/.PROBE_CASE_SENSITIVITY
+	return $probe
+}
+
+__enable_case_sensitivity() {
+	if [ $OSTYPE = "cygwin" ]
+	then
+		inform "Trying to enable case sensitivity on ${workdir}"
+		chattr -fR +C ${workdir} >/dev/null 2>&1 || true
+	fi
+}
+
+# create workdir if it doesn't already exist, and try to enable case-sensitivity
+# if it appears to be absent
+if [ ! -d ${workdir} ]
 then
-	chattr -fR +C ${workdir} >/dev/null 2>&1 || true
+	mkdir -p ${workdir}
+	__probe_case_sensitivity || __enable_case_sensitivity
 fi
 
-rm -f ${workdir}/.probe_case_sensitivity
-touch ${workdir}/.PROBE_CASE_SENSITIVITY
-probe=$( [ -f ${workdir}/.probe_case_sensitivity ] && echo "failed" || echo "" )
-rm -f ${workdir}/.PROBE_CASE_SENSITIVITY
-if [ -n "$probe" ]
+# in any case, probe for case sensitivity, stop if required but missing
+if ! __probe_case_sensitivity
 then
 	if defined _CYGPORT_RESTRICT_case_insensitive_
 	then
@@ -593,7 +609,6 @@ then
 		warning "Building on a case-insensitive filesystem";
 	fi
 fi
-unset probe
 
 # this requires S and B to be already defined
 if ! defined _CYGPORT_RESTRICT_debuginfo_
-- 
2.39.0

Re: chattr makes cygport slow

[ASSI]

Jon Turney via Cygwin-apps writes:
> The warning (error if RESTRICT=case_insensitive) should occur for all
> commands, not just prep.

OK.

> How about the attached.

Looks promising.

> Even then, it should only be modifying every directory, not every file.

Run strace on "chattr -r +C ." and see that it isn't that smart…

Anyway, since it won't succeed on anything but empty directories it
doesn't make sense to use to attempt a recursive change even if it were
just looking at directories (which would of course be way faster at
least).


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

SD adaptations for KORG EX-800 and Poly-800MkII V0.9:
http://Synth.Stromeko.net/Downloads.html#KorgSDada

Re: chattr makes cygport slow

[Jon Turney]

On 16/07/2023 20:32, ASSI via Cygwin-apps wrote:
> Jon Turney via Cygwin-apps writes:
>> The warning (error if RESTRICT=case_insensitive) should occur for all
>> commands, not just prep.
> 
> OK.
> 
>> How about the attached.
> 
> Looks promising.

Ok.  I made a cygport 0.36.6 release with this change.

Please let me know if it makes things better (or worse!)?

Re: chattr makes cygport slow

[ASSI]

Jon Turney via Cygwin-apps writes:
> Ok.  I made a cygport 0.36.6 release with this change.

Thanks.  You haven't tagged that release on GitHub yet it seems.


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

Waldorf MIDI Implementation & additional documentation:
http://Synth.Stromeko.net/Downloads.html#WaldorfDocs

Re: chattr makes cygport slow

[Jon Turney]

On 23/07/2023 20:13, ASSI via Cygwin-apps wrote:
> Jon Turney via Cygwin-apps writes:
>> Ok.  I made a cygport 0.36.6 release with this change.
> 
> Thanks.  You haven't tagged that release on GitHub yet it seems.

The authoritative upstream is 
https://cygwin.com/git/cygwin-apps/cygport.git, which that is just a 
mirror of.

Thanks for pointing out this oversight, should be fixed now.

Re: chattr makes cygport slow

[Andrew Schulman]

> On 16/07/2023 20:32, ASSI via Cygwin-apps wrote:
> > Jon Turney via Cygwin-apps writes:
> >> The warning (error if RESTRICT=case_insensitive) should occur for all
> >> commands, not just prep.
> > 
> > OK.
> > 
> >> How about the attached.
> > 
> > Looks promising.
> 
> Ok.  I made a cygport 0.36.6 release with this change.
> 
> Please let me know if it makes things better (or worse!)?

So much better. Thank you.
Andrew