[PATCH] autobuild: Stop waf uninstall from removing test_tmpdir

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

[PATCH] autobuild: Stop waf uninstall from removing test_tmpdir

Samba - samba-technical mailing list
waf uninstall (via BuildContext.install() in Build.py) removes empty
directories all the way up the directory tree.  This means that it
removes test_tmpdir and any empty directories above it.

While this is arguably a waf bug, the simplest solution is to make
test_tmpdir non-empty so it don't get removed.

Please review and maybe push...

peace & happiness,
martin

0001-autobuild-Stop-waf-uninstall-from-removing-test_tmpd.patch (1K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] autobuild: Stop waf uninstall from removing test_tmpdir

Samba - samba-technical mailing list
On Mon, 20 Mar 2017 14:58:08 +1100, Martin Schwenke via samba-technical
<[hidden email]> wrote:

> waf uninstall (via BuildContext.install() in Build.py) removes empty
> directories all the way up the directory tree.  This means that it
> removes test_tmpdir and any empty directories above it.
>
> While this is arguably a waf bug, the simplest solution is to make
> test_tmpdir non-empty so it don't get removed.
>
> Please review and maybe push...

Reposting with better commit message.  :-)

peace & happiness,
martin

0001-autobuild-Stop-waf-uninstall-from-removing-test_tmpd.patch (1K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] autobuild: Stop waf uninstall from removing test_tmpdir

Samba - samba-technical mailing list
Hi Martin,

>> waf uninstall (via BuildContext.install() in Build.py) removes empty
>> directories all the way up the directory tree.  This means that it
>> removes test_tmpdir and any empty directories above it.

How did you find that?

>> While this is arguably a waf bug, the simplest solution is to make
>> test_tmpdir non-empty so it don't get removed.
>>
>> Please review and maybe push...
>
> Reposting with better commit message.  :-)

Pushed :-)

Thanks!
metze


signature.asc (853 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] autobuild: Stop waf uninstall from removing test_tmpdir

Samba - samba-technical mailing list
Am 20.03.2017 um 09:53 schrieb Stefan Metzmacher via samba-technical:

> Hi Martin,
>
>>> waf uninstall (via BuildContext.install() in Build.py) removes empty
>>> directories all the way up the directory tree.  This means that it
>>> removes test_tmpdir and any empty directories above it.
>
> How did you find that?
>
>>> While this is arguably a waf bug, the simplest solution is to make
>>> test_tmpdir non-empty so it don't get removed.
>>>
>>> Please review and maybe push...
>>
>> Reposting with better commit message.  :-)
>
> Pushed :-)
I opened https://bugzilla.samba.org/show_bug.cgi?id=12703 for the
backport...

metze


signature.asc (853 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] autobuild: Stop waf uninstall from removing test_tmpdir

Samba - samba-technical mailing list
In reply to this post by Samba - samba-technical mailing list
On Monday, 20 March 2017 09:53:08 CET Stefan Metzmacher via samba-technical
wrote:
> Hi Martin,
>
> >> waf uninstall (via BuildContext.install() in Build.py) removes empty
> >> directories all the way up the directory tree.  This means that it
> >> removes test_tmpdir and any empty directories above it.
>
> How did you find that?

One possibility is to use socket_wrapper and wrap rmdir(), then you can can do
something like:

int rmdir(const char *path)
{
        if (strcmp(path, "/tmp") == 0) {
                abort();
        }

        ...
}

Or do something like:

https://blog.cryptomilk.org/2015/03/26/hunting-down-a-fd-closing-bug-in-samba/



        Andreas


--
Andreas Schneider                   GPG-ID: CC014E3D
Samba Team                             [hidden email]
www.samba.org

Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] autobuild: Stop waf uninstall from removing test_tmpdir

Samba - samba-technical mailing list
In reply to this post by Samba - samba-technical mailing list
Hi Metze,

On Mon, 20 Mar 2017 09:53:08 +0100, Stefan Metzmacher <[hidden email]>
wrote:

> >> waf uninstall (via BuildContext.install() in Build.py) removes empty
> >> directories all the way up the directory tree.  This means that it
> >> removes test_tmpdir and any empty directories above it.  
>
> How did you find that?

A lot of patience...  :-)

1. Ran private autobuilds, bisecting the list of default tasks in in
   autobuild.py.

   There's arguably a bug in autobuild.py where it uses a fixed random
   sleep of 1s even if you specify multiple tasks on the command-line.
   We can look at that later.  :-)

   I was able to exclude the big Samba builds and tests quite quickly.

   I finally got down to semi-reliably recreating with only ctdb and
   replace in the list.

2. Couldn't recreate with just ctdb, so went back to ctdb and replace.

   When I did this I hacked CTDB's "make autotest" to only run the
   first test, where it always failed.  That sped things up.  :-)

3. Noticed that the failure was completely reliable when I hacked the
   limits to random-sleep.sh to match the random sleeps in a
   particular failed autobuild. So it became clear that it would fail
   when replace finished before ctdb started.

4. Ran a private autobuild for just replace and confirmed that the tmp
   directory was gone when it completed.

5. Instrumented the replace task in autobuild.py to check if the tmp
   directory still existed after each sub-task.

   Noticed that it was gone after distcheck.

6. Instrumented distcheck() and found the problem was in the recursive
   waf invocation.

7. Split out the uninstall part of the recursive waf invocation, since
   that seemed the most likely suspect.  Confirmed.

8. Looked at BuildContext.install() in Build.py, was amazed and finally
   instrumented the directory removal loop in there to confirm.

   The directory removal loop probably deserves a "break" to optimise
   it.

That was some pretty "big hammer" debugging and it took a while...
but satisfying to track down the culprit. :-)

peace & happiness,
martin

attachment0 (849 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] autobuild: Stop waf uninstall from removing test_tmpdir

Samba - samba-technical mailing list
In reply to this post by Samba - samba-technical mailing list
On Mon, 20 Mar 2017 10:19:49 +0100, Andreas Schneider <[hidden email]>
wrote:

> On Monday, 20 March 2017 09:53:08 CET Stefan Metzmacher via samba-technical
> wrote:
> > Hi Martin,
> >  
>  [...]  
> >
> > How did you find that?  
>
> One possibility is to use socket_wrapper and wrap rmdir(), then you can can do
> something like:
>
> int rmdir(const char *path)
> {
> if (strcmp(path, "/tmp") == 0) {
> abort();
> }
>
> ...
> }

Hmmm.  I never think about socket_wrapper.  That would have been
equivalent to my big hammer chattr idea, without needing root.  It would
have aborted the recursive waf invocation, so would have taken me
straight to distcheck().

Must... remember... to... use... socket_wrapper!  :-)

Thanks!

peace & happiness,
martin