GNOME Bugzilla – Bug 694466
JHBuild deletes the whole checkout root if being passed a module with trailing slashes
Last modified: 2013-07-10 14:27:12 UTC
I was trying to build the 'gnome-desktop' module and being in the checkoutroot I hit TAB to complete the module name. This resulted in the followind command being run (note the trailing slash : $ jhbuild buildone gnome-desktop/ Unfortunately Branch.get_module_basename() calls os.path.basename(self.module) and with a trailing slash it returns and empty string that will be joined in Branch.get_checkoutdir() to the checkoutroot. This caused JHBuild to search for .git directory in the checkoutroot, and after failing it prompted me with the usual recovery options. Since I assumed my repo was broken for whatever reason, I choose "wipe and start over": due to the bug above JHBuild promptly deleted all my checkoutroot. :(
Created attachment 237204 [details] [review] Fix handling of trailing slashes on command-line modules names It is currently possible to delete the whole checkoutroot by adding a trailing slash to a module name passed on the command line: • Run `jhbuild buildone gnome-desktop/` • Watch jhbuild error out as it is unable to find the repo • Select 'Wipe directory and start over' (just kidding, *don't* select it!) • Watch jhbuild delete your whole checkout root. This happens because the module name is passed to os.path.basename() which will return an empty string on trailing slashes. JHBuild will then try to operate on the base checkout root, failing to find any proper repo and promptly deleting all its content if told to do so. This patch does two things to prevent this error to occur: • Strip all the trailing slashes from module names in Branch.get_module_basename() • Refuse to accept an empty string for the module basename in Branch.get_checkoutdir()
If noone complains about it, I'd like to push the patch in a week.
Created attachment 244834 [details] [review] Fix handling of trailing slashes on command-line modules names Fixed trailing slashes handling in a couple of other places.
Review of attachment 244834 [details] [review]: One minor comment. Feel free to push after addressing. ::: jhbuild/moduleset.py @@ +72,2 @@ def get_module(self, module_name, ignore_case = False): + module_name = module_name.rstrip('/') You use a literal '/' here... ::: jhbuild/versioncontrol/__init__.py @@ +99,3 @@ def get_module_basename(self): + # prevent basename() from returning empty strings on trailing '/' + module = self.module.rstrip(os.sep) ...and os.sep here. The latter is better, so pick that consistently?
Changed to use os.sep everywhere. Thanks! The following fix has been pushed: 2d12b3a Fix handling of trailing slashes on command-line modules names
Created attachment 248837 [details] [review] Fix handling of trailing slashes on command-line modules names It is currently possible to delete the whole checkoutroot by adding a trailing slash to a module name passed on the command line: • Run `jhbuild buildone gnome-desktop/` • Watch jhbuild error out as it is unable to find the repo • Select 'Wipe directory and start over' (just kidding, *don't* select it!) • Watch jhbuild delete your whole checkout root. This happens because the module name is passed to os.path.basename() which will return an empty string on trailing slashes. JHBuild will then try to operate on the base checkout root, failing to find any proper repo and promptly deleting all its content if told to do so. This patch does two things to prevent this error to occur: • Strip all the trailing slashes from module names in Branch.get_module_basename() • Refuse to accept an empty string for the module basename in Branch.get_checkoutdir() It also tries to strip trailing slashes from module names in a couple of other places, trying to keep stuff working even when being passed one.