{"thread":{"id":"60009","subject":"[PATCH] blame: allow --contents to work with bare repo","startedAt":"2023-07-19T10:49:04Z","lastAt":"2023-07-21T14:33:11Z","messageCount":4,"participants":["韩仰","Junio C Hamano","Han Young"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"479631","messageId":"CAG1j3zFps5yKwu=DEgkfmZDrzVoRkdeRFMFbq16ZmyOaTXxWeQ@mail.gmail.com","threadId":"60009","inReplyTo":null,"subject":"[PATCH] blame: allow --contents to work with bare repo","fromName":"韩仰","fromEmail":"hanyang.tony@bytedance.com","sentAt":"2023-07-19T10:48:34Z","receivedAt":"2023-07-19T10:49:04Z","isPatch":true,"sender":{"key":"hanyang.tony@bytedance.com","avatar":"https://avatars.githubusercontent.com/u/108711387?v=4"},"body":"The --contents option can be used with git blame to blame the file as if\nit had the contents from the specified file. Since\n1a3119ed06c8fbb1c00a6aa3615299252575abab (\"blame: allow --contents to\nwork with non-HEAD commit\"), the --contents option can work with\nnon-HEAD commit. However, if you try to use --contents in a bare\nrepository, you get the following error:\n\n    fatal: this operation must be run in a work tree\n\nThis is because before trying to generate a fake working tree commit,\nwe always call setup_work_tree. But in a bare repo, work tree is not\navailable. The call to setup_work_tree is used to prepare the reading\nof the blamed file in the work tree, which isn't neccessary if we are\nreading the contents from the specific file instead of the file in the\nwork tree.\n\nAdd a check in setup_scoreboard to skip setup_work_tree if we are\nreading from the file specified in --contents.\n\nThis enables us to use --contents in a bare repo. This is a nice\naddtion on top of 1a3119ed06c8fbb1c00a6aa3615299252575abab, having a\nworking tree to use --contents is optional.\n\nAdd test for the --contents option with bare repo to the\nannotate-tests.sh test script.\n\nSigned-off-by: Han Young <hanyang.tony@bytedance.com>\n---\nBefore commit 1a3119ed06c8fbb1c00a6aa3615299252575abab using\n--contents in a bare repo will lead to\n\n        fatal: cannot use --contents with final commit object name\n\neven without specify a revision, this is because in a bare repo, HEAD\nis seem as a passed revision. After\n1a3119ed06c8fbb1c00a6aa3615299252575abab, git will try to\nsetup_work_tree as long as --contents is passed. So we still cannot\nuse --contents on a bare repo.\n\nI was comparing files between a bare repo on the server and my local\npartial cloned repo. It is convenient for me to be able to use\n--contents option on a bare repo, because the project is very large\nand would occupy a lot of disk space if fully cloned on local machine.\n\nWith this change I can upload my local file to the server and get the\nblame result without keeping a full clone of the repo on my local\nmachine.\n\n blame.c             | 4 +++-\n t/annotate-tests.sh | 9 +++++++++\n 2 files changed, 12 insertions(+), 1 deletion(-)\n\ndiff --git a/blame.c b/blame.c\nindex d12bd9f97b..0b6703d636 100644\n--- a/blame.c\n+++ b/blame.c\n@@ -2806,7 +2806,9 @@ void setup_scoreboard(struct blame_scoreboard *sb,\n                         parent_oid = &head_oid;\n                 }\n\n-                setup_work_tree();\n+                if (!sb->contents_from) {\n+                        setup_work_tree();\n+                }\n                 sb->final = fake_working_tree_commit(sb->repo,\n                                                      &sb->revs->diffopt,\n                                                      sb->path,\nsb->contents_from,\ndiff --git a/t/annotate-tests.sh b/t/annotate-tests.sh\nindex 2ef70235b1..5e21e84f38 100644\n--- a/t/annotate-tests.sh\n+++ b/t/annotate-tests.sh\n@@ -83,6 +83,15 @@ test_expect_success 'blame with --contents' '\n         check_count --contents=file A 2\n '\n\n+test_expect_success 'blame with --contents in a bare repo' '\n+        git clone --bare . bare-contents.git &&\n+        (\n+                cd bare-contents.git &&\n+                echo \"1A quick brown fox jumps over the\" >contents &&\n+                check_count --contents=contents A 1\n+        )\n+'\n+\n test_expect_success 'blame with --contents changed' '\n         echo \"1A quick brown fox jumps over the\" >contents &&\n         echo \"another lazy dog\" >>contents &&\n--\n2.40.0\n"},{"id":"479657","messageId":"xmqqjzuva981.fsf@gitster.g","threadId":"60009","inReplyTo":"CAG1j3zFps5yKwu=DEgkfmZDrzVoRkdeRFMFbq16ZmyOaTXxWeQ@mail.gmail.com","subject":"Re: [PATCH] blame: allow --contents to work with bare repo","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-07-19T21:21:18Z","receivedAt":"2023-07-19T21:21:26Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"韩仰 <hanyang.tony@bytedance.com> writes:\n\n> -                setup_work_tree();\n> +                if (!sb->contents_from) {\n> +                        setup_work_tree();\n> +                }\n\nUnwanted {braces} around a single statement.\n\nThe patch is severely whitespace damaged and does not apply.\n\nPlease check your outgoing mail route and make sure it does not\nhappen when you send out an updated patch.  Sending an e-mail to\nyourself and checking what you received may also work as a way to\nsanity-check without bombarding other people and the list with a\ntest message.\n\nThanks.\n\n"},{"id":"479704","messageId":"20230721035758.61956-1-hanyang.tony@bytedance.com","threadId":"60009","inReplyTo":"CAG1j3zFps5yKwu=DEgkfmZDrzVoRkdeRFMFbq16ZmyOaTXxWeQ@mail.gmail.com","subject":"[PATCH v2] blame: allow --contents to work with bare repo","fromName":"Han Young","fromEmail":"hanyang.tony@bytedance.com","sentAt":"2023-07-21T03:57:58Z","receivedAt":"2023-07-21T03:58:37Z","isPatch":true,"sender":{"key":"hanyang.tony@bytedance.com","avatar":"https://avatars.githubusercontent.com/u/108711387?v=4"},"body":"The --contents option can be used with git blame to blame the file as if\nit had the contents from the specified file. Since 1a3119ed06c8fbb1c00a6aa3615299252575abab (\"blame: allow --contents to work with non-HEAD commit\"), the --contents option can work with non-HEAD commit. However, if you try to use --contents in a bare repository, you get the following error:\n\n    fatal: this operation must be run in a work tree\n\nThis is because before trying to generate a fake working tree commit, we always call setup_work_tree. But in a bare repo, work tree is not available. The call to setup_work_tree is used to prepare the reading of the blamed file in the work tree, which isn't necessary if we are reading the contents from the specific file instead of the file in the work tree.\n\nAdd a check in setup_scoreboard to skip setup_work_tree if we are reading from the file specified in --contents.\n\nThis enables us to use --contents in a bare repo. This is a nice addition on top of 1a3119ed06c8fbb1c00a6aa3615299252575abab, having a working tree to use --contents is optional.\n\nAdd test for the --contents option with bare repo to the annotate-tests.sh test script.\n\nSigned-off-by: Han Young <hanyang.tony@bytedance.com>\n---\nChanges since v1:\n* Removed unwanted {braces} around a single statement\n* Sent the email properly with git-send-email so the tabs are preserved\n* Corrected typos in commit message\n\nBefore commit 1a3119ed06c8fbb1c00a6aa3615299252575abab using --contents in a bare repo will lead to\n\n\tfatal: cannot use --contents with final commit object name\n\neven without specify a revision, this is because in a bare repo, HEAD is seem as a passed revision. After 1a3119ed06c8fbb1c00a6aa3615299252575abab, git will try to setup_work_tree as long as --contents is passed. So we still cannot use --contents on a bare repo.\n\nI was comparing files between a bare repo on the server and my local partial cloned repo. It is convenient for me to be able to use --contents option on a bare repo, because the project is very large and would occupy a lot of disk space if fully cloned on local machine.\n\nWith this change I can upload my local file to the server and get the blame result without keeping a full clone of the repo on my local machine.\n\n blame.c             | 4 +++-\n t/annotate-tests.sh | 9 +++++++++\n 2 files changed, 12 insertions(+), 1 deletion(-)\n\ndiff --git a/blame.c b/blame.c\nindex d12bd9f97..141756975 100644\n--- a/blame.c\n+++ b/blame.c\n@@ -2806,7 +2806,9 @@ void setup_scoreboard(struct blame_scoreboard *sb,\n \t\t\tparent_oid = &head_oid;\n \t\t}\n \n-\t\tsetup_work_tree();\n+\t\tif (!sb->contents_from)\n+\t\t\tsetup_work_tree();\n+\n \t\tsb->final = fake_working_tree_commit(sb->repo,\n \t\t\t\t\t\t     &sb->revs->diffopt,\n \t\t\t\t\t\t     sb->path, sb->contents_from,\ndiff --git a/t/annotate-tests.sh b/t/annotate-tests.sh\nindex 2ef70235b..5e21e84f3 100644\n--- a/t/annotate-tests.sh\n+++ b/t/annotate-tests.sh\n@@ -83,6 +83,15 @@ test_expect_success 'blame with --contents' '\n \tcheck_count --contents=file A 2\n '\n \n+test_expect_success 'blame with --contents in a bare repo' '\n+\tgit clone --bare . bare-contents.git &&\n+\t(\n+\t\tcd bare-contents.git &&\n+\t\techo \"1A quick brown fox jumps over the\" >contents &&\n+\t\tcheck_count --contents=contents A 1\n+\t)\n+'\n+\n test_expect_success 'blame with --contents changed' '\n \techo \"1A quick brown fox jumps over the\" >contents &&\n \techo \"another lazy dog\" >>contents &&\n-- \n2.40.0\n\n"},{"id":"479734","messageId":"xmqq8rb9z65t.fsf@gitster.g","threadId":"60009","inReplyTo":"20230721035758.61956-1-hanyang.tony@bytedance.com","subject":"Re: [PATCH v2] blame: allow --contents to work with bare repo","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-07-21T14:32:46Z","receivedAt":"2023-07-21T14:33:11Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Han Young <hanyang.tony@bytedance.com> writes:\n\n> The --contents option can be used with git blame to blame the file as if\n> it had the contents from the specified file. Since 1a3119ed06c8fbb1c00a6aa3615299252575abab (\"blame: allow --contents to work with non-HEAD commit\"), the --contents option can work with non-HEAD commit. However, if you try to use --contents in a bare repository, you get the following error:\n\nWhen using the \"show -s --pretty=reference --oneline\", abbreviate\nthe commit object name to a reasonable length (like 8 to 12\nhexadecimal digits).  Please wrap overlong lines.\n\nI'll fix them up at the receiving end, if there is no other issues\nin the patch.  Let's read on.\n\n>  blame.c             | 4 +++-\n>  t/annotate-tests.sh | 9 +++++++++\n>  2 files changed, 12 insertions(+), 1 deletion(-)\n>\n> diff --git a/blame.c b/blame.c\n> index d12bd9f97..141756975 100644\n> --- a/blame.c\n> +++ b/blame.c\n> @@ -2806,7 +2806,9 @@ void setup_scoreboard(struct blame_scoreboard *sb,\n>  \t\t\tparent_oid = &head_oid;\n>  \t\t}\n>  \n> -\t\tsetup_work_tree();\n> +\t\tif (!sb->contents_from)\n> +\t\t\tsetup_work_tree();\n> +\n>  \t\tsb->final = fake_working_tree_commit(sb->repo,\n>  \t\t\t\t\t\t     &sb->revs->diffopt,\n>  \t\t\t\t\t\t     sb->path, sb->contents_from,\n\nThanks.\n\n> diff --git a/t/annotate-tests.sh b/t/annotate-tests.sh\n> index 2ef70235b..5e21e84f3 100644\n> --- a/t/annotate-tests.sh\n> +++ b/t/annotate-tests.sh\n> @@ -83,6 +83,15 @@ test_expect_success 'blame with --contents' '\n>  \tcheck_count --contents=file A 2\n>  '\n>  \n> +test_expect_success 'blame with --contents in a bare repo' '\n> +\tgit clone --bare . bare-contents.git &&\n> +\t(\n> +\t\tcd bare-contents.git &&\n> +\t\techo \"1A quick brown fox jumps over the\" >contents &&\n> +\t\tcheck_count --contents=contents A 1\n> +\t)\n> +'\n> +\n>  test_expect_success 'blame with --contents changed' '\n>  \techo \"1A quick brown fox jumps over the\" >contents &&\n>  \techo \"another lazy dog\" >>contents &&\n"}]}