Merge lp://staging/~gary/charms/precise/juju-gui/update-for-juju-core into lp://staging/~juju-gui/charms/precise/juju-gui/trunk

Proposed by Gary Poster
Status: Merged
Merged at revision: 49
Proposed branch: lp://staging/~gary/charms/precise/juju-gui/update-for-juju-core
Merge into: lp://staging/~juju-gui/charms/precise/juju-gui/trunk
Diff against target: 20 lines (+7/-3)
1 file modified
hooks/install (+7/-3)
To merge this branch: bzr merge lp://staging/~gary/charms/precise/juju-gui/update-for-juju-core
Reviewer Review Type Date Requested Status
charmers Pending
Review via email: mp+161322@code.staging.launchpad.net

Description of the change

Add handling for a case important to Juju Core

Juju Core adds a .empty file to empty directories, which illustrated the fact that the last branch needed to be a bit more defensive.

https://codereview.appspot.com/9002043/

To post a comment you must log in.
Revision history for this message
Gary Poster (gary) wrote :

Reviewers: mp+161322_code.launchpad.net,

Message:
Please take a look.

Description:
Add handling for a case important to Juju Core

Juju Core adds a .empty file to empty directories, which illustrated the
fact that the last branch needed to be a bit more defensive.

I plan to land this trivial but important change immediately.

https://code.launchpad.net/~gary/charms/precise/juju-gui/update-for-juju-core/+merge/161322

(do not edit description out of merge proposal)

Please review this at https://codereview.appspot.com/9002043/

Affected files:
   A [revision details]
   M hooks/install

Index: [revision details]
=== added file '[revision details]'
--- [revision details] 2012-01-01 00:00:00 +0000
+++ [revision details] 2012-01-01 00:00:00 +0000
@@ -0,0 +1,2 @@
+Old revision: <email address hidden>
+New revision: <email address hidden>

Index: hooks/install
=== modified file 'hooks/install'
--- hooks/install 2013-04-28 00:59:29 +0000
+++ hooks/install 2013-04-28 09:57:41 +0000
@@ -51,9 +51,13 @@
              try:
                  bootstrap_utils.run(filename)
              except OSError, e:
- # If the exec.d file does not exist or is not runnable,
- # assume we can recover. Log the problem and proceed.
- if e.errno in (errno.ENOENT, errno.EACCES):
+ # If the exec.d file does not exist or is not runnable or
+ # is not a directory, assume we can recover. Log the
problem
+ # and proceed. Note that Juju Core has a special need of
+ # errno.ENOTDIR because it apparently adds a ".empty" file
in
+ # empty charm directories, so trying to run
+ # ./exec.d/.empty/charm-pre-install will trigger that
error.
+ if e.errno in (errno.ENOENT, errno.EACCES, errno.ENOTDIR):
                      log('{}: {}'.format(e.strerror, filename))
                  else:
                      raise

Revision history for this message
Gary Poster (gary) wrote :

*** Submitted:

Add handling for a case important to Juju Core

Juju Core adds a .empty file to empty directories, which illustrated the
fact that the last branch needed to be a bit more defensive.

R=
CC=
https://codereview.appspot.com/9002043

https://codereview.appspot.com/9002043/

Revision history for this message
Richard Harding (rharding) wrote :

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
The diff is not available at this time. You can reload the page or download it.

Subscribers

People subscribed via source and target branches

to all changes: