Fixed RepositoryTool patching of files in uncreated directories

Review Request #10247 — Created Oct. 19, 2018 and submitted

alextechcc
ReviewBot
master
8d24901...
reviewbot, students

When a RepositoryTool is applying the review's patches to the cloned
repository, it simply writes using open(p, 'wb'). This does not create
directories that the patch implies.

This leads to the write failing with:
IOError: [Errno 2] No such file or directory: u'dir/file'

This patch creates directories for each file if the path does not exist
already.

Replciated the error by posting a review for a file in a new directory,
applied patch, and RepositoryTool successfully runs on patched files.

  • 0
  • 0
  • 12
  • 0
  • 12
Description From Last Updated
Checks run (1 failed, 1 succeeded)
flake8 failed.
JSHint passed.

flake8

alextechcc
Review request changed

Change Summary:

Removed unneccessary import

Commit:

-c89f67ab14f7da66647f24441492effb299a19cd
+68b16e6d2c54cd6f564379b0aced9d654dadacd3

Diff:

Revision 2 (+17 -1)

Show changes

Checks run (1 failed, 1 succeeded)

flake8 failed.
JSHint passed.

flake8

brennie
  1. 
      
  2. bot/reviewbot/tools/__init__.py (Diff revision 2)
     
     
     

    Blank line between these.

  3. bot/reviewbot/utils/filesystem.py (Diff revision 2)
     
     

    This doesn't always create directories -- it ensures they exist, so let's call this ensure_directories.

  4. bot/reviewbot/utils/filesystem.py (Diff revision 2)
     
     

    No space after "

  5. bot/reviewbot/utils/filesystem.py (Diff revision 2)
     
     

    The path for which directories should be created if they don't exist.

  6. bot/reviewbot/utils/filesystem.py (Diff revision 2)
     
     
     

    Blank line between here.

  7. bot/reviewbot/utils/filesystem.py (Diff revision 2)
     
     

    We never want to use bare except. This can catch things that should never be caught, like KeyboardInterrupt or RuntimeError. Instead, this needs to specify the exact exception we expect, which should be OSError.

    1. Should I fix the rest of the instances of this in all related files?

    2. No, that isn't related to your change.

  8. 
      
alextechcc
david
  1. 
      
  2. bot/reviewbot/utils/filesystem.py (Diff revision 3)
     
     

    It seems like we'd probably want to surface the error so that the caller can bail out.

  3. 
      
alextechcc
ammar
  1. 
      
  2. bot/reviewbot/utils/filesystem.py (Diff revision 4)
     
     

    Function name would be better as ensure_directories_exist

  3. 
      
alextechcc
david
  1. Ship It!
  2. 
      
gojeffcho
  1. Ship It!
  2. 
      
alextechcc
Review request changed

Status: Closed (submitted)

Change Summary:

Pushed to master (e092554)
Loading...