Re: [RFC PATCHv4] repack: rewrite the shell script in C.

2013-08-21 Thread Johannes Sixt
Am 21.08.2013 00:36, schrieb Stefan Beller: I think I got all the suggestions except the use of git_path/mkpathdup. I replaced mkpathdup by mkpath where possible, but it's still not perfect. I'll wait for the dokumentation patch of Jonathan, before changing all these occurences forth and back aga

Re: [RFC PATCHv4] repack: rewrite the shell script in C.

2013-08-20 Thread Jonathan Nieder
Stefan Beller wrote: > I think I got all the suggestions except the > use of git_path/mkpathdup. > I replaced mkpathdup by mkpath where possible, > but it's still not perfect. No, mkpathdup is generally better unless you know what you're doing. > I'll wait for the dokumentation patch of Jonathan

Re: [RFC PATCHv4] repack: rewrite the shell script in C.

2013-08-20 Thread Stefan Beller
So here is an update of git-repack Thanks for all the reviews and annotations! I think I got all the suggestions except the use of git_path/mkpathdup. I replaced mkpathdup by mkpath where possible, but it's still not perfect. I'll wait for the dokumentation patch of Jonathan, before changing all

Re: [RFC PATCHv4] repack: rewrite the shell script in C.

2013-08-20 Thread Jonathan Nieder
Stefan Beller wrote: > On 08/20/2013 03:31 PM, Johannes Sixt wrote: >>> +packdir = mkpathdup("%s/pack", get_object_directory()); >>> +packtmp = mkpathdup("%s/.tmp-%d-pack", packdir, getpid()); >> >> Should this not be >> >> packdir = xstrdup(git_path("pack")); >> packtmp = xstrdu

Re: [RFC PATCHv4] repack: rewrite the shell script in C.

2013-08-20 Thread Stefan Beller
On 08/20/2013 03:31 PM, Johannes Sixt wrote: > >> +packdir = mkpathdup("%s/pack", get_object_directory()); >> +packtmp = mkpathdup("%s/.tmp-%d-pack", packdir, getpid()); > > Should this not be > > packdir = xstrdup(git_path("pack")); > packtmp = xstrdup(git_path("pack/.tmp-%d-pac

Re: [RFC PATCHv4] repack: rewrite the shell script in C.

2013-08-20 Thread René Scharfe
Am 20.08.2013 17:08, schrieb Stefan Beller: On 08/20/2013 03:31 PM, Johannes Sixt wrote: Are the long forms of options your invention? I tried to keep strong similarity with the shell script for ease of review. In the shellscript the options where put in variables having these names, so for e

Re: [RFC PATCHv4] repack: rewrite the shell script in C.

2013-08-20 Thread Johannes Sixt
Am 20.08.2013 17:08, schrieb Stefan Beller: On 08/20/2013 03:31 PM, Johannes Sixt wrote: You cannot run_command() and then later read its output! You must split it into start_command(), read stdout, finish_command(). Thanks for this hint. Could that explain rare non-deterministic failures in t

Re: [RFC PATCHv4] repack: rewrite the shell script in C.

2013-08-20 Thread Stefan Beller
On 08/20/2013 03:31 PM, Johannes Sixt wrote: > > Are the long forms of options your invention? I tried to keep strong similarity with the shell script for ease of review. In the shellscript the options where put in variables having these names, so for example there was: -f) no_reuse

Re: [RFC PATCHv4] repack: rewrite the shell script in C.

2013-08-20 Thread Johannes Sixt
I didn't look at functions above cmd_repack. Am 20.08.2013 01:23, schrieb Stefan Beller: +int cmd_repack(int argc, const char **argv, const char *prefix) { + + int pack_everything = 0; + int pack_everything_but_loose = 0; + int delete_redundant = 0; + char *unpack_unreach