{"thread":{"id":"50212","subject":"[PATCH 0/2] git-p4: handle moved files when updating a P4 shelve","startedAt":"2019-01-13T13:58:25Z","lastAt":"2019-01-14T19:03:13Z","messageCount":6,"participants":["Luke Diamand","Eric Sunshine","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"366632","messageId":"20190113135815.11286-1-luke@diamand.org","threadId":"50212","inReplyTo":null,"subject":"[PATCH 0/2] git-p4: handle moved files when updating a P4 shelve","fromName":"Luke Diamand","fromEmail":"luke@diamand.org","sentAt":"2019-01-13T13:58:13Z","receivedAt":"2019-01-13T13:58:25Z","isPatch":true,"sender":{"key":"luke@diamand.org","avatar":"https://avatars.githubusercontent.com/u/5330967?v=4"},"body":"I found this bug recently in git-p4 - updating a shelved changelist where files\nare being moved doesn't work. The destination of any moves needs to be\nadded to the list of files passed to P4.\n\nLuke Diamand (2):\n git-p4: add failing test for shelved CL update involving move\n git-p4: handle update of moved files when updating a shelve\n\n git-p4.py | 1 +\n t/t9807-git-p4-submit.sh | 53 +++++++++++++++++++++++++++++++++++++---\n 2 files changed, 51 insertions(+), 3 deletions(-)\n\n-- \n2.20.1.100.g9ee79a14a8\n\n"},{"id":"366633","messageId":"20190113135815.11286-2-luke@diamand.org","threadId":"50212","inReplyTo":"20190113135815.11286-1-luke@diamand.org","subject":"[PATCH 1/2] git-p4: add failing test for shelved CL update involving move","fromName":"Luke Diamand","fromEmail":"luke@diamand.org","sentAt":"2019-01-13T13:58:14Z","receivedAt":"2019-01-13T13:58:27Z","isPatch":true,"sender":{"key":"luke@diamand.org","avatar":"https://avatars.githubusercontent.com/u/5330967?v=4"},"body":"Updating a shelved P4 changelist where one or more of the files have\nbeen moved does not work. Add a test for this.\n\nSigned-off-by: Luke Diamand \n---\n t/t9807-git-p4-submit.sh | 53 +++++++++++++++++++++++++++++++++++++---\n 1 file changed, 50 insertions(+), 3 deletions(-)\n\ndiff --git a/t/t9807-git-p4-submit.sh b/t/t9807-git-p4-submit.sh\nindex 2325599ee6..08dc8d2caf 100755\n--- a/t/t9807-git-p4-submit.sh\n+++ b/t/t9807-git-p4-submit.sh\n@@ -500,6 +500,12 @@ test_expect_success 'submit --shelve' '\n \t)\n '\n \n+last_shelve() {\n+\tchange=$(p4 -G changes -s shelved -m 1 //depot/... | \\\n+\t\tmarshal_dump change)\n+\techo $change\n+}\n+\n make_shelved_cl() {\n \ttest_commit \"$1\" >/dev/null &&\n \tgit p4 submit --origin HEAD^ --shelve >/dev/null &&\n@@ -533,12 +539,53 @@ test_expect_success 'submit --update-shelve' '\n \t) &&\n \t(\n \t\tcd \"$cli\" &&\n-\t\tchange=$(p4 -G changes -s shelved -m 1 //depot/... | \\\n-\t\t\t marshal_dump change) &&\n+\t\tchange=$(last_shelve) &&\n \t\tp4 unshelve -c $change -s $change &&\n \t\tgrep -q updated-line shelf.t &&\n \t\tp4 describe -S $change | grep added-file.t &&\n-\t\ttest_path_is_missing shelved-change-1.t\n+\t\ttest_path_is_missing shelved-change-1.t &&\n+\t\tp4 revert ...\n+\t)\n+'\n+\n+test_expect_failure 'update a shelve involving a moved file' '\n+\ttest_when_finished cleanup_git &&\n+\t(\n+\t\tcd \"$cli\" &&\n+\t\t: >file_to_move &&\n+\t\tp4 add file_to_move &&\n+\t\tp4 submit -d \"change1\" &&\n+\t\tp4 edit file_to_move &&\n+\t\techo change >>file_to_move &&\n+\t\tp4 submit -d \"change2\" &&\n+\t\tp4 opened\n+\t) &&\n+\tgit p4 clone --dest=\"$git\" //depot &&\n+\t(\n+\t\tcd \"$git\" &&\n+\t\tgit config git-p4.skipSubmitEdit true &&\n+\t\tmkdir moved &&\n+\t\tgit mv file_to_move moved/ &&\n+\t\tgit commit -m \"rename a file\" &&\n+\t\tgit p4 submit -M --shelve --origin HEAD^ &&\n+\t\t: >new_file &&\n+\t\tgit add new_file &&\n+\t\tgit commit --amend &&\n+\t\tgit show --stat HEAD &&\n+\t\tchange=$(last_shelve) &&\n+\t\tgit p4 submit -M --update-shelve $change --commit HEAD\n+\t) &&\n+\t(\n+\t\tcd \"$cli\" &&\n+\t\tchange=$(last_shelve) &&\n+\t\techo change=$change &&\n+\t\tp4 unshelve -s $change &&\n+\t\tp4 submit -d \"Testing update-shelve\" &&\n+\t\ttest_path_is_file moved/file_to_move &&\n+\t\ttest_path_is_missing file_to_move &&\n+\t\ttest_path_is_file new_file &&\n+\t\techo \"unshelved and submitted change $change\" &&\n+\t\tp4 changes moved/file_to_move | grep \"Testing update-shelve\"\n \t)\n '\n \n-- \n2.20.1.100.g9ee79a14a8\n\n"},{"id":"366634","messageId":"20190113135815.11286-3-luke@diamand.org","threadId":"50212","inReplyTo":"20190113135815.11286-2-luke@diamand.org","subject":"[PATCH 2/2] git-p4: handle update of moved files when updating a shelve","fromName":"Luke Diamand","fromEmail":"luke@diamand.org","sentAt":"2019-01-13T13:58:15Z","receivedAt":"2019-01-13T13:58:28Z","isPatch":true,"sender":{"key":"luke@diamand.org","avatar":"https://avatars.githubusercontent.com/u/5330967?v=4"},"body":"Perforce requires a complete list of files being operated on. If\ngit is updating an existing shelved changelist, then any files\nwhich are moved were not being added to this list.\n\nSigned-off-by: Luke Diamand \n---\n git-p4.py | 1 +\n t/t9807-git-p4-submit.sh | 2 +-\n 2 files changed, 2 insertions(+), 1 deletion(-)\n\ndiff --git a/git-p4.py b/git-p4.py\nindex 1998c3e141..20c5ce9903 100755\n--- a/git-p4.py\n+++ b/git-p4.py\n@@ -1875,6 +1875,7 @@ def applyCommit(self, id):\n editedFiles.add(dest)\n elif modifier == \"R\":\n src, dest = diff['src'], diff['dst']\n+ all_files.append(dest)\n if self.p4HasMoveCommand:\n p4_edit(src) # src must be open before move\n p4_move(src, dest) # opens for (move/delete, move/add)\ndiff --git a/t/t9807-git-p4-submit.sh b/t/t9807-git-p4-submit.sh\nindex 08dc8d2caf..4d5ea9e64c 100755\n--- a/t/t9807-git-p4-submit.sh\n+++ b/t/t9807-git-p4-submit.sh\n@@ -548,7 +548,7 @@ test_expect_success 'submit --update-shelve' '\n \t)\n '\n \n-test_expect_failure 'update a shelve involving a moved file' '\n+test_expect_success 'update a shelve involving a moved file' '\n \ttest_when_finished cleanup_git &&\n \t(\n \t\tcd \"$cli\" &&\n-- \n2.20.1.100.g9ee79a14a8\n\n"},{"id":"366640","messageId":"CAPig+cSPL4vcfWR7Pos91N_SO-qCSBMYFY8vbyHX-POKyyRJpg@mail.gmail.com","threadId":"50212","inReplyTo":"20190113135815.11286-2-luke@diamand.org","subject":"Re: [PATCH 1/2] git-p4: add failing test for shelved CL update involving move","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2019-01-13T21:57:42Z","receivedAt":"2019-01-13T21:57:56Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Sun, Jan 13, 2019 at 8:58 AM Luke Diamand wrote:\n> Updating a shelved P4 changelist where one or more of the files have\n> been moved does not work. Add a test for this.\n\nPerhaps this message could give more detail about the actual problem\nthan the generic \"does not work\" which provides no useful information.\n\n> Signed-off-by: Luke Diamand \n> ---\n> diff --git a/t/t9807-git-p4-submit.sh b/t/t9807-git-p4-submit.sh\n> @@ -500,6 +500,12 @@ test_expect_success 'submit --shelve' '\n> +last_shelve() {\n> + change=$(p4 -G changes -s shelved -m 1 //depot/... | \\\n> + marshal_dump change)\n> + echo $change\n> +}\n\nA simpler definition for this function would be:\n\nlast_shelve () {\n p4 -G changes -s shelved -m 1 //depot/... | marshal_dump change\n}\n\nwhich will give the same result when you later capture its output with:\n\n change=$(last_shelve) &&\n\n> @@ -533,12 +539,53 @@ test_expect_success 'submit --update-shelve' '\n> ) &&\n> (\n> cd \"$cli\" &&\n> - change=$(p4 -G changes -s shelved -m 1 //depot/... | \\\n> - marshal_dump change) &&\n> + change=$(last_shelve) &&\n> p4 unshelve -c $change -s $change &&\n> grep -q updated-line shelf.t &&\n> p4 describe -S $change | grep added-file.t &&\n> - test_path_is_missing shelved-change-1.t\n> + test_path_is_missing shelved-change-1.t &&\n> + p4 revert ...\n> + )\n> +'\n"},{"id":"366671","messageId":"xmqqva2rukz6.fsf@gitster-ct.c.googlers.com","threadId":"50212","inReplyTo":"20190113135815.11286-1-luke@diamand.org","subject":"Re: [PATCH 0/2] git-p4: handle moved files when updating a P4 shelve","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-01-14T18:56:13Z","receivedAt":"2019-01-14T18:56:19Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Luke Diamand writes:\n\n> I found this bug recently in git-p4 - updating a shelved changelist where files\n> are being moved doesn't work. The destination of any moves needs to be\n> added to the list of files passed to P4.\n\nThanks.\n\n>\n> Luke Diamand (2):\n> git-p4: add failing test for shelved CL update involving move\n> git-p4: handle update of moved files when updating a shelve\n>\n> git-p4.py | 1 +\n> t/t9807-git-p4-submit.sh | 53 +++++++++++++++++++++++++++++++++++++---\n> 2 files changed, 51 insertions(+), 3 deletions(-)\n"},{"id":"366672","messageId":"xmqqr2dfuknm.fsf@gitster-ct.c.googlers.com","threadId":"50212","inReplyTo":"CAPig+cSPL4vcfWR7Pos91N_SO-qCSBMYFY8vbyHX-POKyyRJpg@mail.gmail.com","subject":"Re: [PATCH 1/2] git-p4: add failing test for shelved CL update involving move","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-01-14T19:03:09Z","receivedAt":"2019-01-14T19:03:13Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Eric Sunshine writes:\n\n> On Sun, Jan 13, 2019 at 8:58 AM Luke Diamand wrote:\n>> Updating a shelved P4 changelist where one or more of the files have\n>> been moved does not work. Add a test for this.\n>\n> Perhaps this message could give more detail about the actual problem\n> than the generic \"does not work\" which provides no useful information.\n>\n>> Signed-off-by: Luke Diamand \n>> ---\n>> diff --git a/t/t9807-git-p4-submit.sh b/t/t9807-git-p4-submit.sh\n>> @@ -500,6 +500,12 @@ test_expect_success 'submit --shelve' '\n>> +last_shelve() {\n>> + change=$(p4 -G changes -s shelved -m 1 //depot/... | \\\n>> + marshal_dump change)\n>> + echo $change\n>> +}\n>\n> A simpler definition for this function would be:\n>\n> last_shelve () {\n> p4 -G changes -s shelved -m 1 //depot/... | marshal_dump change\n> }\n\nIndeed, and it will work better even when the output from marshal_dump\nhas $IFS and other traits that do not survive \"echo $change\" intact.\n\n"}]}