{"thread":{"id":"14516","subject":"stgit: bug with refresh and -p","startedAt":"2008-07-18T01:29:01Z","lastAt":"2008-07-25T21:37:23Z","messageCount":9,"participants":["Jon Smirl","Karl Hasselström","Catalin Marinas"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"83784","messageId":"9e4733910807171829q6abdcfc2m90c40a70dbc8fef5@mail.gmail.com","threadId":"14516","inReplyTo":null,"subject":"stgit: bug with refresh and -p","fromName":"Jon Smirl","fromEmail":"jonsmirl@gmail.com","sentAt":"2008-07-18T01:29:01Z","receivedAt":"2008-07-18T01:29:01Z","isPatch":false,"sender":{"key":"jonsmirl@gmail.com","avatar":"https://gravatar.com/avatar/cff3bf5bfdfa6708b905712ff91f0f9b8aaca161659f38c02b787920d5d28b7e?d=mp&s=160"},"body":"A refresh with -p is not picking up newly added files.\n\njonsmirl@terra:~/fs$ stg status\n? include/linux/i2c/max9485.h\njonsmirl@terra:~/fs$ git add include/linux/i2c/max9485.h\njonsmirl@terra:~/fs$ stg refresh -p max9485\nChecking for changes in the working directory ... done\nPopping patches \"jds-audio\" - \"jds-psc\" ... done\nRefreshing patch \"max9485\" ... done\nFast-forwarded patches \"jds-psc\" - \"jds-audio\"\njonsmirl@terra:~/fs$ stg status\n? include/linux/i2c/max9485.h\njonsmirl@terra:~/fs$ stg --version\nStacked GIT 0.14.3.163.g06f9\ngit version 1.5.6.1\nPython version 2.5.2 (r252:60911, Apr 21 2008, 11:17:30)\n[GCC 4.2.3 (Ubuntu 4.2.3-2ubuntu7)]\njonsmirl@terra:~/fs$\n\n-- \nJon Smirl\njonsmirl@gmail.com\n"},{"id":"83843","messageId":"20080718084127.GA7042@diana.vm.bytemark.co.uk","threadId":"14516","inReplyTo":"9e4733910807171829q6abdcfc2m90c40a70dbc8fef5@mail.gmail.com","subject":"Re: stgit: bug with refresh and -p","fromName":"Karl Hasselström","fromEmail":"kha@treskal.com","sentAt":"2008-07-18T08:41:27Z","receivedAt":"2008-07-18T08:41:27Z","isPatch":false,"sender":{"key":"kha@treskal.com","avatar":"https://gravatar.com/avatar/f0120c734b5279b345075a28521e1ac66acb20c9913ffe9bf6ae97e53f7f3f13?d=mp&s=160"},"body":"On 2008-07-17 21:29:01 -0400, Jon Smirl wrote:\n\n> A refresh with -p is not picking up newly added files.\n>\n> jonsmirl@terra:~/fs$ stg status\n> ? include/linux/i2c/max9485.h\n> jonsmirl@terra:~/fs$ git add include/linux/i2c/max9485.h\n> jonsmirl@terra:~/fs$ stg refresh -p max9485\n> Checking for changes in the working directory ... done\n> Popping patches \"jds-audio\" - \"jds-psc\" ... done\n> Refreshing patch \"max9485\" ... done\n> Fast-forwarded patches \"jds-psc\" - \"jds-audio\"\n> jonsmirl@terra:~/fs$ stg status\n> ? include/linux/i2c/max9485.h\n> jonsmirl@terra:~/fs$ stg --version\n> Stacked GIT 0.14.3.163.g06f9\n\nHmm, I don't immediately see why this happens. I've posted a bug about\nit: https://gna.org/bugs/index.php?12038\n\nIt's fixed by the refresh rewrite in my experimental branch, but\nthere's no test for it, so I had to check by hand.\n\n-- \nKarl Hasselström, kha@treskal.com\n      www.treskal.com/kalle\n"},{"id":"83892","messageId":"20080718170225.10086.17504.stgit@yoghurt","threadId":"14516","inReplyTo":"20080718084127.GA7042@diana.vm.bytemark.co.uk","subject":"[StGit PATCH] Test that we can add a new file to a non-topmost patch with refresh -p","fromName":"Karl Hasselström","fromEmail":"kha@treskal.com","sentAt":"2008-07-18T17:03:06Z","receivedAt":"2008-07-18T17:03:06Z","isPatch":true,"sender":{"key":"kha@treskal.com","avatar":"https://gravatar.com/avatar/f0120c734b5279b345075a28521e1ac66acb20c9913ffe9bf6ae97e53f7f3f13?d=mp&s=160"},"body":"We currently can't -- this is bug 12038, found by Jon Smirl. See\n\n  https://gna.org/bugs/index.php?12038\n\nSigned-off-by: Karl Hasselström <kha@treskal.com>\n\n---\n\nHere's a proper test that demonstrates the bug. It applies to the\nstable branch.\n\n t/t2701-refresh-p.sh |   46 ++++++++++++++++++++++++++++++++++++++++++++++\n 1 files changed, 46 insertions(+), 0 deletions(-)\n create mode 100755 t/t2701-refresh-p.sh\n\n\ndiff --git a/t/t2701-refresh-p.sh b/t/t2701-refresh-p.sh\nnew file mode 100755\nindex 0000000..d42e90f\n--- /dev/null\n+++ b/t/t2701-refresh-p.sh\n@@ -0,0 +1,46 @@\n+#!/bin/sh\n+\n+test_description='Run \"stg refresh -p\"'\n+\n+. ./test-lib.sh\n+\n+# Ignore our own temp files.\n+cat >> .git/info/exclude <<EOF\n+expected*.txt\n+files*.txt\n+status*.txt\n+EOF\n+\n+test_expect_success 'Initialize StGit stack' '\n+    stg init &&\n+    for i in 1 2; do\n+        echo x > $i.txt &&\n+        git add $i.txt &&\n+        stg new p$i -m \"Patch $i\" &&\n+        stg refresh\n+    done\n+'\n+\n+touch expected0.txt\n+cat > expected1.txt <<EOF\n+A 1.txt\n+A new.txt\n+EOF\n+cat > expected2.txt <<EOF\n+A 2.txt\n+EOF\n+test_expect_failure 'Add new file to non-top patch' '\n+    stg status > status1.txt &&\n+    diff -u expected0.txt status1.txt &&\n+    echo y > new.txt &&\n+    git add new.txt &&\n+    stg refresh -p p1 &&\n+    stg status > status2.txt &&\n+    diff -u expected0.txt status2.txt &&\n+    stg files p1 > files1.txt &&\n+    diff -u expected1.txt files1.txt &&\n+    stg files p2 > files2.txt &&\n+    diff -u expected2.txt files2.txt\n+'\n+\n+test_done\n"},{"id":"83899","messageId":"9e4733910807181045mb19ab1dxaaa76696db294ddf@mail.gmail.com","threadId":"14516","inReplyTo":"20080718180109.GA14825@diana.vm.bytemark.co.uk","subject":"Re: [StGit PATCH] Test that we can add a new file to a non-topmost patch with refresh -p","fromName":"Jon Smirl","fromEmail":"jonsmirl@gmail.com","sentAt":"2008-07-18T17:45:41Z","receivedAt":"2008-07-18T17:45:41Z","isPatch":true,"sender":{"key":"jonsmirl@gmail.com","avatar":"https://gravatar.com/avatar/cff3bf5bfdfa6708b905712ff91f0f9b8aaca161659f38c02b787920d5d28b7e?d=mp&s=160"},"body":"On 7/18/08, Karl Hasselström <kha@treskal.com> wrote:\n> On 2008-07-18 19:03:06 +0200, Karl Hasselström wrote:\n>\n>  > We currently can't -- this is bug 12038, found by Jon Smirl. See\n>  >\n>  >   https://gna.org/bugs/index.php?12038\n>\n>  OK, the problem is that to pop the patches on top of the one we are to\n>  refresh, we call pop_patch(keep = True). This in turn calls\n>  git.switch(keep = True), which resets the index (but is careful to not\n>  touch the worktree).\n>\n>  I'm not quite sure how to fix this in a simple way -- the code simply\n>  assumes that the index contains nothing of interest. And since I\n>  already have a rewrite of refresh that handles this and a handful of\n>  other cases that the old code does not, I'm kind of disinclined to\n>  undertake a larger restructuring of the code.\n\nIt's no big deal to me, it is easy to work around. But it did take me\na while to notice that the add was missing.\n\nWhen is the refresh rewrite going to be ready for prime time?\n\n>\n>  Catalin, what do you think?\n>\n>\n>  --\n>  Karl Hasselström, kha@treskal.com\n>       www.treskal.com/kalle\n>\n\n\n-- \nJon Smirl\njonsmirl@gmail.com\n"},{"id":"83898","messageId":"20080718180109.GA14825@diana.vm.bytemark.co.uk","threadId":"14516","inReplyTo":"20080718170225.10086.17504.stgit@yoghurt","subject":"Re: [StGit PATCH] Test that we can add a new file to a non-topmost patch with refresh -p","fromName":"Karl Hasselström","fromEmail":"kha@treskal.com","sentAt":"2008-07-18T18:01:09Z","receivedAt":"2008-07-18T18:01:09Z","isPatch":true,"sender":{"key":"kha@treskal.com","avatar":"https://gravatar.com/avatar/f0120c734b5279b345075a28521e1ac66acb20c9913ffe9bf6ae97e53f7f3f13?d=mp&s=160"},"body":"On 2008-07-18 19:03:06 +0200, Karl Hasselström wrote:\n\n> We currently can't -- this is bug 12038, found by Jon Smirl. See\n>\n>   https://gna.org/bugs/index.php?12038\n\nOK, the problem is that to pop the patches on top of the one we are to\nrefresh, we call pop_patch(keep = True). This in turn calls\ngit.switch(keep = True), which resets the index (but is careful to not\ntouch the worktree).\n\nI'm not quite sure how to fix this in a simple way -- the code simply\nassumes that the index contains nothing of interest. And since I\nalready have a rewrite of refresh that handles this and a handful of\nother cases that the old code does not, I'm kind of disinclined to\nundertake a larger restructuring of the code.\n\nCatalin, what do you think?\n\n-- \nKarl Hasselström, kha@treskal.com\n      www.treskal.com/kalle\n"},{"id":"83909","messageId":"20080718191608.GA16725@diana.vm.bytemark.co.uk","threadId":"14516","inReplyTo":"9e4733910807181045mb19ab1dxaaa76696db294ddf@mail.gmail.com","subject":"Re: [StGit PATCH] Test that we can add a new file to a non-topmost patch with refresh -p","fromName":"Karl Hasselström","fromEmail":"kha@treskal.com","sentAt":"2008-07-18T19:16:08Z","receivedAt":"2008-07-18T19:16:08Z","isPatch":true,"sender":{"key":"kha@treskal.com","avatar":"https://gravatar.com/avatar/f0120c734b5279b345075a28521e1ac66acb20c9913ffe9bf6ae97e53f7f3f13?d=mp&s=160"},"body":"On 2008-07-18 13:45:41 -0400, Jon Smirl wrote:\n\n> It's no big deal to me, it is easy to work around. But it did take\n> me a while to notice that the add was missing.\n\nAh, OK, good.\n\n> When is the refresh rewrite going to be ready for prime time?\n\nIt depends on the stack log stuff, but that is actually kind of close\nto being ready now. It just needs support for hidden patches.\n\nNo time estimate, sorry -- vacation time's coming up, so I can't\nreally predict how much time I'll spend hacking.\n\n-- \nKarl Hasselström, kha@treskal.com\n      www.treskal.com/kalle\n"},{"id":"84248","messageId":"b0943d9e0807211501g68d1cffeh12bcb34df0f28dae@mail.gmail.com","threadId":"14516","inReplyTo":"20080718180109.GA14825@diana.vm.bytemark.co.uk","subject":"Re: [StGit PATCH] Test that we can add a new file to a non-topmost patch with refresh -p","fromName":"Catalin Marinas","fromEmail":"catalin.marinas@gmail.com","sentAt":"2008-07-21T22:01:17Z","receivedAt":"2008-07-21T22:01:17Z","isPatch":true,"sender":{"key":"catalin.marinas@gmail.com","avatar":null},"body":"2008/7/18 Karl Hasselström <kha@treskal.com>:\n> On 2008-07-18 19:03:06 +0200, Karl Hasselström wrote:\n>\n>> We currently can't -- this is bug 12038, found by Jon Smirl. See\n>>\n>>   https://gna.org/bugs/index.php?12038\n>\n> OK, the problem is that to pop the patches on top of the one we are to\n> refresh, we call pop_patch(keep = True). This in turn calls\n> git.switch(keep = True), which resets the index (but is careful to not\n> touch the worktree).\n\nYes.\n\n> I'm not quite sure how to fix this in a simple way -- the code simply\n> assumes that the index contains nothing of interest. And since I\n> already have a rewrite of refresh that handles this and a handful of\n> other cases that the old code does not, I'm kind of disinclined to\n> undertake a larger restructuring of the code.\n>\n> Catalin, what do you think?\n\nI don't think we should spend time on fixing the current code as you\nalready have a new implementation. Maybe we could add a simple test\nand refuse refreshing other than the topmost patch in case of files\nadded to the index.\n\n-- \nCatalin\n"},{"id":"84285","messageId":"20080722071438.GA16807@diana.vm.bytemark.co.uk","threadId":"14516","inReplyTo":"b0943d9e0807211501g68d1cffeh12bcb34df0f28dae@mail.gmail.com","subject":"Re: [StGit PATCH] Test that we can add a new file to a non-topmost patch with refresh -p","fromName":"Karl Hasselström","fromEmail":"kha@treskal.com","sentAt":"2008-07-22T07:14:38Z","receivedAt":"2008-07-22T07:14:38Z","isPatch":true,"sender":{"key":"kha@treskal.com","avatar":"https://gravatar.com/avatar/f0120c734b5279b345075a28521e1ac66acb20c9913ffe9bf6ae97e53f7f3f13?d=mp&s=160"},"body":"On 2008-07-21 23:01:17 +0100, Catalin Marinas wrote:\n\n> I don't think we should spend time on fixing the current code as you\n> already have a new implementation. Maybe we could add a simple test\n> and refuse refreshing other than the topmost patch in case of files\n> added to the index.\n\nYes, I guess that's OK. Hmm, how do we check that cheaply?\n\n-- \nKarl Hasselström, kha@treskal.com\n      www.treskal.com/kalle\n"},{"id":"84952","messageId":"b0943d9e0807251437u4c01fb45n98e1b0b29e9c4c78@mail.gmail.com","threadId":"14516","inReplyTo":"20080722071438.GA16807@diana.vm.bytemark.co.uk","subject":"Re: [StGit PATCH] Test that we can add a new file to a non-topmost patch with refresh -p","fromName":"Catalin Marinas","fromEmail":"catalin.marinas@gmail.com","sentAt":"2008-07-25T21:37:23Z","receivedAt":"2008-07-25T21:37:23Z","isPatch":true,"sender":{"key":"catalin.marinas@gmail.com","avatar":null},"body":"2008/7/22 Karl Hasselström <kha@treskal.com>:\n> On 2008-07-21 23:01:17 +0100, Catalin Marinas wrote:\n>\n>> I don't think we should spend time on fixing the current code as you\n>> already have a new implementation. Maybe we could add a simple test\n>> and refuse refreshing other than the topmost patch in case of files\n>> added to the index.\n>\n> Yes, I guess that's OK. Hmm, how do we check that cheaply?\n\nIn the stable branch, we already call git.check_status() in the\nrefresh implementation, so it's pretty cheap to test.\n\n-- \nCatalin\n"}]}