From mboxrd@z Thu Jan  1 00:00:00 1970
From: Simon Glass <sjg@chromium.org>
Date: Sun, 8 Apr 2018 19:15:50 +0800
Subject: [U-Boot] [PATCH v2] tools: buildman: Don't use the working dir
	as build dir
In-Reply-To: <20180403101627.590d375c@karo-electronics.de>
References: <20180402084312.252457-1-sjg@chromium.org>
 <20180403101627.590d375c@karo-electronics.de>
Message-ID: <CAPnjgZ2WXnmbuLGLBYo0joJHBoV0bJOVYY6HJzDm9uZiicHeCg@mail.gmail.com>
List-Id: <u-boot.lists.denx.de>
MIME-Version: 1.0
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: 8bit
To: u-boot@lists.denx.de

Hi Lothar,

On 3 April 2018 at 16:16, Lothar Waßmann <LW@karo-electronics.de> wrote:
> Hi,
>
> On Mon,  2 Apr 2018 02:43:12 -0600 Simon Glass wrote:
>> From: Lothar Waßmann <LW@KARO-electronics.de>
>>
>> When the U-Boot base directory happens to have the same name as the branch
>> that buildman is directed to use via the '-b' option and no output
>> directory is specified with '-o', buildman happily starts removing the
>> whole U-Boot sources eventually only stopped with the error message:
>>
>> OSError: [Errno 20] Not a directory: '../<branch-name>/boards.cfg
>>
>> Add a check to avoid this and also deal with the case where '-o' points
>> to the source directory, or any subdirectory of it.
>>
>> Finally, tidy up the confusing logic for removing the old tree when using
>> -b. This is only done when building a branch.
>>
>> Signed-off-by: Lothar Waßmann <LW@KARO-electronics.de>
>> Signed-off-by: Simon Glass <sjg@chromium.org>
>> ---
>>
>> Changes in v2:
>> - Updated to check directories on start-up as per comments on v1 patch
>> - Added a test
>> - Expanded check to handle subdirectories
>>
>>  tools/buildman/builderthread.py |  4 ++++
>>  tools/buildman/control.py       | 28 +++++++++++++++++++++++++---
>>  tools/buildman/func_test.py     |  9 +++++++++
>>  3 files changed, 38 insertions(+), 3 deletions(-)
>>
> [...]
[...]

>> +def CheckOutputDir(output_dir):
>> +    """Make sure that the output directory is not within the current directory
>> +
>> +    If we try to use an output directory which is within the current directory
>> +    (which is assumed to hold the U-Boot source) we may end up deleting the
>> +    U-Boot source code. Detect this and print an error in this case.
>> +
>> +    Args:
>> +        output_dir: Output directory path to check
>> +    """
>> +    path = os.path.realpath(output_dir)
>> +    cwd_path = os.path.realpath('.')
>> +    while True:
>> +        if os.path.realpath(path) == cwd_path:
>> +            Print("Cannot use output directory '%s' since it is within the "
>> +                  "current directtory '%s'" % (path, cwd_path))
> s/directtory/directory/
> NB: IMO its a bad habit to split format strings across multiple lines,
>     since it makes it harder to grep the source code for a message
>     that was printed on the terminal.
>
> Otherwise, looks good to me.
>
> Tested-by: Lothar Waßmann <LW@KARO-electronics.de>

Yes I agree although for some reason I have tended to stick strictly
to 80 columns in Python. I'm not sure why though, so will change it.

Regards,
Simon