Skip to content

Commit

Permalink
Fix mkdir params to work on OS X as well as Linux
Browse files Browse the repository at this point in the history
  • Loading branch information
glennmatthews committed Apr 3, 2017
1 parent 3db703c commit f70b912
Show file tree
Hide file tree
Showing 5 changed files with 16 additions and 13 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.rst
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ This project adheres to `Semantic Versioning`_.
- Fixed issue where UnboundLocalError would be raised during COT's
attempt to clean up after a qemu-img error occurring while trying to
convert a disk to VMDK (`#67`_).
- Fixed incorrect invocation of 'sudo mkdir' on Mac OS X.

`2.0.2`_ - 2017-03-20
---------------------
Expand Down
6 changes: 4 additions & 2 deletions COT/helpers/helper.py
Original file line number Diff line number Diff line change
Expand Up @@ -466,7 +466,8 @@ def mkdir(directory, permissions=493): # 493 == 0o755
directory)
try:
check_call(['sudo', 'mkdir', '-p',
'--mode=%o' % permissions,
# We previously used '--mode' but OS X lacks it.
'-m', '%o' % permissions,
directory])
except HelperError:
# That failed too - re-raise the original exception
Expand Down Expand Up @@ -588,7 +589,8 @@ def check_call(args, require_success=True, retry_with_sudo=False, **kwargs):
# to:
# ENOEXEC "Exec format error"
# We shouldn't see ENOEXEC otherwise, so we special case this.
if exc.errno == errno.ENOEXEC and args[0] == 'sudo': # pragma: no cover
if (exc.errno == errno.ENOEXEC and # pragma: no cover
args[0] == 'sudo'):
raise HelperError(exc.errno, "The 'sudo' command is unavailable")
if exc.errno != errno.ENOENT:
raise
Expand Down
6 changes: 3 additions & 3 deletions COT/helpers/tests/test_fatdisk.py
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ def test_install_apt_get(self,
['apt-get', '-q', 'install', 'make'],
['apt-get', '-q', 'install', 'gcc'],
['./RUNME'],
['sudo', 'mkdir', '-p', '--mode=755', '/usr/local/bin'],
['sudo', 'mkdir', '-p', '-m', '755', '/usr/local/bin'],
])
self.assertTrue(re.search("/fatdisk$", mock_copy.call_args[0][0]))
self.assertEqual('/usr/local/bin', mock_copy.call_args[0][1])
Expand All @@ -119,7 +119,7 @@ def test_install_apt_get(self,
mock_check_call,
[
['./RUNME'],
['sudo', 'mkdir', '-p', '--mode=755',
['sudo', 'mkdir', '-p', '-m', '755',
'/home/cot/opt/local/bin'],
])
self.assertTrue(re.search("/fatdisk$", mock_copy.call_args[0][0]))
Expand Down Expand Up @@ -156,7 +156,7 @@ def test_install_yum(self,
['yum', '--quiet', 'install', 'make'],
['yum', '--quiet', 'install', 'gcc'],
['./RUNME'],
['sudo', 'mkdir', '-p', '--mode=755', '/usr/local/bin'],
['sudo', 'mkdir', '-p', '-m', '755', '/usr/local/bin'],
])
self.assertTrue(re.search("/fatdisk$", mock_copy.call_args[0][0]))
self.assertEqual('/usr/local/bin', mock_copy.call_args[0][1])
Expand Down
4 changes: 2 additions & 2 deletions COT/helpers/tests/test_helper.py
Original file line number Diff line number Diff line change
Expand Up @@ -500,7 +500,7 @@ def test_need_sudo(self, mock_isdir, mock_exists,
mock_exists.assert_called_with('/foo/bar')
mock_makedirs.assert_called_with('/foo/bar', 493) # 493 == 0o755
mock_check_call.assert_called_with(
['sudo', 'mkdir', '-p', '--mode=755', '/foo/bar'])
['sudo', 'mkdir', '-p', '-m', '755', '/foo/bar'])

def test_nondefault_permissions(self, mock_isdir, mock_exists,
mock_makedirs, mock_check_call):
Expand All @@ -518,7 +518,7 @@ def test_nondefault_permissions(self, mock_isdir, mock_exists,
self.assertTrue(Helper.mkdir('/foo/bar', 511)) # 511 == 0o777
mock_makedirs.assert_called_with('/foo/bar', 511)
mock_check_call.assert_called_with(
['sudo', 'mkdir', '-p', '--mode=777', '/foo/bar'])
['sudo', 'mkdir', '-p', '-m', '777', '/foo/bar'])


@mock.patch('COT.helpers.helper.check_call')
Expand Down
12 changes: 6 additions & 6 deletions COT/helpers/tests/test_vmdktool.py
Original file line number Diff line number Diff line change
Expand Up @@ -84,8 +84,8 @@ def test_install_helper_apt_get(self,
['apt-get', '-q', 'install', 'make'],
['apt-get', '-q', 'install', 'zlib1g-dev'],
['make', 'CFLAGS="-D_GNU_SOURCE -g -O -pipe"'],
['sudo', 'mkdir', '-p', '--mode=755', '/usr/local/man/man8'],
['sudo', 'mkdir', '-p', '--mode=755', '/usr/local/bin'],
['sudo', 'mkdir', '-p', '-m', '755', '/usr/local/man/man8'],
['sudo', 'mkdir', '-p', '-m', '755', '/usr/local/bin'],
['make', 'install', 'PREFIX=/usr/local'],
])
self.assertAptUpdated()
Expand All @@ -111,9 +111,9 @@ def test_install_helper_apt_get(self,
mock_check_call,
[
['make', 'CFLAGS="-D_GNU_SOURCE -g -O -pipe"'],
['sudo', 'mkdir', '-p', '--mode=755',
['sudo', 'mkdir', '-p', '-m', '755',
'/home/cot/opt/local/man/man8'],
['sudo', 'mkdir', '-p', '--mode=755',
['sudo', 'mkdir', '-p', '-m', '755',
'/home/cot/opt/local/bin'],
['make', 'install', 'PREFIX=/opt/local', 'DESTDIR=/home/cot'],
])
Expand Down Expand Up @@ -146,8 +146,8 @@ def test_install_helper_yum(self,
['yum', '--quiet', 'install', 'make'],
['yum', '--quiet', 'install', 'zlib-devel'],
['make', 'CFLAGS="-D_GNU_SOURCE -g -O -pipe"'],
['sudo', 'mkdir', '-p', '--mode=755', '/usr/local/man/man8'],
['sudo', 'mkdir', '-p', '--mode=755', '/usr/local/bin'],
['sudo', 'mkdir', '-p', '-m', '755', '/usr/local/man/man8'],
['sudo', 'mkdir', '-p', '-m', '755', '/usr/local/bin'],
['make', 'install', 'PREFIX=/usr/local'],
])

Expand Down

0 comments on commit f70b912

Please sign in to comment.