threads / patch / 30811

patchgit-http-fetch: remove unused cmd_http_fetch

Subject: [PATCH] git-http-fetch: remove unused cmd_http_fetch

## tl;dr

5 messages between Jun 14, 2012 and Jun 15, 2012. Diffs are folded; open one to read it.

replies: 4people: 3as markdown or json

Luka Perkov· Jun 14, 2012, 20:23 UTC · lore

It was left out from commit 1088261f6fc90324014b5306cca4171987da85ce

Signed-off-by: Luka Perkov <lists@lukaperkov.net>
---

 builtin.h |    1 -
 1 file changed, 1 deletion(-)

Show changes to builtin.h +0 −1

diff --git a/builtin.h b/builtin.h
index 338f540..f5b8ed1 100644
--- a/builtin.h
+++ b/builtin.h
@@ -83,7 +83,6 @@ extern int cmd_get_tar_commit_id(int argc, const char **argv, const char *prefix
 extern int cmd_grep(int argc, const char **argv, const char *prefix);
 extern int cmd_hash_object(int argc, const char **argv, const char *prefix);
 extern int cmd_help(int argc, const char **argv, const char *prefix);
-extern int cmd_http_fetch(int argc, const char **argv, const char *prefix);
 extern int cmd_index_pack(int argc, const char **argv, const char *prefix);
 extern int cmd_init_db(int argc, const char **argv, const char *prefix);
 extern int cmd_log(int argc, const char **argv, const char *prefix);

Jeff King· Jun 15, 2012, 16:21 UTC · re: Luka Perkov · lore

Re: [PATCH] git-http-fetch: remove unused cmd_http_fetch

On Thu, Jun 14, 2012 at 10:23:37PM +0200, Luka Perkov wrote:

> It was left out from commit 1088261f6fc90324014b5306cca4171987da85ce

This commit message left me scratching my head. Did we get rid of http-fetch? If not, then don't we still need cmd_http_fetch? Or did we just make it not a builtin, in which case we wouldn't be getting rid of cmd_http_fetch, but rather converting it to main?

Reading the 1088261, I find the answer: we did make it not a builtin, and it was indeed converted into "main". But its _declaration_ hung around.

So maybe a better commit message would be:

  Subject: builtin.h: drop cmd_http_fetch declaration

  This was converted from a builtin into a stand-alone program by
  1088261f6fc90324014b5306cca4171987da85ce, but that commit forgot to
  drop the declaration.

Other than that, the patch looks obviously correct.

-Peff

PS There seem to be some other similar declarations: at least
   cmd_upload_tar and cmd_pickaxe.

Junio C Hamano· Jun 15, 2012, 18:09 UTC · re: Jeff King · lore

Re: [PATCH] git-http-fetch: remove unused cmd_http_fetch

Jeff King <peff@peff.net> writes:

> PS There seem to be some other similar declarations: at least
>    cmd_upload_tar and cmd_pickaxe.

Thanks.

$ sed -ne 's/^extern int $cmd_[^(]*$(.*/\1/p' builtin.h | sort >/var/tmp/1 $ nm -g git| sed -ne 's/^[0-9a-f]* T cmd_/cmd_/p' | sort > /var/tmp/2 $ comm -3 /var/tmp/[12]

finds only the two you mentioned, so how about doing it this way?

-- >8 --
From: Luka Perkov <lists@lukaperkov.net>
Date: Thu, 14 Jun 2012 22:23:37 +0200
Subject: [PATCH] builtin.h: remove unused cmd_<foo> declarations

These were left in builtin.h after they were converted into stand-alone programs or removed after experiments finished.

Signed-off-by: Luka Perkov <lists@lukaperkov.net>
Helped-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 builtin.h | 3 ---
 1 file changed, 3 deletions(-)

Show changes to builtin.h +0 −3

diff --git a/builtin.h b/builtin.h
index 20427d2..3e44816 100644
--- a/builtin.h
+++ b/builtin.h
@@ -58,7 +58,6 @@ extern int cmd_gc(int argc, const char **argv, const char *prefix);
 extern int cmd_get_tar_commit_id(int argc, const char **argv, const char *prefix);
 extern int cmd_grep(int argc, const char **argv, const char *prefix);
 extern int cmd_help(int argc, const char **argv, const char *prefix);
-extern int cmd_http_fetch(int argc, const char **argv, const char *prefix);
 extern int cmd_init_db(int argc, const char **argv, const char *prefix);
 extern int cmd_log(int argc, const char **argv, const char *prefix);
 extern int cmd_log_reflog(int argc, const char **argv, const char *prefix);
@@ -76,7 +75,6 @@ extern int cmd_mktree(int argc, const char **argv, const char *prefix);
 extern int cmd_mv(int argc, const char **argv, const char *prefix);
 extern int cmd_name_rev(int argc, const char **argv, const char *prefix);
 extern int cmd_pack_objects(int argc, const char **argv, const char *prefix);
-extern int cmd_pickaxe(int argc, const char **argv, const char *prefix);
 extern int cmd_prune(int argc, const char **argv, const char *prefix);
 extern int cmd_prune_packed(int argc, const char **argv, const char *prefix);
 extern int cmd_push(int argc, const char **argv, const char *prefix);
@@ -104,7 +102,6 @@ extern int cmd_unpack_objects(int argc, const char **argv, const char *prefix);
 extern int cmd_update_index(int argc, const char **argv, const char *prefix);
 extern int cmd_update_ref(int argc, const char **argv, const char *prefix);
 extern int cmd_upload_archive(int argc, const char **argv, const char *prefix);
-extern int cmd_upload_tar(int argc, const char **argv, const char *prefix);
 extern int cmd_verify_tag(int argc, const char **argv, const char *prefix);
 extern int cmd_version(int argc, const char **argv, const char *prefix);
 extern int cmd_whatchanged(int argc, const char **argv, const char *prefix);

-- 
1.7.11.rc3.30.g3bdace2

Jeff King· Jun 15, 2012, 18:16 UTC · re: Junio C Hamano · lore

Re: [PATCH] git-http-fetch: remove unused cmd_http_fetch

On Fri, Jun 15, 2012 at 11:09:53AM -0700, Junio C Hamano wrote:

Show 12 quoted lines

> Jeff King <peff@peff.net> writes:
> 
> > PS There seem to be some other similar declarations: at least
> >    cmd_upload_tar and cmd_pickaxe.
> 
> Thanks.
> 
> $ sed -ne 's/^extern int \(cmd_[^(]*\)(.*/\1/p' builtin.h | sort >/var/tmp/1
> $ nm -g git| sed -ne 's/^[0-9a-f]* T cmd_/cmd_/p' | sort > /var/tmp/2
> $ comm -3 /var/tmp/[12]
> 
> finds only the two you mentioned, so how about doing it this way?

I used a slightly grosser grep that looked in "*.c", and come up with only those 2. Patch looks good to me.

-Peff

Luka Perkov· Jun 15, 2012, 18:40 UTC · re: Jeff King · lore

Re: [PATCH] git-http-fetch: remove unused cmd_http_fetch

On Fri, Jun 15, 2012 at 12:21:36PM -0400, Jeff King wrote:

Show 20 quoted lines

> On Thu, Jun 14, 2012 at 10:23:37PM +0200, Luka Perkov wrote:
> 
> > It was left out from commit 1088261f6fc90324014b5306cca4171987da85ce
> 
> This commit message left me scratching my head. Did we get rid of
> http-fetch? If not, then don't we still need cmd_http_fetch? Or did we
> just make it not a builtin, in which case we wouldn't be getting rid of
> cmd_http_fetch, but rather converting it to main?
> 
> Reading the 1088261, I find the answer: we did make it not a builtin,
> and it was indeed converted into "main". But its _declaration_ hung
> around.
> 
> So maybe a better commit message would be:
> 
>   Subject: builtin.h: drop cmd_http_fetch declaration
> 
>   This was converted from a builtin into a stand-alone program by
>   1088261f6fc90324014b5306cca4171987da85ce, but that commit forgot to
>   drop the declaration.

I agree. Sorry for the confusing message.

Show 6 quoted lines

> Other than that, the patch looks obviously correct.
> 
> -Peff
> 
> PS There seem to be some other similar declarations: at least
>    cmd_upload_tar and cmd_pickaxe.

I noticed only cmd_http_fetch when I was hacking a patch for OpenWrt.

Luka