{"thread":{"id":"9783","subject":"[PATCH] git.__remotes_from_dir() should only return lists","startedAt":"2007-09-05T16:57:22Z","lastAt":"2007-09-06T23:11:19Z","messageCount":5,"participants":["Pavel Roskin","Karl Hasselström"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"52592","messageId":"20070905165722.17744.56584.stgit@dv.roinet.com","threadId":"9783","inReplyTo":null,"subject":"[PATCH] git.__remotes_from_dir() should only return lists","fromName":"Pavel Roskin","fromEmail":"proski@gnu.org","sentAt":"2007-09-05T16:57:22Z","receivedAt":"2007-09-05T16:57:22Z","isPatch":true,"sender":{"key":"proski@gnu.org","avatar":null},"body":"If there are no remotes, return empty list, not None.  The later doesn't\nwork with builtin set().\n\nThis fixes t1001-branch-rename.sh\n\nSigned-off-by: Pavel Roskin <proski@gnu.org>\n---\n\n stgit/git.py |    2 +-\n 1 files changed, 1 insertions(+), 1 deletions(-)\n\n\ndiff --git a/stgit/git.py b/stgit/git.py\nindex 4b4c626..f847cce 100644\n--- a/stgit/git.py\n+++ b/stgit/git.py\n@@ -953,7 +953,7 @@ def __remotes_from_dir(dir):\n     if os.path.exists(d):\n         return os.listdir(d)\n     else:\n-        return None\n+        return []\n \n def remotes_list():\n     \"\"\"Return the list of remotes in the repository\n"},{"id":"52741","messageId":"20070906112645.GA31888@diana.vm.bytemark.co.uk","threadId":"9783","inReplyTo":"20070905165722.17744.56584.stgit@dv.roinet.com","subject":"Re: [PATCH] git.__remotes_from_dir() should only return lists","fromName":"Karl Hasselström","fromEmail":"kha@treskal.com","sentAt":"2007-09-06T11:26:45Z","receivedAt":"2007-09-06T11:26:45Z","isPatch":true,"sender":{"key":"kha@treskal.com","avatar":"https://gravatar.com/avatar/f0120c734b5279b345075a28521e1ac66acb20c9913ffe9bf6ae97e53f7f3f13?d=mp&s=160"},"body":"On 2007-09-05 12:57:22 -0400, Pavel Roskin wrote:\n\n> If there are no remotes, return empty list, not None. The later\n> doesn't work with builtin set().\n\nThanks. But I guess an even nicer fix would be to make this function\nreturn a set in the first place.\n\n> This fixes t1001-branch-rename.sh\n\nHmm. I don't believe I saw t1001 break without this patch (I run the\ntest suite before I push, but I might have made a mistake of course).\nDoes the user's environment leak into the test sandbox?\n\n-- \nKarl Hasselström, kha@treskal.com\n      www.treskal.com/kalle\n"},{"id":"52744","messageId":"1189082306.3695.5.camel@gx","threadId":"9783","inReplyTo":"20070906112645.GA31888@diana.vm.bytemark.co.uk","subject":"Re: [PATCH] git.__remotes_from_dir() should only return lists","fromName":"Pavel Roskin","fromEmail":"proski@gnu.org","sentAt":"2007-09-06T12:38:26Z","receivedAt":"2007-09-06T12:38:26Z","isPatch":true,"sender":{"key":"proski@gnu.org","avatar":null},"body":"On Thu, 2007-09-06 at 13:26 +0200, Karl Hasselström wrote:\n> On 2007-09-05 12:57:22 -0400, Pavel Roskin wrote:\n> \n> > If there are no remotes, return empty list, not None. The later\n> > doesn't work with builtin set().\n> \n> Thanks. But I guess an even nicer fix would be to make this function\n> return a set in the first place.\n\nFine with me.  But it was returning a list or None, so the simplest fix\nwas to return a list in all cases.\n\n> > This fixes t1001-branch-rename.sh\n> \n> Hmm. I don't believe I saw t1001 break without this patch (I run the\n> test suite before I push, but I might have made a mistake of course).\n> Does the user's environment leak into the test sandbox?\n\nI don't think it's the user environment, at least on my side.  I'm using\nFedora 7, which has python-2.5-12.fc7.  That's the error from the t1001\nbefore my patch:\n\nTraceback (most recent call last):\n  File \"/home/proski/src/stgit/t/../stg\", line 43, in <module>\n    main()\n  File \"/home/proski/src/stgit/stgit/main.py\", line 284, in main\n    command.func(parser, options, args)\n  File \"/home/proski/src/stgit/stgit/commands/branch.py\", line 163, in func\n    parentremote = git.identify_remote(parentbranch)\n  File \"/home/proski/src/stgit/stgit/git.py\", line 994, in identify_remote\n    for remote in remotes_list():\n  File \"/home/proski/src/stgit/stgit/git.py\", line 963, in remotes_list\n    | set(__remotes_from_dir('branches')))\nTypeError: 'NoneType' object is not iterable\n\n-- \nRegards,\nPavel Roskin\n"},{"id":"52754","messageId":"20070906142649.GA2406@diana.vm.bytemark.co.uk","threadId":"9783","inReplyTo":"1189082306.3695.5.camel@gx","subject":"Re: [PATCH] git.__remotes_from_dir() should only return lists","fromName":"Karl Hasselström","fromEmail":"kha@treskal.com","sentAt":"2007-09-06T14:26:49Z","receivedAt":"2007-09-06T14:26:49Z","isPatch":true,"sender":{"key":"kha@treskal.com","avatar":"https://gravatar.com/avatar/f0120c734b5279b345075a28521e1ac66acb20c9913ffe9bf6ae97e53f7f3f13?d=mp&s=160"},"body":"On 2007-09-06 08:38:26 -0400, Pavel Roskin wrote:\n\n> On Thu, 2007-09-06 at 13:26 +0200, Karl Hasselström wrote:\n>\n> > Thanks. But I guess an even nicer fix would be to make this\n> > function return a set in the first place.\n>\n> Fine with me. But it was returning a list or None, so the simplest\n> fix was to return a list in all cases.\n\nOh, your fix is excellent to fix the immediate problem. I was just\ntrying to say that making this function (an a heap of others) return\nsets would be a useful refactoring.\n\n> > Hmm. I don't believe I saw t1001 break without this patch (I run\n> > the test suite before I push, but I might have made a mistake of\n> > course). Does the user's environment leak into the test sandbox?\n>\n> I don't think it's the user environment, at least on my side. I'm\n> using Fedora 7, which has python-2.5-12.fc7. That's the error from\n> the t1001 before my patch:\n\nOK. I'll try to reproduce it when I get home, but it certainly looks\nlike I only _thought_ I'd run the test suite.\n\n-- \nKarl Hasselström, kha@treskal.com\n      www.treskal.com/kalle\n"},{"id":"52813","messageId":"20070906231119.GB11829@diana.vm.bytemark.co.uk","threadId":"9783","inReplyTo":"1189082306.3695.5.camel@gx","subject":"Re: [PATCH] git.__remotes_from_dir() should only return lists","fromName":"Karl Hasselström","fromEmail":"kha@treskal.com","sentAt":"2007-09-06T23:11:19Z","receivedAt":"2007-09-06T23:11:19Z","isPatch":true,"sender":{"key":"kha@treskal.com","avatar":"https://gravatar.com/avatar/f0120c734b5279b345075a28521e1ac66acb20c9913ffe9bf6ae97e53f7f3f13?d=mp&s=160"},"body":"On 2007-09-06 08:38:26 -0400, Pavel Roskin wrote:\n\n> On Thu, 2007-09-06 at 13:26 +0200, Karl Hasselström wrote:\n>\n> > Hmm. I don't believe I saw t1001 break without this patch (I run\n> > the test suite before I push, but I might have made a mistake of\n> > course). Does the user's environment leak into the test sandbox?\n>\n> I don't think it's the user environment, at least on my side. I'm\n> using Fedora 7, which has python-2.5-12.fc7. That's the error from\n> the t1001 before my patch:\n\nMmm, irritating. I really don't get the error, and debug printouts\nconfirm that it's because the directories .git/remotes and\n.git/branches both exist.\n\nYour patch is the right thing to do anyway, obviously.\n\n-- \nKarl Hasselström, kha@treskal.com\n      www.treskal.com/kalle\n"}]}