* [U-Boot] [PATCH v2] tools: buildman: Don't use the working dir as build dir
@ 2018-04-02 8:43 Simon Glass
2018-04-03 8:16 ` Lothar Waßmann
0 siblings, 1 reply; 3+ messages in thread
From: Simon Glass @ 2018-04-02 8:43 UTC (permalink / raw)
To: u-boot
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(-)
diff --git a/tools/buildman/builderthread.py b/tools/buildman/builderthread.py
index 9ac101a5a4..16a3fa452d 100644
--- a/tools/buildman/builderthread.py
+++ b/tools/buildman/builderthread.py
@@ -7,6 +7,7 @@ import errno
import glob
import os
import shutil
+import sys
import threading
import command
@@ -27,6 +28,9 @@ def Mkdir(dirname, parents = False):
os.mkdir(dirname)
except OSError as err:
if err.errno == errno.EEXIST:
+ if os.path.realpath('.') == os.path.realpath(dirname):
+ print "Cannot create the current working directory '%s'!" % dirname
+ sys.exit(1)
pass
else:
raise
diff --git a/tools/buildman/control.py b/tools/buildman/control.py
index 3cac9f7cf6..5bf128357d 100644
--- a/tools/buildman/control.py
+++ b/tools/buildman/control.py
@@ -81,6 +81,28 @@ def ShowActions(series, why_selected, boards_selected, builder, options):
print ('Total boards to build for each commit: %d\n' %
len(why_selected['all']))
+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))
+ sys.exit(1)
+ parent = os.path.dirname(path)
+ if parent == path:
+ break
+ path = parent
+
def DoBuildman(options, args, toolchains=None, make_func=None, boards=None,
clean_dir=False):
"""The main control code for buildman
@@ -252,9 +274,9 @@ def DoBuildman(options, args, toolchains=None, make_func=None, boards=None,
# output directory itself rather than any subdirectory.
if not options.no_subdirs:
output_dir = os.path.join(options.output_dir, dirname)
- if (clean_dir and output_dir != options.output_dir and
- os.path.exists(output_dir)):
- shutil.rmtree(output_dir)
+ if clean_dir and os.path.exists(output_dir):
+ shutil.rmtree(output_dir)
+ CheckOutputDir(output_dir)
builder = Builder(toolchains, output_dir, options.git_dir,
options.threads, options.jobs, gnu_make=gnu_make, checkout=True,
show_unknown=options.show_unknown, step=options.step,
diff --git a/tools/buildman/func_test.py b/tools/buildman/func_test.py
index eec0f9bd37..4df1679842 100644
--- a/tools/buildman/func_test.py
+++ b/tools/buildman/func_test.py
@@ -521,3 +521,12 @@ class TestFunctional(unittest.TestCase):
self._RunControl('-b', self._test_branch, clean_dir=False)
self.assertEqual(self._builder.count, self._total_builds)
self.assertEqual(self._builder.fail, 0)
+
+ def testBadOutputDir(self):
+ """Test building with an output dir the same as out current dir"""
+ self._test_branch = '/__dev/__testbranch'
+ with self.assertRaises(SystemExit):
+ self._RunControl('-b', self._test_branch, '-o', os.getcwd())
+ with self.assertRaises(SystemExit):
+ self._RunControl('-b', self._test_branch, '-o',
+ os.path.join(os.getcwd(), 'test'))
--
2.17.0.rc1.321.gba9d0f2565-goog
^ permalink raw reply related [flat|nested] 3+ messages in thread
* [U-Boot] [PATCH v2] tools: buildman: Don't use the working dir as build dir
2018-04-02 8:43 [U-Boot] [PATCH v2] tools: buildman: Don't use the working dir as build dir Simon Glass
@ 2018-04-03 8:16 ` Lothar Waßmann
2018-04-08 11:15 ` Simon Glass
0 siblings, 1 reply; 3+ messages in thread
From: Lothar Waßmann @ 2018-04-03 8:16 UTC (permalink / raw)
To: u-boot
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(-)
>
[...]
> diff --git a/tools/buildman/control.py b/tools/buildman/control.py
> index 3cac9f7cf6..5bf128357d 100644
> --- a/tools/buildman/control.py
> +++ b/tools/buildman/control.py
> @@ -81,6 +81,28 @@ def ShowActions(series, why_selected, boards_selected, builder, options):
> print ('Total boards to build for each commit: %d\n' %
> len(why_selected['all']))
>
> +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>
Lothar Waßmann
^ permalink raw reply [flat|nested] 3+ messages in thread
* [U-Boot] [PATCH v2] tools: buildman: Don't use the working dir as build dir
2018-04-03 8:16 ` Lothar Waßmann
@ 2018-04-08 11:15 ` Simon Glass
0 siblings, 0 replies; 3+ messages in thread
From: Simon Glass @ 2018-04-08 11:15 UTC (permalink / raw)
To: u-boot
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
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2018-04-08 11:15 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-04-02 8:43 [U-Boot] [PATCH v2] tools: buildman: Don't use the working dir as build dir Simon Glass
2018-04-03 8:16 ` Lothar Waßmann
2018-04-08 11:15 ` Simon Glass
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.