# bug: stgit doesn't handle branch names with / in them 11 messages from 2006-02-14 to 2006-04-18. Participants: Karl Hasselström, Sam Vilain, Catalin Marinas, Yann Dirson. Thread: https://gitlist.dev/t/3336 ## Karl Hasselström, 2006-02-14 17:35 Subject: bug: stgit doesn't handle branch names with / in them Message-ID: <20060214173509.GA8666@diana.vm.bytemark.co.uk> URL: https://gitlist.dev/e/20060214173509.GA8666%40diana.vm.bytemark.co.uk ``` "stg branch -l" only shows branches directly under refs/heads, and completely ignores branches in subdirectories. But it does print the name of the subdirectories . . . "stg branch" prints only the part of the branch name following the final slash. $ git-branch kha/abc123 kha/dab * kha/patches kha/powerup kha/dare kha/dare-build kha/boo123 kha/lisp106 local/test0 local/test3 local/test4 SVN-import test5 $ stg branch -l Available branches: SVN-import | kha | local | test5 | $ stg branch patches No patch (yet), sorry. -- Karl Hasselström, kha@treskal.com www.treskal.com/kalle ``` ## Karl Hasselström, 2006-02-17 01:41 Subject: [PATCH] Handle branch names with slashes Message-ID: <20060217014117.12525.21330.stgit@backpacker.hemma.treskal.com> URL: https://gitlist.dev/e/20060217014117.12525.21330.stgit%40backpacker.hemma.treskal.com In-Reply-To: <20060214173509.GA8666@diana.vm.bytemark.co.uk> ``` Let StGIT grok branch names with slashes in them. It used to fall flat on its face when confronted with them. I think I've covered all, or at least most cases, but there are probably some bugs left if you look hard enough. Signed-off-by: Karl Hasselström --- stgit/commands/branch.py | 6 +++- stgit/git.py | 12 +++++-- stgit/stack.py | 42 ++++++++++--------------- stgit/utils.py | 77 ++++++++++++++++++++++++++++++++++++++++------ 4 files changed, 97 insertions(+), 40 deletions(-) diff --git a/stgit/commands/branch.py b/stgit/commands/branch.py index ef44349..d3e8a3c 100644 --- a/stgit/commands/branch.py +++ b/stgit/commands/branch.py @@ -173,7 +173,11 @@ def func(parser, options, args): if len(args) != 0: parser.error('incorrect number of arguments') - branches = os.listdir(os.path.join(git.get_base_dir(), 'refs', 'heads')) + branches = [] + basepath = os.path.join(git.get_base_dir(), 'refs', 'heads') + for path, dirs, files in os.walk(basepath): + branches += [remove_leading_dir(basepath, os.path.join(path, f)) + for f in files] branches.sort() max_len = max([len(i) for i in branches]) diff --git a/stgit/git.py b/stgit/git.py index 582e803..724b6fd 100644 --- a/stgit/git.py +++ b/stgit/git.py @@ -232,7 +232,8 @@ def get_head(): def get_head_file(): """Returns the name of the file pointed to by the HEAD link """ - return os.path.basename(_output_one_line('git-symbolic-ref HEAD')) + return remove_leading_dir(os.path.join('refs', 'heads'), + _output_one_line('git-symbolic-ref HEAD')) def set_head_file(ref): """Resets HEAD to point to a new ref @@ -325,7 +326,9 @@ def delete_branch(name): branch_head = os.path.join('refs', 'heads', name) if not branch_exists(branch_head): raise GitException, 'Branch "%s" does not exist' % name - os.remove(os.path.join(get_base_dir(), branch_head)) + base = get_base_dir() + rm_file_and_dirs(os.path.join(base, branch_head), + os.path.join(base, 'refs', 'heads')) def rename_branch(from_name, to_name): """Rename a git branch @@ -339,8 +342,9 @@ def rename_branch(from_name, to_name): if get_head_file() == from_name: set_head_file(to_head) - os.rename(os.path.join(get_base_dir(), from_head), \ - os.path.join(get_base_dir(), to_head)) + base = os.path.join(get_base_dir()) + rename_dirs(os.path.join(base, from_head), os.path.join(base, to_head), + os.path.join(base, 'refs', 'heads')) def add(names): """Add the files or recursively add the directory contents diff --git a/stgit/stack.py b/stgit/stack.py index 556c40e..68a2936 100644 --- a/stgit/stack.py +++ b/stgit/stack.py @@ -406,7 +406,7 @@ class Series: """ if len(self.get_applied()) == 0: head = git.get_head() - write_string(self.__base_file, head) + write_string(self.__base_file, head, mkdir = True) def __end_stack_check(self): """Remove .git/refs/heads/base if the stack is empty. @@ -499,9 +499,11 @@ class Series: git.rename_branch(self.__name, to_name) if os.path.isdir(self.__series_dir): - os.rename(self.__series_dir, to_stack.__series_dir) + rename_dirs(self.__series_dir, to_stack.__series_dir, + os.path.join(self.__base_dir, 'patches')) if os.path.exists(self.__base_file): - os.rename(self.__base_file, to_stack.__base_file) + rename_dirs(self.__base_file, to_stack.__base_file, + os.path.join(self.__base_dir, 'refs', 'bases')) self.__init__(to_name) @@ -543,29 +545,19 @@ class Series: for p in patches: Patch(p, self.__patch_dir, self.__refs_dir).delete() - if os.path.exists(self.__applied_file): - os.remove(self.__applied_file) - if os.path.exists(self.__unapplied_file): - os.remove(self.__unapplied_file) - if os.path.exists(self.__current_file): - os.remove(self.__current_file) - if os.path.exists(self.__descr_file): - os.remove(self.__descr_file) - if not os.listdir(self.__patch_dir): - os.rmdir(self.__patch_dir) - else: - print 'Patch directory %s is not empty.' % self.__name - if not os.listdir(self.__series_dir): - os.rmdir(self.__series_dir) - else: - print 'Series directory %s is not empty.' % self.__name - if not os.listdir(self.__refs_dir): - os.rmdir(self.__refs_dir) - else: - print 'Refs directory %s is not empty.' % self.__refs_dir + for f in [self.__applied_file, self.__unapplied_file, + self.__current_file, self.__descr_file]: + rm_if_exists(f) + + for (d, n) in [(self.__patch_dir, 'Patch'), + (self.__series_dir, 'Series'), + (self.__refs_dir, 'Refs')]: + if os.path.isdir(d) and os.listdir(d): + print '%s directory %s is not empty.' % (n, self.__name) + rmdir_while_empty(d, self.__base_dir) - if os.path.exists(self.__base_file): - os.remove(self.__base_file) + rm_if_exists(self.__base_file) + rmdir_while_empty(os.path.dirname(self.__base_file), self.__base_dir) def refresh_patch(self, files = None, message = None, edit = False, show_patch = False, diff --git a/stgit/utils.py b/stgit/utils.py index 5749b3b..33c62be 100644 --- a/stgit/utils.py +++ b/stgit/utils.py @@ -18,6 +18,18 @@ along with this program; if not, write t Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA """ +import os.path + +def mkdir_file(filename, mode, mkdir): + """Opens filename with the given mode, creating the directory it's + in if it doesn't already exist and mkdir is true + """ + if mkdir: + d = os.path.dirname(filename) + if not os.path.isdir(d): + os.makedirs(d) + return file(filename, mode) + def read_string(filename, multiline = False): """Reads the first line from a file """ @@ -29,42 +41,87 @@ def read_string(filename, multiline = Fa f.close() return result -def write_string(filename, line, multiline = False): +def write_string(filename, line, multiline = False, mkdir = False): """Writes 'line' to file and truncates it """ - f = file(filename, 'w+') + f = mkdir_file(filename, 'w+', mkdir) if multiline: f.write(line) else: print >> f, line f.close() -def append_strings(filename, lines): +def append_strings(filename, lines, mkdir = False): """Appends 'lines' sequence to file """ - f = file(filename, 'a+') + f = mkdir_file(filename, 'a+', mkdir) for line in lines: print >> f, line f.close() -def append_string(filename, line): +def append_string(filename, line, mkdir = False): """Appends 'line' to file """ - f = file(filename, 'a+') + f = mkdir_file(filename, 'a+', mkdir) print >> f, line f.close() -def insert_string(filename, line): +def insert_string(filename, line, mkdir = False): """Inserts 'line' at the beginning of the file """ - f = file(filename, 'r+') + f = mkdir_file(filename, 'r+', mkdir) lines = f.readlines() f.seek(0); f.truncate() print >> f, line f.writelines(lines) f.close() -def create_empty_file(name): +def create_empty_file(name, mkdir = False): """Creates an empty file """ - file(name, 'w+').close() + mkdir_file(name, 'w+', mkdir).close() + +def remove_leading_dir(leading, path): + """Remove leading directories from a pathname + """ + if not path.startswith(leading): + raise Exception('"%s" does not begin with "%s"' % (path, leading)) + path = path[len(leading):] + if len(path) > 0 and path[0] in [os.path.sep, os.path.altsep]: + path = path[1:] + return path + +def rmdir_while_empty(path, stop): + """Delete dirs until we reach a directory that isn't empty, or + until we reach the path stop + """ + while path.startswith(stop) and len(path) > len(stop): + parent = os.path.dirname(path) + try: + os.rmdir(path) + except OSError: + return # directory not empty + path = parent + +def rm_file_and_dirs(path, stop): + """Delete the file, and keep deleting dirs until we reach a + directory that isn't empty, or until we reach the path stop + """ + os.remove(path) + rmdir_while_empty(os.path.dirname(path), stop) + +def rm_if_exists(f): + """Delete file if it exists + """ + if os.path.exists(f): + os.remove(f) + +def rename_dirs(from_path, to_path, stop): + """Rename file or directory, creating new directories at to_path + as necessary, and removing leftover empty directories at from_path + until we reach stop + """ + if not os.path.isdir(os.path.dirname(to_path)): + os.makedirs(os.path.dirname(to_path)) + os.rename(from_path, to_path) + rmdir_while_empty(os.path.dirname(from_path), stop) ``` ## Sam Vilain, 2006-02-17 03:01 Subject: Re: [PATCH] Handle branch names with slashes Message-ID: <43F53C76.6080902@vilain.net> URL: https://gitlist.dev/e/43F53C76.6080902%40vilain.net In-Reply-To: <20060217014117.12525.21330.stgit@backpacker.hemma.treskal.com> ``` Karl Hasselström wrote: > Let StGIT grok branch names with slashes in them. It used to fall flat > on its face when confronted with them. > > I think I've covered all, or at least most cases, but there are > probably some bugs left if you look hard enough. Does `stgit -r patchname/bottom` still work? Sam. ``` ## Karl Hasselström, 2006-02-17 04:21 Subject: Re: [PATCH] Handle branch names with slashes Message-ID: <20060217042108.GB28114@diana.vm.bytemark.co.uk> URL: https://gitlist.dev/e/20060217042108.GB28114%40diana.vm.bytemark.co.uk In-Reply-To: <43F53C76.6080902@vilain.net> ``` On 2006-02-17 16:01:10 +1300, Sam Vilain wrote: > Karl Hasselström wrote: > > > Let StGIT grok branch names with slashes in them. It used to fall > > flat on its face when confronted with them. > > > > I think I've covered all, or at least most cases, but there are > > probably some bugs left if you look hard enough. > > Does `stgit -r patchname/bottom` still work? Yes (if you mean 'stg diff -r ... ' :-). It's just branches that can have slashes in their names, not patches. -- Karl Hasselström, kha@treskal.com www.treskal.com/kalle ``` ## Catalin Marinas, 2006-02-17 09:47 Subject: Re: [PATCH] Handle branch names with slashes Message-ID: URL: https://gitlist.dev/e/tnxbqx6z592.fsf%40arm.com In-Reply-To: <20060217014117.12525.21330.stgit@backpacker.hemma.treskal.com> ``` Karl Hasselström wrote: > Let StGIT grok branch names with slashes in them. It used to fall flat > on its face when confronted with them. Thanks for the patches you sent. I'll have a look at them tomorrow. As a side note, for future patches, could you please use my catalin.marinas@gmail.com address instead of the company one? I maintain StGIT outside the working hours and it's much easier to grab them from my personal address. Thanks, -- Catalin ``` ## Karl Hasselström, 2006-02-17 09:53 Subject: Re: [PATCH] Handle branch names with slashes Message-ID: <20060217095323.GA1188@diana.vm.bytemark.co.uk> URL: https://gitlist.dev/e/20060217095323.GA1188%40diana.vm.bytemark.co.uk In-Reply-To: ``` On 2006-02-17 09:47:21 +0000, Catalin Marinas wrote: > Karl Hasselström wrote: > > > Let StGIT grok branch names with slashes in them. It used to fall > > flat on its face when confronted with them. > > Thanks for the patches you sent. I'll have a look at them tomorrow. > > As a side note, for future patches, could you please use my > catalin.marinas@gmail.com address instead of the company one? I > maintain StGIT outside the working hours and it's much easier to > grab them from my personal address. Ah, I hadn't realized you were using more than one address. Will do. -- Karl Hasselström, kha@treskal.com www.treskal.com/kalle ``` ## Karl Hasselström, 2006-02-27 12:11 Subject: Re: [PATCH] Handle branch names with slashes Message-ID: <20060227121108.GA22398@diana.vm.bytemark.co.uk> URL: https://gitlist.dev/e/20060227121108.GA22398%40diana.vm.bytemark.co.uk In-Reply-To: <20060217042108.GB28114@diana.vm.bytemark.co.uk> ``` On 2006-02-17 05:21:08 +0100, Karl Hasselström wrote: > On 2006-02-17 16:01:10 +1300, Sam Vilain wrote: > > > Karl Hasselström wrote: > > > > > Let StGIT grok branch names with slashes in them. It used to > > > fall flat on its face when confronted with them. > > > > > > I think I've covered all, or at least most cases, but there are > > > probably some bugs left if you look hard enough. > > > > Does `stgit -r patchname/bottom` still work? > > Yes (if you mean 'stg diff -r ... ' :-). It's just branches that can > have slashes in their names, not patches. There was a bug here after all: I just tried "stg pick multi@kha/patches" (to pick a patch named "multi" from the branch "kha/patches"), and StGIT tried to pick the patch from branch "kha". Looking closer, I realized that the complete patch specification syntax is "patchname@branchname/bottom", not "patchname/bottom@branchname" as I had assumed. This is obviously hard to reconcile with branch names containing /. Thoughts? -- Karl Hasselström, kha@treskal.com www.treskal.com/kalle ``` ## Catalin Marinas, 2006-02-27 12:29 Subject: Re: [PATCH] Handle branch names with slashes Message-ID: <1141043391.3438.66.camel@pc1117> URL: https://gitlist.dev/e/1141043391.3438.66.camel%40pc1117 In-Reply-To: <20060227121108.GA22398@diana.vm.bytemark.co.uk> ``` On Mon, 2006-02-27 at 13:11 +0100, Karl Hasselström wrote: > There was a bug here after all: I just tried "stg pick > multi@kha/patches" (to pick a patch named "multi" from the branch > "kha/patches"), and StGIT tried to pick the patch from branch "kha". I haven't applied your patch yet (too busy to properly review it). > Looking closer, I realized that the complete patch specification > syntax is "patchname@branchname/bottom", not > "patchname/bottom@branchname" as I had assumed. This is obviously hard > to reconcile with branch names containing /. I don't have any strong opinion on either. Maybe we should use the latter if it makes things easier for supporting branch names with /'s. -- Catalin ``` ## Karl Hasselström, 2006-02-27 13:09 Subject: Re: [PATCH] Handle branch names with slashes Message-ID: <20060227130917.GA22933@diana.vm.bytemark.co.uk> URL: https://gitlist.dev/e/20060227130917.GA22933%40diana.vm.bytemark.co.uk In-Reply-To: <1141043391.3438.66.camel@pc1117> ``` On 2006-02-27 12:29:51 +0000, Catalin Marinas wrote: > On Mon, 2006-02-27 at 13:11 +0100, Karl Hasselström wrote: > > > There was a bug here after all: I just tried "stg pick > > multi@kha/patches" (to pick a patch named "multi" from the branch > > "kha/patches"), and StGIT tried to pick the patch from branch > > "kha". > > I haven't applied your patch yet (too busy to properly review it). And as I just demonstrated, it certainly needed reviewing! (Actually, I believe I said that back when I posted the patch, too.) > > Looking closer, I realized that the complete patch specification > > syntax is "patchname@branchname/bottom", not > > "patchname/bottom@branchname" as I had assumed. This is obviously > > hard to reconcile with branch names containing /. > > I don't have any strong opinion on either. Maybe we should use the > latter if it makes things easier for supporting branch names with > /'s. The problem is that the current from is better (bottom is a modifier to patch@branch, not just patch). And using the other form will break when someone decides that patches with slashes in their names are a good idea (not a joke). Perhaps change /bottom to #bottom (making the complete form patchname@branchname#bottom), and for backward compatibility accept patchname@branchname/bottom as well when no branch called "branchname/bottom" exists. -- Karl Hasselström, kha@treskal.com www.treskal.com/kalle ``` ## Yann Dirson, 2006-04-12 19:16 Subject: Re: [PATCH] Handle branch names with slashes Message-ID: <20060412191655.GD27397@nowhere.earth> URL: https://gitlist.dev/e/20060412191655.GD27397%40nowhere.earth In-Reply-To: ``` On Fri, Feb 17, 2006 at 09:47:21AM +0000, Catalin Marinas wrote: > Karl Hasselström wrote: > > Let StGIT grok branch names with slashes in them. It used to fall flat > > on its face when confronted with them. > > Thanks for the patches you sent. I'll have a look at them tomorrow. Is this still being worked on ? -- Yann Dirson | Debian-related: | Support Debian GNU/Linux: | Freedom, Power, Stability, Gratis http://ydirson.free.fr/ | Check ``` ## Karl Hasselström, 2006-04-18 11:07 Subject: Re: [PATCH] Handle branch names with slashes Message-ID: <20060418110709.GA25558@diana.vm.bytemark.co.uk> URL: https://gitlist.dev/e/20060418110709.GA25558%40diana.vm.bytemark.co.uk In-Reply-To: <20060412191655.GD27397@nowhere.earth> ``` On 2006-04-12 21:16:55 +0200, Yann Dirson wrote: > Is this still being worked on? I haven't worked any more on the slashes-in-branch-names patch, but I have been using it myself, and it works for me. Getting that fancy new test suite to excercise it would be a really good next step . . . -- Karl Hasselström, kha@treskal.com www.treskal.com/kalle ```