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

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

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)

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

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

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-pack, getpid()));

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 =

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: 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,

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

2013-08-19 Thread Stefan Beller
Subject: [RFC PATCHv4] repack: rewrite the shell script in C. This is the beginning of the rewrite of the repacking. All tests have been positive at least once now. However there is still a non-deterministic error occuring in about 1 out of 4 test suite runs (usually in 7701 or 9301, but could also

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

2013-08-18 Thread Stefan Beller
This is the beginning of the rewrite of the repacking. * rename get_pack_sha1_list to get_pack_filename_list, which * reads the pack directory only once as suggested by Rene. * fix the grammar as suggested by Kyle. All tests have been positive at least once now. However there is still one