Page MenuHomePhabricator

Fix git-llvm to not delete non-empty directories.
ClosedPublic

Authored by jyknight on Jul 29 2019, 1:54 PM.

Details

Summary

Previously, if a directory contained only other sub-directories, one
of which was being removed, git llvm would delete the parent and all
its subdirs, even though only one should've been deleted.

This error occurred in r366590, where the commit attempted to remove
lldb/packages/Python/lldbsuite/test/tools/lldb-mi, but git-llvm
erroneously removed the entire contents of
lldb/packages/Python/lldbsuite/test/tools.

This happened because "git apply" automatically removes empty
directories locally, and the absence of a local directory was
previously taken as an indication to call 'svn rm' on that
directory. However, an empty local directory does not necessarily
indicate that the directory is truly empty.

Fix that by removing directories only when they're empty on the git
side.

Diff Detail

Repository
rL LLVM

Event Timeline

jyknight created this revision.Jul 29 2019, 1:54 PM
Herald added a project: Restricted Project. · View Herald TranscriptJul 29 2019, 1:54 PM
mehdi_amini accepted this revision.Jul 29 2019, 9:39 PM
mehdi_amini added inline comments.
llvm/utils/git-svn/git-llvm
370 ↗(On Diff #212219)

I assume we can't just check here the status in git instead because the mapping from SVN -> monorepo is not straightforward to get?

This revision is now accepted and ready to land.Jul 29 2019, 9:39 PM
jyknight marked an inline comment as done.Jul 30 2019, 7:34 AM
jyknight added inline comments.
llvm/utils/git-svn/git-llvm
370 ↗(On Diff #212219)

We don't have a reverse mapping at the moment.

But the larger issue is that svn status only reports the toplevel missing directory -- not the subdirs/files under it. So, we'd have to mkdir the toplevel dir, then parse svn status again, to test if we need to recreate or remove the newly-exposed subdirs, and repeat until nothing changes. That seemed a lot more complex.

I also started out thinking I'd remove the svn status parsing in favor of using the git diff status outputs for file addition too. But then realized you cannot 'svn add' files if their containing directory hasn't been added yet, and it's not entirely trivial to determine when to call 'svn add' on the dir. So, in the end I left directory/file addition alone, and only switched over deletion.

This revision was automatically updated to reflect the committed changes.