Re: Git issues with submodules
Hey! Sorry for the delayed reply. Am i right the intention is to make it so `git add .` and `git commit .` doesn't include changes to submodule hash unless -f argument is provided? On Sun, Nov 24, 2013 at 10:29 PM, Jens Lehmann wrote: > Am 24.11.2013 01:52, schrieb Heiko Voigt: >> Hi, >> >> On Sat, Nov 23, 2013 at 09:10:44PM +0100, Jens Lehmann wrote: >>> Am 22.11.2013 23:09, schrieb Jonathan Nieder: Heiko Voigt wrote: > After that we can discuss whether add should add submodules that are > tracked but not shown. How about commit -a ? Should it also ignore the > change? I am undecided here. There does not seem to be any good > decision. From the users point of view we should probably not add it > since its not visible in status. What do others think? I agree --- it should not add. >>> >>> I concur: adding a change that is hidden from the user during >>> the process is not a good idea. >> >> Here is a patch achieving that. Still missing a test which I will add. > > Looking good to me. Please add tests for "diff.ignoreSubmodules" > and "submodule..ignore", the latter both in .gitmodules and > .git/config. While doing some testing for this thread I found an > inconsistency in git show which currently honors the submodule > specific option only from .git/config and ignores it in the > .gitmodules file (depending on the outcome of the discussion on > what '--ignore-submodules=all' should ignore we might have to fix > that one afterwards). > > I'd suggest to also add the --ignore-submodules option in another > patch on top, because the user should be able to override the > configuration either way. And what about having the '-f' option > imply '--ignore-submodules=none'? > >> Cheers Heiko >> >> ---8< >> Subject: [PATCH] fix 'git add' to skip submodules configured as ignored >> >> If submodules are configured as ignore=all they are not shown by status. >> Lets also ignore them when adding files to the index. This avoids that >> users accidentially add ignored submodules with: git add . >> >> We achieve this by reading the submodule config and thus correctly >> initializing the infrastructure to take the ignore decision. >> >> Signed-off-by: Heiko Voigt >> --- >> builtin/add.c | 6 ++ >> 1 file changed, 6 insertions(+) >> >> diff --git a/builtin/add.c b/builtin/add.c >> index 226f758..2d0d2ef 100644 >> --- a/builtin/add.c >> +++ b/builtin/add.c >> @@ -15,6 +15,7 @@ >> #include "diffcore.h" >> #include "revision.h" >> #include "bulk-checkin.h" >> +#include "submodule.h" >> >> static const char * const builtin_add_usage[] = { >> N_("git add [options] [--] ..."), >> @@ -378,6 +379,10 @@ static int add_config(const char *var, const char >> *value, void *cb) >> ignore_add_errors = git_config_bool(var, value); >> return 0; >> } >> + >> + if (!prefixcmp(var, "submodule.")) >> + return parse_submodule_config_option(var, value); >> + >> return git_default_config(var, value, cb); >> } >> >> @@ -415,6 +420,7 @@ int cmd_add(int argc, const char **argv, const char >> *prefix) >> int implicit_dot = 0; >> struct update_callback_data update_data; >> >> + gitmodules_config(); >> git_config(add_config, NULL); >> >> argc = parse_options(argc, argv, prefix, builtin_add_options, >> > -- With best regards, Sergey Sharybin -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH] disable complete ignorance of submodules for index <-> HEAD diff
Hi, Tested the patch. `git status` now shows the changes to the submodules, which is nice :) However, is it possible to make it so `git commit` lists submodules in "changes to be committed" section, so you'll see what's gonna to be in the commit while typing the commit message as well? On Sat, Nov 23, 2013 at 7:11 AM, Heiko Voigt wrote: > If the value of ignore for submodules is set to "all" we would not show > whats actually committed during status or diff. This can result in the > user committing unexpected submodule references. Lets be nicer and always > show whats in the index. > > Signed-off-by: Heiko Voigt > --- > This probably needs splitting up into two patches one for the > refactoring and one for the actual fix. It is also missing tests, but I > would first like to know what you think about this approach. > > builtin/diff.c | 43 +++ > diff.h | 2 +- > submodule.c| 6 -- > wt-status.c| 3 +++ > 4 files changed, 35 insertions(+), 19 deletions(-) > > diff --git a/builtin/diff.c b/builtin/diff.c > index adb93a9..e9a356c 100644 > --- a/builtin/diff.c > +++ b/builtin/diff.c > @@ -249,6 +249,21 @@ static int builtin_diff_files(struct rev_info *revs, int > argc, const char **argv > return run_diff_files(revs, options); > } > > +static int have_cached_option(int argc, const char **argv) > +{ > + int i; > + for (i = 1; i < argc; i++) { > + const char *arg = argv[i]; > + if (!strcmp(arg, "--")) > + return 0; > + else if (!strcmp(arg, "--cached") || > +!strcmp(arg, "--staged")) { > + return 1; > + } > + } > + return 0; > +} > + > int cmd_diff(int argc, const char **argv, const char *prefix) > { > int i; > @@ -259,6 +274,7 @@ int cmd_diff(int argc, const char **argv, const char > *prefix) > struct blobinfo blob[2]; > int nongit; > int result = 0; > + int have_cached; > > /* > * We could get N tree-ish in the rev.pending_objects list. > @@ -305,6 +321,11 @@ int cmd_diff(int argc, const char **argv, const char > *prefix) > > if (nongit) > die(_("Not a git repository")); > + > + have_cached = have_cached_option(argc, argv); > + if (have_cached) > + DIFF_OPT_SET(&rev.diffopt, NO_IGNORE_SUBMODULE); > + > argc = setup_revisions(argc, argv, &rev, NULL); > if (!rev.diffopt.output_format) { > rev.diffopt.output_format = DIFF_FORMAT_PATCH; > @@ -319,22 +340,12 @@ int cmd_diff(int argc, const char **argv, const char > *prefix) > * Do we have --cached and not have a pending object, then > * default to HEAD by hand. Eek. > */ > - if (!rev.pending.nr) { > - int i; > - for (i = 1; i < argc; i++) { > - const char *arg = argv[i]; > - if (!strcmp(arg, "--")) > - break; > - else if (!strcmp(arg, "--cached") || > -!strcmp(arg, "--staged")) { > - add_head_to_pending(&rev); > - if (!rev.pending.nr) { > - struct tree *tree; > - tree = > lookup_tree(EMPTY_TREE_SHA1_BIN); > - add_pending_object(&rev, > &tree->object, "HEAD"); > - } > - break; > - } > + if (!rev.pending.nr && have_cached) { > + add_head_to_pending(&rev); > + if (!rev.pending.nr) { > + struct tree *tree; > + tree = lookup_tree(EMPTY_TREE_SHA1_BIN); > + add_pending_object(&rev, &tree->object, "HEAD"); > } > } > > diff --git a/diff.h b/diff.h > index e342325..81561b3 100644 > --- a/diff.h > +++ b/diff.h > @@ -64,7 +64,7 @@ typedef struct strbuf *(*diff_prefix_fn_t)(struct > diff_options *opt, void *data) > #define DIFF_OPT_FIND_COPIES_HARDER (1 << 6) > #define DIFF_OPT_FOLLOW_RENAMES (1 << 7) > #define DIFF_OPT_RENAME_EMPTY(1 << 8) > -/* (1 << 9) unused */ > +#define DIFF_OPT_NO_IGNORE_SUBMODULE (1 << 9) > #define DIFF_OPT_HAS_CHANGES (1 << 10) > #define DIFF_OPT_QUICK (1 << 11) > #define DIFF_OPT_NO_INDEX(1 << 12) > diff --git a/submodule.c b/submodule.c > index 1905d75..9d81712 100644 > --- a/submodule.c > +++ b/submodule.c > @@ -301,9 +301,11 @@ void handle_ignore_submodules_arg(struct diff_options > *diffopt, > DIFF_OPT_CLR(diffopt, IGNORE_UNTRACKED_IN_SUBMODULES); > DIFF_OPT_CLR(diffopt, IGNORE_DIRTY_SUBMODULES); > > - if (!strcmp(arg, "all")) > + if (!st
Re: [PATCH] gitweb: Make showing branches configurable
On Fri, 2013-11-22 at 09:34 -0800, Junio C Hamano wrote: > Krzesimir Nowak writes: > > > Running 'make GITWEB_WANTED_REFS="heads wip" gitweb.cgi' will create a > > gitweb CGI script showing branches that appear in refs/heads/ and in > > refs/wip/. Might be useful for gerrit setups where user branches are > > not stored under refs/heads/. > > > > Signed-off-by: Krzesimir Nowak > > --- > > > > Notes: > > I'm actually not sure if all those changes are really necessary as I > > was mostly targeting it for Gerrit use. Especially I mean the changes > > in git_get_remotes_list, fill_remote_heads and print_page_nav. I tried > > to make it as general as it gets, so there's nothing Gerrit specific. > > Thanks. > > Two knee-jerk reactions after a quick scan. > > - You include "heads" for normal builds by hardcoded >"GITWEB_WANTED_REFS = heads" but include "tags" unconditionally >by having @ref_views = ("tags", @wanted_refs) in the code. Why? > Earlier both "tags" and "heads" were hardcoded there. So now instead of "heads" we have @wanted_refs. I suppose I should have given it a better name, like @branch_refs. Right? > - Does this have be a compile-time decision? It looks like this is >something that can and should be made controllable with the >normal gitweb configuration mechanism. > Maybe. I was just looking at Makefile and saw a bunch of configuration options there, so I just added another one. Haven't noticed the gitweb config thing. Sorry. So, we should just hardcode the @wanted_refs (or @branch_refs after the rename) to simply ('heads'), let it be overriden by perl gitweb config file and get rid of a new substitution from Makefile? > > > gitweb/Makefile| 4 ++- > > gitweb/gitweb.perl | 94 > > +++--- > > 2 files changed, 72 insertions(+), 26 deletions(-) > > > > diff --git a/gitweb/Makefile b/gitweb/Makefile > > index cd194d0..361dce9 100644 > > --- a/gitweb/Makefile > > +++ b/gitweb/Makefile > > @@ -38,6 +38,7 @@ GITWEB_SITE_HTML_HEAD_STRING = > > GITWEB_SITE_HEADER = > > GITWEB_SITE_FOOTER = > > HIGHLIGHT_BIN = highlight > > +GITWEB_WANTED_REFS = heads > > > > # include user config > > -include ../config.mak.autogen > > @@ -148,7 +149,8 @@ GITWEB_REPLACE = \ > > -e > > 's|++GITWEB_SITE_HTML_HEAD_STRING++|$(GITWEB_SITE_HTML_HEAD_STRING)|g' \ > > -e 's|++GITWEB_SITE_HEADER++|$(GITWEB_SITE_HEADER)|g' \ > > -e 's|++GITWEB_SITE_FOOTER++|$(GITWEB_SITE_FOOTER)|g' \ > > - -e 's|++HIGHLIGHT_BIN++|$(HIGHLIGHT_BIN)|g' > > + -e 's|++HIGHLIGHT_BIN++|$(HIGHLIGHT_BIN)|g' \ > > + -e 's|++GITWEB_WANTED_REFS++|$(GITWEB_WANTED_REFS)|g' > > > > GITWEB-BUILD-OPTIONS: FORCE > > @rm -f $@+ > > diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl > > index 68c77f6..8bc9e9a 100755 > > --- a/gitweb/gitweb.perl > > +++ b/gitweb/gitweb.perl > > @@ -17,6 +17,7 @@ use Encode; > > use Fcntl ':mode'; > > use File::Find qw(); > > use File::Basename qw(basename); > > +use List::Util qw(min); > > use Time::HiRes qw(gettimeofday tv_interval); > > binmode STDOUT, ':utf8'; > > > > @@ -122,6 +123,9 @@ our $logo_label = "git homepage"; > > # source of projects list > > our $projects_list = "++GITWEB_LIST++"; > > > > +# list of "directories" under "refs/" we want to display as branches > > +our @wanted_refs = qw{++GITWEB_WANTED_REFS++}; > > + > > # the width (in characters) of the projects list "Description" column > > our $projects_list_description_width = 25; > > > > @@ -632,8 +636,19 @@ sub feature_avatar { > > sub check_head_link { > > my ($dir) = @_; > > my $headfile = "$dir/HEAD"; > > - return ((-e $headfile) || > > - (-l $headfile && readlink($headfile) =~ /^refs\/heads\//)); > > + > > + if (-e $headfile) { > > + return 1; > > + } > > + if (-l $headfile) { > > + my $rl = readlink($headfile); > > + > > + for my $ref (@wanted_refs) { > > + return 1 if $rl =~ /^refs\/$ref\//; > > + } > > + } > > + > > + return 0; > > } > > > > sub check_export_ok { > > @@ -2515,6 +2530,7 @@ sub format_snapshot_links { > > sub get_feed_info { > > my $format = shift || 'Atom'; > > my %res = (action => lc($format)); > > + my $matched_ref = 0; > > > > # feed links are possible only for project views > > return unless (defined $project); > > @@ -2522,12 +2538,17 @@ sub get_feed_info { > > # or don't have specific feed yet (so they should use generic) > > return if (!$action || $action =~ /^(?:tags|heads|forks|tag|search)$/x); > > > > - my $branch; > > - # branches refs uses 'refs/heads/' prefix (fullname) to differentiate > > - # from tag links; this also makes possible to detect branch links > > - if ((defined $hash_base && $hash_base =~ m!^refs/heads/(.*)$!) || > > - (defined $hash && $hash =~ m!^refs/heads/(.*)$!)) { > > - $branch = $1; > > + my $branch =
Re: error: git-remote-https died of signal 13
On Mon, Nov 25, 2013 at 08:20:18AM +0100, Daniel Stenberg wrote: > >Daniel, does the call to Curl_disconnect need to be wrapped with > >sigpipe_ignore/reset, similar to 7d80ed64e435155? > > Yes. It very much looks like that. The SSL "closing" is what was the > problem I had to adress. > > But I then decided that if a 3rd library has one way to generate > SIGPIPE it may very well have another in a separate spot so I decided > to do the wrap at the top level immediately in the entry point when > getting called by the application. Following that, the SIGPIPE > ignore/restore should rather be made in curl_multi_cleanup. Unfortunately, we need an actual SessionHandle to know whether it is OK to reset signals at all. There may be a more elegant way of checking that, but here's the patch series I came up with. It does turn off SIGPIPE for the specific case I'm seeing (again, I'm having trouble actually getting EPIPE in the first place, but from Stefan's strace, I think this would fix his problem). [1/2]: factor out sigpipe_reset from easy.c [2/2]: ignore SIGPIPE during curl_multi_cleanup -Peff -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[curl PATCH 1/2] factor out sigpipe_reset from easy.c
Commit 7d80ed64e43515 introduced some helpers to handle sigpipe in easy.c. However, that fix was incomplete, and we need to add more callers in other files. The first step is making the helpers globally accessible. Since the functions are small and should generally end up inlined anyway, we simply define them in the header as static functions. Signed-off-by: Jeff King --- We can also do it as a separate '.c' file if you prefer. Note that this will #include the system signal.h at a different point in easy.c now (in particular, after we have included more local headers). I don't know if that has any portability implications. I almost wonder if the SIGPIPE_IGNORE definition (and inclusion of signal.h) should go into curl-setup.h. I have very little knowledge of the curl code base and conventions, so please feel free to hack it up or point me in the right direction. lib/Makefile.inc | 2 +- lib/easy.c | 56 +- lib/sigpipe.h| 58 3 files changed, 60 insertions(+), 56 deletions(-) create mode 100644 lib/sigpipe.h diff --git a/lib/Makefile.inc b/lib/Makefile.inc index ec71770..62bc0b3 100644 --- a/lib/Makefile.inc +++ b/lib/Makefile.inc @@ -46,4 +46,4 @@ HHEADERS = arpa_telnet.h netrc.h file.h timeval.h qssl.h hostip.h \ curl_ntlm_msgs.h curl_sasl.h curl_schannel.h curl_multibyte.h \ curl_darwinssl.h hostcheck.h bundles.h conncache.h curl_setup_once.h \ multihandle.h setup-vms.h pipeline.h dotdot.h x509asn1.h gskit.h \ - http2.h + http2.h sigpipe.h diff --git a/lib/easy.c b/lib/easy.c index 1e718ab..1dbdcb7 100644 --- a/lib/easy.c +++ b/lib/easy.c @@ -50,11 +50,6 @@ #include #endif -#if defined(HAVE_SIGNAL_H) && defined(HAVE_SIGACTION) && defined(USE_OPENSSL) -#define SIGPIPE_IGNORE 1 -#include -#endif - #include "strequal.h" #include "urldata.h" #include @@ -78,6 +73,7 @@ #include "warnless.h" #include "conncache.h" #include "multiif.h" +#include "sigpipe.h" #define _MPRINTF_REPLACE /* use our functions only */ #include @@ -85,56 +81,6 @@ /* The last #include file should be: */ #include "memdebug.h" -#ifdef SIGPIPE_IGNORE -struct sigpipe_ignore { - struct sigaction old_pipe_act; - bool no_signal; -}; - -#define SIGPIPE_VARIABLE(x) struct sigpipe_ignore x - -/* - * sigpipe_ignore() makes sure we ignore SIGPIPE while running libcurl - * internals, and then sigpipe_restore() will restore the situation when we - * return from libcurl again. - */ -static void sigpipe_ignore(struct SessionHandle *data, - struct sigpipe_ignore *ig) -{ - /* get a local copy of no_signal because the SessionHandle might not be - around when we restore */ - ig->no_signal = data->set.no_signal; - if(!data->set.no_signal) { -struct sigaction action; -/* first, extract the existing situation */ -memset(&ig->old_pipe_act, 0, sizeof(struct sigaction)); -sigaction(SIGPIPE, NULL, &ig->old_pipe_act); -action = ig->old_pipe_act; -/* ignore this signal */ -action.sa_handler = SIG_IGN; -sigaction(SIGPIPE, &action, NULL); - } -} - -/* - * sigpipe_restore() puts back the outside world's opinion of signal handler - * and SIGPIPE handling. It MUST only be called after a corresponding - * sigpipe_ignore() was used. - */ -static void sigpipe_restore(struct sigpipe_ignore *ig) -{ - if(!ig->no_signal) -/* restore the outside state */ -sigaction(SIGPIPE, &ig->old_pipe_act, NULL); -} - -#else -/* for systems without sigaction */ -#define sigpipe_ignore(x,y) Curl_nop_stmt -#define sigpipe_restore(x) Curl_nop_stmt -#define SIGPIPE_VARIABLE(x) -#endif - /* win32_cleanup() is for win32 socket cleanup functionality, the opposite of win32_init() */ static void win32_cleanup(void) diff --git a/lib/sigpipe.h b/lib/sigpipe.h new file mode 100644 index 000..73fb853 --- /dev/null +++ b/lib/sigpipe.h @@ -0,0 +1,58 @@ +#ifndef HEADER_CURL_SIGPIPE_H +#define HEADER_CURL_SIGPIPE_H + +#include "curl_setup.h" + +#if defined(HAVE_SIGNAL_H) && defined(HAVE_SIGACTION) && defined(USE_OPENSSL) +#include + +struct sigpipe_ignore { + struct sigaction old_pipe_act; + bool no_signal; +}; + +#define SIGPIPE_VARIABLE(x) struct sigpipe_ignore x + +/* + * sigpipe_ignore() makes sure we ignore SIGPIPE while running libcurl + * internals, and then sigpipe_restore() will restore the situation when we + * return from libcurl again. + */ +static void sigpipe_ignore(struct SessionHandle *data, + struct sigpipe_ignore *ig) +{ + /* get a local copy of no_signal because the SessionHandle might not be + around when we restore */ + ig->no_signal = data->set.no_signal; + if(!data->set.no_signal) { +struct sigaction action; +/* first, extract the existing situation */ +memset(&ig->old_pipe_act, 0, sizeof(struct sigaction)); +sigaction(SIGPIPE, NULL, &ig->old_pipe_act); +action
[curl PATCH 2/2] ignore SIGPIPE during curl_multi_cleanup
This is an extension to the fix in 7d80ed64e43515. We may call Curl_disconnect() while cleaning up the multi handle, which could lead to openssl sending packets, which could get a SIGPIPE. Signed-off-by: Jeff King --- I really am just cargo-culting here. I have no idea what multi->closure_handle does, except that it gets used as conn->data for the connection we pass to Curl_disconnect, so it seems like a reasonable place to check for the magic no_signal variable. lib/multi.c | 9 + 1 file changed, 9 insertions(+) diff --git a/lib/multi.c b/lib/multi.c index 923e2ce..2ecb1a4 100644 --- a/lib/multi.c +++ b/lib/multi.c @@ -41,6 +41,7 @@ #include "bundles.h" #include "multihandle.h" #include "pipeline.h" +#include "sigpipe.h" #define _MPRINTF_REPLACE /* use our functions only */ #include @@ -1786,12 +1787,18 @@ CURLMcode curl_multi_cleanup(CURLM *multi_handle) struct SessionHandle *nextdata; if(GOOD_MULTI_HANDLE(multi)) { +SIGPIPE_VARIABLE(pipe); +bool restore_pipe = FALSE; + multi->type = 0; /* not good anymore */ /* Close all the connections in the connection cache */ close_all_connections(multi); if(multi->closure_handle) { + sigpipe_ignore(multi->closure_handle, &pipe); + restore_pipe = TRUE; + multi->closure_handle->dns.hostcache = multi->hostcache; Curl_hostcache_clean(multi->closure_handle, multi->closure_handle->dns.hostcache); @@ -1836,6 +1843,8 @@ CURLMcode curl_multi_cleanup(CURLM *multi_handle) Curl_pipeline_set_server_blacklist(NULL, &multi->pipelining_server_bl); free(multi); +if (restore_pipe) + sigpipe_restore(&pipe); return CURLM_OK; } -- 1.8.5.rc3.491.ge1614cf -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
branch annotations?
Is there any way to associate some sort of note with a branch that would be shown when listing them? While I currently have things like "issue/QA-42", it would be nice to have a note associated with it so I could do something like $ git branch --show-notes issue/CR-88: make sure NoSQL engines support .CSV file backends issue/QA-31: add missile launch codes * issue/QA-42: support flying cars master as I currently need to flip back and forth between my git/terminal and my issue-tracker to know that, if I need to be adding missile launch codes, I need to be working on QA-31. I know there are "note" features elsewhere for commits, and I know that git-branch supports showing the most recent commit (that's not always enough info to discern the branch's purpose). Thanks for any ideas on how to ease this pain-point, :-) -tkc -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: error: git-remote-https died of signal 13
On Mon, Nov 25, 2013 at 09:32:13AM -0500, Jeff King wrote: > > But I then decided that if a 3rd library has one way to generate > > SIGPIPE it may very well have another in a separate spot so I decided > > to do the wrap at the top level immediately in the entry point when > > getting called by the application. Following that, the SIGPIPE > > ignore/restore should rather be made in curl_multi_cleanup. > > Unfortunately, we need an actual SessionHandle to know whether it is OK > to reset signals at all. There may be a more elegant way of checking > that, but here's the patch series I came up with. Scratch that. I had originally written something like: if (conn->data) sigpipe_ignore(conn->data, &pipe); Curl_disconnect(conn, ...); sigpipe_restore(&pipe); but while sending it out, I realized that the "data" we attach to each connection when we close it is just the multi->closure_handle. So I was able to hoist the check out to curl_multi_cleanup (and that's what I just sent). -Peff -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: branch annotations?
On Mon, Nov 25, 2013 at 3:46 PM, Tim Chase wrote: > Is there any way to associate some sort of note with a branch that > would be shown when listing them? While I currently have things like > "issue/QA-42", it would be nice to have a note associated with it so > I could do something like > > $ git branch --show-notes > issue/CR-88: make sure NoSQL engines support .CSV file backends > issue/QA-31: add missile launch codes > * issue/QA-42: support flying cars > master > > as I currently need to flip back and forth between my git/terminal > and my issue-tracker to know that, if I need to be adding missile > launch codes, I need to be working on QA-31. I know there are "note" > features elsewhere for commits, and I know that git-branch supports > showing the most recent commit (that's not always enough info to > discern the branch's purpose). > > Thanks for any ideas on how to ease this pain-point, :-) "git branch --edit-description" allows you to write a descriptive string for your branch. AFAICS, however, it currently only shows up when using request-pull. It does not show up in any git branch command. IMHO that should be fixed. As a workaround, you can use this one-liner: git for-each-ref --format "%(refname:short)" refs/heads/ | while read branch; do echo "$branch - $(git config branch.$branch.description)"; done I guess that could even be turned into an alias... Hope this helps, ...Johan -- Johan Herland, www.herland.net -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: branch annotations?
On Mon, Nov 25, 2013 at 08:46:44AM -0600, Tim Chase wrote: > Is there any way to associate some sort of note with a branch that > would be shown when listing them? While I currently have things like > "issue/QA-42", it would be nice to have a note associated with it so > I could do something like > > $ git branch --show-notes > issue/CR-88: make sure NoSQL engines support .CSV file backends > issue/QA-31: add missile launch codes > * issue/QA-42: support flying cars > master There is a "branch description" config entry that you can edit with "git branch --edit-description" (or by simply tweaking the config file yourself). However, it is currently only used by git-request-pull when generating the request. I think it makes sense to be able to show it as part of "git branch", but the verbose branch listing there is a bit of a mess. Doing it cleanly would probably involve refactoring the branch-display code to allow users to specify more flexible formats. Ramkumar (cc'd) was looking into that refactoring a while back, but I did not follow it closely (it looks like some of the underlying for-each-ref refactoring is on the 'next' branch?). -Peff -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: branch annotations?
On 2013-11-25 15:55, Johan Herland wrote: > "git branch --edit-description" allows you to write a descriptive > string for your branch. AFAICS, however, it currently only shows up > when using request-pull. It does not show up in any git branch > command. IMHO that should be fixed. > > As a workaround, you can use this one-liner: > > git for-each-ref --format "%(refname:short)" refs/heads/ | while > read branch; do echo "$branch - $(git config > branch.$branch.description)"; done That works like a charm. Thanks! -tkc -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Test suite fails if temporary space is mounted noexec
Hi, tl;dr: The filesystem used as temporary space for the Git test suite must not be mounted "noexec". It took me a while to figure out why many tests in the Git test suite were failing on my new notebook computer. The failures started right in test t: [15:06:31] t-basic.sh . Dubious, test returned 1 (wstat 256, 0x100) Failed 10/59 subtests (less 5 skipped subtests: 44 okay) and maybe 5% of the other test scripts were failing, too. The reason is that I was using the tmpfs (RAM filesystem) at /run/shm as temporary space for running the tests. That speeds the tests up quite a bit relative to using a hard disk as temp space. I had options like the following in config.mak: DEFAULT_TEST_TARGET = prove GIT_TEST_OPTS = -q --root=/run/shm --tee GIT_PROVE_OPTS = --timer --jobs 5 --state=fresh,hot,slow,save This used to work, but my new notebook is running Debian testing, which mounts /run/shm with the "noexec" option. Since some of the tests in the test suite write temporary scripts to the temporary space then try to execute them, they were failing on this filesystem. In fact, my old setup was deficient in several ways: * /run/shm isn't really meant to be used as temporary disk space in the first place * Lots of temporary directories were being created directly in the top-level /run/shm directory rather than being tucked in a subdirectory * It was unreliable to run the test suite in different working copies simultaneously (because they might clobber each others temporary files) To fix these problems, I am now doing the following: 1. Have Debian mount a tmpfs at /tmp rather than using an on-disk /tmp directory. To do this, I added a file /etc/fstab.d/tmp.fstab containing the following line (see tmpfs(5) for more information): tmpfs /tmp tmpfs nodev,nosuid,size=25%,mode=177700 2. Change the above lines in config.mak to the following: TMP := /tmp/git-tests-$(shell git rev-parse --show-toplevel | sha1sum | head -c10) DEFAULT_TEST_TARGET = prove GIT_TEST_OPTS = -q --root=$(TMP) --tee GIT_PROVE_OPTS = --timer --jobs 16 --state=fresh,hot,slow,save The idea is that the path of the working copy is encoded in the name of the directory used for temporary space, so test suites for multiple working copies can be run simultaneously. The test suite creates this directory automatically if it doesn't already exist (however, it doesn't remove it when the tests are over). I also boosted the parallelism of prove from 5 to 16. With these settings, the test suite completes in about 33 seconds for me, including the cvsimport tests but not the svn or p4 tests. I hope that is useful to somebody, Michael -- Michael Haggerty mhag...@alum.mit.edu http://softwareswirl.blogspot.com/ -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 18/24] read-cache: write index-v5
[sorry about taking so much time to get back to you, was too busy with other stuff to work on git] Duy Nguyen writes: > On Mon, Aug 19, 2013 at 2:42 AM, Thomas Gummerer wrote: >> +char *super_directory(const char *filename) >> +{ >> + char *slash; >> + >> + slash = strrchr(filename, '/'); >> + if (slash) >> + return xmemdupz(filename, slash-filename); >> + return NULL; >> +} >> + > > Why is this function not static? There are a few other in this patch > (I did not notice in others, but I wasn't looking for them..) That's just an oversight, it should be static. Will look for the others and change them to static too. > But isn't this what dirname() is? > > Another point about this function is it returns a new allocated > string, but I see no free() anywhere in this patch. Leak alert! Yes, you're right, I'll use dirname instead. That also solves the free problem, as dirname modifies the string. I'll still use a wrapper around it, because dirname returns "." when there is no super_directory, but using NULL for that makes it simpler to check it. >> +struct directory_entry *init_directory_entry(char *pathname, int len) >> +{ >> + struct directory_entry *de = xmalloc(directory_entry_size(len)); >> + >> + memcpy(de->pathname, pathname, len); >> + de->pathname[len] = '\0'; >> + de->de_flags = 0; >> + de->de_foffset= 0; >> + de->de_cr = 0; >> + de->de_ncr= 0; >> + de->de_nsubtrees = 0; >> + de->de_nfiles = 0; >> + de->de_nentries = 0; >> + memset(de->sha1, 0, 20); >> + de->de_pathlen= len; >> + de->next = NULL; >> + de->next_hash = NULL; >> + de->ce= NULL; >> + de->ce_last = NULL; >> + de->conflict = NULL; >> + de->conflict_last = NULL; >> + de->conflict_size = 0; >> + return de; >> +} > > I think this function could be shortened to > > struct directory_entry *de = xcalloc(1, directory_entry_size(len)); > memcpy(de->pathname, pathname, len); > de->de_pathlen = len; > return de; Makes sense, thanks. >> +static struct directory_entry *get_directory(char *dir, unsigned int >> dir_len, >> +struct hash_table *table, >> +unsigned int *total_dir_len, >> +unsigned int *ndir, >> +struct directory_entry >> **current) >> +{ >> + struct directory_entry *tmp = NULL, *search, *new, *ret; >> + uint32_t crc; >> + >> + search = find_directory(dir, dir_len, &crc, table); >> + if (search) >> + return search; >> + while (!search) { >> + new = init_directory_entry(dir, dir_len); >> + insert_directory_entry(new, table, total_dir_len, ndir, crc); >> + if (!tmp) >> + ret = new; >> + else >> + new->de_nsubtrees = 1; >> + new->next = tmp; >> + tmp = new; >> + dir = super_directory(dir); > > It feels more natural to create directory_entry(s) from parent to > subdir. If you do so you could reset dir to the remaining of directory > and perform strchr() and do not need to allocate new memory everytime > you call super_directory (because it relies on NUL at the end of the > string). I'm creating them from subdir to parent because it saves a few calls whenever a superdir is already in the hash_table. And I'm using dirname so the new memory allocations are not a problem anymore. >> + dir_len = dir ? strlen(dir) : 0; >> + search = find_directory(dir, dir_len, &crc, table); >> + } >> + search->de_nsubtrees++; >> + (*current)->next = tmp; >> + while ((*current)->next) >> + *current = (*current)->next; >> + >> + return ret; >> +} > >> +static struct directory_entry *compile_directory_data(struct index_state >> *istate, >> + int nfile, >> + unsigned int *ndir, >> + unsigned int >> *total_dir_len, >> + unsigned int >> *total_file_len) >> +{ >> + int i, dir_len = -1; >> + char *dir; >> + struct directory_entry *de, *current, *search; >> + struct cache_entry **cache = istate->cache; >> + struct conflict_entry *conflict_entry; >> + struct hash_table table; >> + uint32_t crc; >> + >> + init_hash(&table); >> + de = init_directory_entry("", 0); >> + current = de; >> + *ndir = 1; >> + *total_dir_len = 1; >> + crc = crc32(0, (Bytef*)de->pathname, de->de_pathlen); >> + insert_hash(crc, de, &table); >> +
Re: [PATCH v3 17/24] read-cache: read cache-tree in index-v5
Duy Nguyen writes: > On Mon, Aug 19, 2013 at 2:42 AM, Thomas Gummerer wrote: >> +/* >> + * This function modifies the directory argument that is given to it. >> + * Don't use it if the directory entries are still needed after. >> + */ > > There goes my hope of keeping directory_entry* in core so that at > write-time, after validation, we only need to recreate some trees > instead of all of them.. > > Or we could make cache-tree keep references to directory_entry. If a > cache-tree is not invalidated, then the attached directory_tree should > be reused.. I've now re-written the algorithm that converts the directory entries to cache entries, and it's now no longer destructive. For now the directory entries are not needed in core, so I'll free them when done with reading the index, but it's possible to keep it. >> +static struct cache_tree *cache_tree_convert_v5(struct directory_entry *de) >> +{ >> + if (!de->de_nentries) >> + return NULL; >> + sort_directories(de); >> + return convert_one(de); >> +} >> + >> static int read_entries(struct index_state *istate, struct directory_entry >> *de, >> unsigned int first_entry_offset, void *mmap, >> unsigned long mmap_size, unsigned int *nr, >> @@ -591,6 +668,7 @@ static int read_index_v5(struct index_state *istate, >> void *mmap, >> } >> de = de->next; >> } >> + istate->cache_tree = cache_tree_convert_v5(root_directory); >> istate->cache_nr = nr; >> return 0; >> } > > Otherwise we do need to free root_directory down to the deepest > subtrees, I think. People have been complaining about read-cache > leaking memory like mad, so this is a real issue. Even if you keep > references in cache-tree, you still need to free it > cache_tree_invalidate_path() to avoid leaking I'm freeing them for now, as they are not used anywhere, but in the future we might want to keep them for some optimizations. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] Document khash
For squashing into a commit that adds khash.h. Signed-off-by: Thomas Rast --- > I think I'll also lend you a hand writing > Documentation/technical/api-khash.txt > (expect it tomorrow) so that we also have documentation in the git > style, where gitters can be expected to find it on their own. Here goes. > Furthermore, would it be a problem to name the second hash sha1_int > instead? I have another use for such a hash, and I can't imagine I'm > the only one. (That's not critical however, I can do the required > editing in that other series.) Actually, let's not do that. Since everything is 'static inline' anyway, there's no cost to simply instantiating more hashes as needed. Documentation/technical/api-khash.txt | 109 ++ 1 file changed, 109 insertions(+) create mode 100644 Documentation/technical/api-khash.txt diff --git a/Documentation/technical/api-khash.txt b/Documentation/technical/api-khash.txt new file mode 100644 index 000..e7ca883 --- /dev/null +++ b/Documentation/technical/api-khash.txt @@ -0,0 +1,109 @@ +khash += + +The khash API is a collection of macros that instantiate hash table +routines for arbitrary key/value types. + +It lives in 'khash.h', which we imported from + https://github.com/attractivechaos/klib/blob/master/khash.h +and then gitified somewhat. + +Usage +- + + +#import "khash.h" +KHASH_INIT(NAME, key_t, value_t, is_map, key_hash_fn, key_equal_fn) + + +The arguments are as follows: + +`NAME`:: + Used to give your hash and its API a unique name. Spelled + `NAME` here to set it apart from the rest of the names, but + generally should be a lowercase C identifier. + +`key_t`:: + Type of the keys for your hashes. Generally should be + `const`. + +`value_t`:: + Type of the values of your hashes. + +`is_map`:: + Whether the hash holds values. Set to 0 to create a hash for + use as a set. + +`khint_t key_hash_fn(key_t)`:: + Hash function. + +`int key_equal_fn(key_t a, key_t b)`:: + Comparison function. Return 1 if the two keys are the same, 0 + otherwise. + +These two functions may also be macros. + + +API +--- + +The above instantiation defines a single type: + +`kh_NAME_t`:: + A struct that holds your hash. You should only ever use + pointers to it. + +After the above instantiation, the following functions and +function-like macros are defined: + +`kh_NAME_t *kh_init_NAME(void)`:: + Allocate and initialize a new hash table. + +`void kh_destroy_NAME(kh_NAME_t *)`:: + Free the hash table. + +`void kh_clear_NAME(kh_NAME_t *)`:: + Clear (but do not free) the hash table. + +`khint_t kh_get_NAME(const kh_NAME_t *hash, key_t key)`:: + Find the given key in the hash table. The returned khint_t + should be treated as an opaque iterator token that indexes + into the hash. Use `kh_value` to get the value from the + token. If the key does not exist, returns kh_end(hash). + +`khint_t kh_put_NAME(const kh_NAME_t *hash, key_t key, int *ret)`:: + Put the given key in the hash table. The returned khint_t + should be treated as an opaque iterator token that indexes + into the hash. Use `kh_value(hash, token) = value` to assign + a value after putting the key. ++ +'ret' tells you whether the key already existed: it is -1 if the +operation failed; 0 if the key was already present; 1 if the bucket is +empty; 2 if the bucket has been deleted. + +`kh_key(kh_NAME_t *, khint_t)`:: + The key slot for the given iterator token. This can be used + as an lvalue, but you must not change the key! + +`kh_value(kh_NAME_t *, khint_t)`:: +`kh_val(kh_NAME_t *, khint_t)`:: + The value slot for the given iterator token. This can be used + as an lvalue. + +`int kh_exist(const kh_NAME_t *, key_t)`:: + Tests if the given bucket contains data. + +`khint_t kh_begin(const kh_NAME_t *)`:: + The smallest iterator token. + +`khint_t kh_end(const kh_NAME_t *)`:: + The beyond-the-hash iterator token; it is one larger than the + largest token that points to a hash member. + +`khint_t kh_size(const kh_NAME_t *)`:: + The number of elements in the hash. + +`kh_foreach(const kh_NAME_t *, keyvar, valuevar, { ... loop body ... })`:: + Iterate over every key-value pair in the hash. This is a + macro, and keyvar and valuevar must be pre-declared of type + key_t and value_t, respectively. -- 1.8.5.rc3.397.g2a3acd5 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
возверните себе мочь блестяще различать
полезный рецепт спасти глазики http://goo.gl/csFULt -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: What's cooking in git.git (Nov 2013, #05; Thu, 21)
Antoine Pelisse writes: > On Fri, Nov 22, 2013 at 1:19 AM, Junio C Hamano wrote: >> * rh/remote-hg-bzr-updates (2013-11-18) 9 commits >> (merged to 'next' on 2013-11-20 at a36f3c4) >> + remote-bzr, remote-hg: fix email address regular expression >> + test-hg.sh: help user correlate verbose output with email test >> + test-hg.sh: fix duplicate content strings in author tests >> + test-hg.sh: avoid obsolete 'test' syntax >> + test-hg.sh: eliminate 'local' bashism >> + test-bzr.sh, test-hg.sh: prepare for change to push.default=simple >> + test-bzr.sh, test-hg.sh: allow running from any dir >> + test-lib.sh: convert $TEST_DIRECTORY to an absolute path >> + remote-hg: don't decode UTF-8 paths into Unicode objects >> >> Can wait in 'next'. > > Would it be possible to merge the first commit of this series in > master (and eventually in maint) ? > My commit (11362653: remote-hg: unquote C-style paths when exporting) > breaks the remote-hg tests since v1.8.4.3 (sorry about that), and is > fixed by this commit. It would be nice to deliver 1.8.5 with working > remote-helpers tests. Surely. Let's do that. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] bash prompt: add option to disable for a repository
Hello, On Sat, Nov 23, 2013 at 6:35 PM, SZEDER Gábor wrote: > Hm, strange. I wonder what can cause performance problems in big > repositories. > > Sure, there are status indicators that can be expensive, in particular > the indicators for dirty index/worktree, untracked files, and > divergence from upstream. However, these must be enabled globally by > environment variables and even then can already be disabled on a > per-repo basis by configuration variables. And the rest of the prompt > code should perform pretty much independently from the repository > size. You are right. The performance issue seems to be indeed fixed by setting bash.showDirtyState and bash.showUntrackedFiles to false. And I feel a bit stupid for not realizing those were the only reason :) Now the only remaining issue for me is that I wouldn't like to see git prompt under the home directory repository, because then it's turned on pretty much everywhere. > I spent quite some time eliminating fork()s and exec()s from the > prompt, so a fork() for the command substitution's subshell and a > fork()+exec() for running a git command in the main code path saddens > me deeply ;) Seeing how the dirty state/untracked files/upstream configs are implemented, I'm thinking, what if "bash.prompt" setting was checked similarly only when something like GIT_PS1_PERREPOBASIS was set? It would keep the default execution path free from added forks, but still allow people to disable git prompt on a per-repository basis. Regards, -- Heikki Hokkanen -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: slow git-cherry-pick.
Paweł Sikora writes: > On Sunday 24 of November 2013 19:47:10 Duy Nguyen wrote: >> On Sun, Nov 24, 2013 at 5:45 PM, Paweł Sikora wrote: >> > i've recently reinstalled a fresh system (fc20-beta) on my workstation >> > and observing a big slowdown on git cherry-pick operation (git-1.8.4.2-1). >> > the previous centos installation with an old git version works faster >> > (few seconds per cherry pick). now the same operation takes >1 min. >> >> What is the git version before the reinstallation? > > git-1.7.11.3-1.el5.rf. > > i've checked this version on another machine with centos-5.$latest > and it does similar amout of stat/read operation quickly (~6s). > this "fast" centos-5 machine has /home on raid-0 (2x500GB) while > my "slow (>1min)" workstation has /home on linear lvm (250G+1T). > > so, i suppose that my "slow" working copy crosses disks boundary > or spread over 1TB drive and the random git i/o impacts performance. > > the question still remains - does the git need to scan whole checkout > during picking well defined set of files? We do update-index to see what local changes you have upfront in order to avoid stomping on them (and we do not know upfront what paths the cherry-picked commit would change, given that there may be renames involved), so the answer is unfortunately yes, we would need to do lstat(2) the whole thing. Doing that lstat(2) more lazily and do away with the update-index might be possible, but I suspect that may be quite a lot of work. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Re: Git issues with submodules
On Mon, Nov 25, 2013 at 03:02:51PM +0600, Sergey Sharybin wrote: > Am i right the intention is to make it so `git add .` and `git commit > .` doesn't include changes to submodule hash unless -f argument is > provided? Yes thats the goal. My patch currently only disables it when ignore is set to all. I will add another patch that implements the -f and --submodule-ignore option to both of them so the user has an easy way to bypass that. But having said that we changing existing behavior here so we have to investigate carefully whether we are not breaking peoples expectations (and script). That also applies to the other patch that enables showing them in diff and friends again. Cheers Heiko -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Re: Git issues with submodules
Heiko, yeah sure see what you mean. Changing existing behavior is pretty PITA. Just one more question for now, are you referencing to the patch "[RFC PATCH] disable complete ignorance of submodules for index <-> HEAD diff"? Coz i tested it and seems it doesn't change behavior of add/commit. Also, i'm around to test the all patches which are related on submodules :) On Mon, Nov 25, 2013 at 11:49 PM, Heiko Voigt wrote: > On Mon, Nov 25, 2013 at 03:02:51PM +0600, Sergey Sharybin wrote: >> Am i right the intention is to make it so `git add .` and `git commit >> .` doesn't include changes to submodule hash unless -f argument is >> provided? > > Yes thats the goal. My patch currently only disables it when ignore is > set to all. I will add another patch that implements the -f and > --submodule-ignore option to both of them so the user has an easy way to > bypass that. But having said that we changing existing behavior here so > we have to investigate carefully whether we are not breaking peoples > expectations (and script). That also applies to the other patch > that enables showing them in diff and friends again. > > Cheers Heiko -- With best regards, Sergey Sharybin -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Re: Re: Git issues with submodules
On Mon, Nov 25, 2013 at 11:57:45PM +0600, Sergey Sharybin wrote: > Heiko, yeah sure see what you mean. Changing existing behavior is pretty PITA. > > Just one more question for now, are you referencing to the patch "[RFC > PATCH] disable complete ignorance of submodules for index <-> HEAD > diff"? Coz i tested it and seems it doesn't change behavior of > add/commit. Yep, that was just an RFC for status and diff. I think teaching add and commit to skip submodules if ignored are a separate topic and thus will be in a separate patch. I have to add tests and probably some more commands. The logic of ignoring submodules is implemented quite deep in the diff code. So changing it can affect quite some commands so we have to check quite carefully what will be affected and if we can change it without to much fallout. > Also, i'm around to test the all patches which are related on submodules :) Thanks, good to know. Stay tuned! Cheers Heiko -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: branch annotations?
Jeff King wrote: > I think it makes sense to be able to show it as part of "git branch", > but the verbose branch listing there is a bit of a mess. Doing it > cleanly would probably involve refactoring the branch-display code to > allow users to specify more flexible formats. Certainly. I'm quite unhappy with the current 'git branch', and am looking to improve it. Branch descriptions will definitely be useful to display as well. > Ramkumar (cc'd) was looking into that refactoring a while back, but I > did not follow it closely (it looks like some of the underlying > for-each-ref refactoring is on the 'next' branch?). The previous effort in collaboration with Duy fell apart, unfortunately. Currently, there are a few small patches enhancing f-e-r from the original series in 'next'. Once they graduate, the plan is to refactor f-e-r a bit so that 'git branch' can slowly be moved into using its machinery. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] drop support for "experimental" loose objects
Jeff King writes: > On Sun, Nov 24, 2013 at 03:44:44AM -0500, Jeff King wrote: > >> In any code path where we call parse_object, we double-check that the >> result matches the sha1 we asked for. But low-level commands like >> cat-file just call read_sha1_file directly, and do not have such a >> check. We could add it, but I suspect the processing cost would be >> noticeable. > > Curious, I tested this. It is noticeable. Here's the best-of-five > timings for the patch below when running a --batch cat-file on every > object in my git.git repo, using the patch below: > > [before] > real0m12.941s > user0m12.700s > sys 0m0.244s > > [after] > real0m15.800s > user0m15.472s > sys 0m0.344s > > So it's about 20% slower. I don't know what the right tradeoff is. It's > cool to check the data each time we look at it, but it does carry a > performance penalty. > diff --git a/builtin/cat-file.c b/builtin/cat-file.c > index b2ca775..2b09773 100644 > --- a/builtin/cat-file.c > +++ b/builtin/cat-file.c > @@ -199,6 +199,8 @@ static void print_object_or_die(int fd, const unsigned > char *sha1, > if (type == OBJ_BLOB) { > if (stream_blob_to_fd(fd, sha1, NULL, 0) < 0) > die("unable to stream %s to stdout", sha1_to_hex(sha1)); > + if (check_sha1_signature(sha1, NULL, 0, NULL) < 0) > + die("object %s sha1 mismatch", sha1_to_hex(sha1)); check_sha1_signature() opens the object again and streams the data. Essentially the read side is doing twice the work with that patch, isn't it? I wonder if we want to extend the stream_blob_to_fd() API to optionally allow the caller to ask to validate that the returned data is consistent with the object name the caller asked the data for. Something along the lines of the attached weatherbaloon patch? builtin/fsck.c | 3 ++- entry.c| 2 +- streaming.c| 29 - streaming.h| 4 +++- 4 files changed, 34 insertions(+), 4 deletions(-) diff --git a/builtin/fsck.c b/builtin/fsck.c index 97ce678..f42ed9c 100644 --- a/builtin/fsck.c +++ b/builtin/fsck.c @@ -237,7 +237,8 @@ static void check_unreachable_object(struct object *obj) if (!(f = fopen(filename, "w"))) die_errno("Could not open '%s'", filename); if (obj->type == OBJ_BLOB) { - if (stream_blob_to_fd(fileno(f), obj->sha1, NULL, 1)) + if (stream_blob_to_fd(fileno(f), obj->sha1, NULL, + STREAMING_OUTPUT_CAN_SEEK)) die_errno("Could not write '%s'", filename); } else fprintf(f, "%s\n", sha1_to_hex(obj->sha1)); diff --git a/entry.c b/entry.c index 7b7aa81..b3bc827 100644 --- a/entry.c +++ b/entry.c @@ -127,7 +127,7 @@ static int streaming_write_entry(const struct cache_entry *ce, char *path, if (fd < 0) return -1; - result |= stream_blob_to_fd(fd, ce->sha1, filter, 1); + result |= stream_blob_to_fd(fd, ce->sha1, filter, STREAMING_OUTPUT_CAN_SEEK); *fstat_done = fstat_output(fd, state, statbuf); result |= close(fd); diff --git a/streaming.c b/streaming.c index debe904..50599df 100644 --- a/streaming.c +++ b/streaming.c @@ -2,6 +2,7 @@ * Copyright (c) 2011, Google Inc. */ #include "cache.h" +#include "object.h" #include "streaming.h" enum input_source { @@ -496,19 +497,33 @@ static open_method_decl(incore) / int stream_blob_to_fd(int fd, unsigned const char *sha1, struct stream_filter *filter, - int can_seek) + unsigned flags) { struct git_istream *st; enum object_type type; unsigned long sz; ssize_t kept = 0; int result = -1; + int can_seek = flags & STREAMING_OUTPUT_CAN_SEEK; + + int want_verify = flags & STREAMING_VERIFY_OBJECT_NAME; + git_SHA_CTX ctx; st = open_istream(sha1, &type, &sz, filter); if (!st) return result; if (type != OBJ_BLOB) goto close_and_exit; + + if (want_verify) { + char hdr[32]; + int hdrlen; + + git_SHA1_Init(&ctx); + hdrlen = sprintf(hdr, "%s %lu", typename(type), sz) + 1; + git_SHA1_Update(&ctx, hdr, hdrlen); + } + for (;;) { char buf[1024 * 16]; ssize_t wrote, holeto; @@ -518,6 +533,10 @@ int stream_blob_to_fd(int fd, unsigned const char *sha1, struct stream_filter *f goto close_and_exit; if (!readlen) break; + + if (want_verify) + git_SHA1_Update(&ctx, buf, readlen); +
[PATCH 1/2] commit-slab: document clear_$slabname()
The clear_$slabname() function was only documented by source code so far. Write something about it. Signed-off-by: Thomas Rast --- commit-slab.h | 4 1 file changed, 4 insertions(+) diff --git a/commit-slab.h b/commit-slab.h index d4c8286..d77aaea 100644 --- a/commit-slab.h +++ b/commit-slab.h @@ -24,6 +24,10 @@ * to each commit. 'stride' specifies how big each array is. The slab * that id initialied by the variant without "_with_stride" associates * each commit with an array of one integer. + * + * - void clear_indegree(struct indegree *); + * + * Free the slab's data structures. */ /* allocate ~512kB at once, allowing for malloc overhead */ -- 1.8.5.rc3.397.g2a3acd5 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/2] commit-slab: declare functions "static inline"
This shuts up compiler warnings about unused functions. While there, also remove the redundant second declaration of stat_##slabname##realloc. Signed-off-by: Thomas Rast --- commit-slab.h | 17 - 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/commit-slab.h b/commit-slab.h index d77aaea..d5c353e 100644 --- a/commit-slab.h +++ b/commit-slab.h @@ -45,8 +45,8 @@ struct slabname { \ }; \ static int stat_ ##slabname## realloc; \ \ -static void init_ ##slabname## _with_stride(struct slabname *s, \ - unsigned stride)\ +static inline void init_ ##slabname## _with_stride(struct slabname *s, \ + unsigned stride) \ { \ unsigned int elem_size; \ if (!stride)\ @@ -58,12 +58,12 @@ struct slabname { \ s->slab = NULL; \ } \ \ -static void init_ ##slabname(struct slabname *s) \ +static inline void init_ ##slabname(struct slabname *s) \ { \ init_ ##slabname## _with_stride(s, 1); \ } \ \ -static void clear_ ##slabname(struct slabname *s) \ +static inline void clear_ ##slabname(struct slabname *s) \ { \ int i; \ for (i = 0; i < s->slab_count; i++) \ @@ -73,8 +73,8 @@ struct slabname { \ s->slab = NULL; \ } \ \ -static elemtype *slabname## _at(struct slabname *s,\ - const struct commit *c) \ +static inline elemtype *slabname## _at(struct slabname *s, \ + const struct commit *c) \ { \ int nth_slab, nth_slot; \ \ @@ -94,8 +94,7 @@ struct slabname { \ s->slab[nth_slab] = xcalloc(s->slab_size, \ sizeof(**s->slab) * s->stride); \ return &s->slab[nth_slab][nth_slot * s->stride]; \ -} \ - \ -static int stat_ ##slabname## realloc +} + #endif /* COMMIT_SLAB_H */ -- 1.8.5.rc3.397.g2a3acd5 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 0/2] commit-slab cleanups
I gathered these while writing an "all merge bases simultaneously" algorithm. Turns out this fancy algorithm loses vs. simply calling get_merge_bases() a lot, so I dropped that. But the cleanups seem valid anyway. Thomas Rast (2): commit-slab: document clear_$slabname() commit-slab: declare functions "static inline" commit-slab.h | 21 - 1 file changed, 12 insertions(+), 9 deletions(-) -- 1.8.5.rc3.397.g2a3acd5 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: gettext CTYPE for libc
Duy Nguyen wrote: > Do you think it's ok to revert the workaround if we detect > the running glibc is fixed (or if it does not run glibc at all)? I > think we could use gnu_get_libc_version() to detect it. That would be wonderful. Thanks, Jonathan -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] gitweb: Make showing branches configurable
Krzesimir Nowak writes: > On Fri, 2013-11-22 at 09:34 -0800, Junio C Hamano wrote: >> Krzesimir Nowak writes: >> >> > Running 'make GITWEB_WANTED_REFS="heads wip" gitweb.cgi' will create a >> > gitweb CGI script showing branches that appear in refs/heads/ and in >> > refs/wip/. Might be useful for gerrit setups where user branches are >> > not stored under refs/heads/. >> > >> > Signed-off-by: Krzesimir Nowak >> > --- >> > >> > Notes: >> > I'm actually not sure if all those changes are really necessary as I >> > was mostly targeting it for Gerrit use. Especially I mean the changes >> > in git_get_remotes_list, fill_remote_heads and print_page_nav. I tried >> > to make it as general as it gets, so there's nothing Gerrit specific. >> >> Thanks. >> >> Two knee-jerk reactions after a quick scan. >> >> - You include "heads" for normal builds by hardcoded >>"GITWEB_WANTED_REFS = heads" but include "tags" unconditionally >>by having @ref_views = ("tags", @wanted_refs) in the code. Why? >> > > Earlier both "tags" and "heads" were hardcoded there. So now instead of > "heads" we have @wanted_refs. > > I suppose I should have given it a better name, like @branch_refs. > Right? My original question was why the change was not done like this: In gitweb/Makefile, to give the default that is the same as before: GITWEB_WANTED_REFS = heads tags In format_refs_views my @ref_views = @wanted_refs; But looking at the existing code again, you are only interested in renaming 'heads' to some other possibly multiple hierarchies, so that would not fly well. Indeed, as you said, "wanted refs" is a misnomer that led to the above confusion. If it is only about what are the branches, then the configuration should be named after the "branch" ness of that thing. But that leads to another question. Is there _any_ use case where showing 'heads/' hierarchy is undesirable? It would be utterly confusing if something that claims to show branches does not include the 'heads/', even though it might be acceptable if it showed other things. >> - Does this have be a compile-time decision? It looks like this is >>something that can and should be made controllable with the >>normal gitweb configuration mechanism. >> > > Maybe. I was just looking at Makefile and saw a bunch of configuration > options there, so I just added another one. Haven't noticed the gitweb > config thing. Sorry. > > So, we should just hardcode the @wanted_refs (or @branch_refs after the > rename) to simply ('heads'), let it be overriden by perl gitweb config > file and get rid of a new substitution from Makefile? Something along that line, but perhaps use @additional_branch_refs to allow users to specify additional hierarchies to be shown, and always include 'heads' to where we currently show 'heads' hierarchy, just like your version of format_refs_views always included 'tags' hierarchy regardless of the setting of @wanted_refs? Thanks. >> >> > gitweb/Makefile| 4 ++- >> > gitweb/gitweb.perl | 94 >> > +++--- >> > 2 files changed, 72 insertions(+), 26 deletions(-) >> > >> > diff --git a/gitweb/Makefile b/gitweb/Makefile >> > index cd194d0..361dce9 100644 >> > --- a/gitweb/Makefile >> > +++ b/gitweb/Makefile >> > @@ -38,6 +38,7 @@ GITWEB_SITE_HTML_HEAD_STRING = >> > GITWEB_SITE_HEADER = >> > GITWEB_SITE_FOOTER = >> > HIGHLIGHT_BIN = highlight >> > +GITWEB_WANTED_REFS = heads >> > >> > # include user config >> > -include ../config.mak.autogen >> > @@ -148,7 +149,8 @@ GITWEB_REPLACE = \ >> >-e >> > 's|++GITWEB_SITE_HTML_HEAD_STRING++|$(GITWEB_SITE_HTML_HEAD_STRING)|g' \ >> >-e 's|++GITWEB_SITE_HEADER++|$(GITWEB_SITE_HEADER)|g' \ >> >-e 's|++GITWEB_SITE_FOOTER++|$(GITWEB_SITE_FOOTER)|g' \ >> > - -e 's|++HIGHLIGHT_BIN++|$(HIGHLIGHT_BIN)|g' >> > + -e 's|++HIGHLIGHT_BIN++|$(HIGHLIGHT_BIN)|g' \ >> > + -e 's|++GITWEB_WANTED_REFS++|$(GITWEB_WANTED_REFS)|g' >> > >> > GITWEB-BUILD-OPTIONS: FORCE >> >@rm -f $@+ >> > diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl >> > index 68c77f6..8bc9e9a 100755 >> > --- a/gitweb/gitweb.perl >> > +++ b/gitweb/gitweb.perl >> > @@ -17,6 +17,7 @@ use Encode; >> > use Fcntl ':mode'; >> > use File::Find qw(); >> > use File::Basename qw(basename); >> > +use List::Util qw(min); >> > use Time::HiRes qw(gettimeofday tv_interval); >> > binmode STDOUT, ':utf8'; >> > >> > @@ -122,6 +123,9 @@ our $logo_label = "git homepage"; >> > # source of projects list >> > our $projects_list = "++GITWEB_LIST++"; >> > >> > +# list of "directories" under "refs/" we want to display as branches >> > +our @wanted_refs = qw{++GITWEB_WANTED_REFS++}; >> > + >> > # the width (in characters) of the projects list "Description" column >> > our $projects_list_description_width = 25; >> > >> > @@ -632,8 +636,19 @@ sub feature_avatar { >> > sub check_head_link { >> >my ($dir) = @_; >> >
Re: [PATCH 2/2] commit-slab: declare functions "static inline"
Thomas Rast writes: > This shuts up compiler warnings about unused functions. Thanks. > While there, also remove the redundant second declaration of > stat_##slabname##realloc. I think the latter was done very much deliberately to allow the using code to say: define_commit_slab(name, type); by ending the macro with something that requires a terminating semicolon. If you just remove it, doesn't it break the compilation by forcing the expanded source to define a function slabname ## _at(...) { ... }; with a trailing and undesired semicolon? > > Signed-off-by: Thomas Rast > --- > commit-slab.h | 17 - > 1 file changed, 8 insertions(+), 9 deletions(-) > > diff --git a/commit-slab.h b/commit-slab.h > index d77aaea..d5c353e 100644 > --- a/commit-slab.h > +++ b/commit-slab.h > @@ -45,8 +45,8 @@ struct slabname { > \ > }; \ > static int stat_ ##slabname## realloc; > \ > \ > -static void init_ ##slabname## _with_stride(struct slabname *s, > \ > - unsigned stride)\ > +static inline void init_ ##slabname## _with_stride(struct slabname *s, > \ > +unsigned stride) \ > {\ > unsigned int elem_size; \ > if (!stride)\ > @@ -58,12 +58,12 @@ struct slabname { > \ > s->slab = NULL; \ > }\ > \ > -static void init_ ##slabname(struct slabname *s) \ > +static inline void init_ ##slabname(struct slabname *s) > \ > {\ > init_ ##slabname## _with_stride(s, 1); \ > }\ > \ > -static void clear_ ##slabname(struct slabname *s)\ > +static inline void clear_ ##slabname(struct slabname *s) \ > {\ > int i; \ > for (i = 0; i < s->slab_count; i++) \ > @@ -73,8 +73,8 @@ struct slabname { > \ > s->slab = NULL; \ > }\ > \ > -static elemtype *slabname## _at(struct slabname *s, \ > - const struct commit *c) \ > +static inline elemtype *slabname## _at(struct slabname *s, \ > +const struct commit *c) \ > {\ > int nth_slab, nth_slot; \ > \ > @@ -94,8 +94,7 @@ struct slabname { > \ > s->slab[nth_slab] = xcalloc(s->slab_size, \ > sizeof(**s->slab) * s->stride); > \ > return &s->slab[nth_slab][nth_slot * s->stride]; > \ > -}\ > - \ > -static int stat_ ##slabname## realloc > +} > + > > #endif /* COMMIT_SLAB_H */ -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] commit-slab: declare functions "static inline"
Junio C Hamano writes: > Thomas Rast writes: > >> This shuts up compiler warnings about unused functions. > > Thanks. > >> While there, also remove the redundant second declaration of >> stat_##slabname##realloc. > > I think the latter was done very much deliberately to allow the > using code to say: > > define_commit_slab(name, type); > > by ending the macro with something that requires a terminating > semicolon. If you just remove it, doesn't it break the compilation > by forcing the expanded source to define a function > > slabname ## _at(...) > { > ... > }; > > with a trailing and undesired semicolon? Oooh. The sudden enlightenment. -- Thomas Rast t...@thomasrast.ch -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2] commit-slab: declare functions "static inline"
This shuts up compiler warnings about unused functions. No such warnings are currently triggered, but if someone were to actually use init_NAME_with_stride() as documented, they would get a warning about init_NAME() being unused. While there, write a comment about why we need two declarations of the same variable. Signed-off-by: Thomas Rast --- Here's a version that has a fat comment instead of the removal. Also, since I was rerolling anyway I put a reason why we need this. In the original motivation I actually created more functions afterwards, which made it more convincing, but the problem already exists. commit-slab.h | 24 ++-- 1 file changed, 18 insertions(+), 6 deletions(-) diff --git a/commit-slab.h b/commit-slab.h index d77aaea..21d54f1 100644 --- a/commit-slab.h +++ b/commit-slab.h @@ -45,8 +45,8 @@ struct slabname { \ }; \ static int stat_ ##slabname## realloc; \ \ -static void init_ ##slabname## _with_stride(struct slabname *s, \ - unsigned stride)\ +static inline void init_ ##slabname## _with_stride(struct slabname *s, \ + unsigned stride) \ { \ unsigned int elem_size; \ if (!stride)\ @@ -58,12 +58,12 @@ struct slabname { \ s->slab = NULL; \ } \ \ -static void init_ ##slabname(struct slabname *s) \ +static inline void init_ ##slabname(struct slabname *s) \ { \ init_ ##slabname## _with_stride(s, 1); \ } \ \ -static void clear_ ##slabname(struct slabname *s) \ +static inline void clear_ ##slabname(struct slabname *s) \ { \ int i; \ for (i = 0; i < s->slab_count; i++) \ @@ -73,8 +73,8 @@ struct slabname { \ s->slab = NULL; \ } \ \ -static elemtype *slabname## _at(struct slabname *s,\ - const struct commit *c) \ +static inline elemtype *slabname## _at(struct slabname *s, \ + const struct commit *c) \ { \ int nth_slab, nth_slot; \ \ @@ -98,4 +98,16 @@ struct slabname { \ \ static int stat_ ##slabname## realloc +/* + * Note that this seemingly redundant second declaration is required + * to allow a terminating semicolon, which makes instantiations look + * like function declarations. I.e., the expansion of + * + *define_commit_slab(indegree, int); + * + * ends in 'static int stat_indegreerealloc;'. This would otherwise + * be a syntax error according (at least) to ISO C. It's hard to + * catch because GCC silently parses it by default. + */ + #endif /* COMMIT_SLAB_H */ -- 1.8.5.rc2.355.g6969a19 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 04/28] update-server-info: do not publish shallow clones
Nguyễn Thái Ngọc Duy writes: > Dumb commit walker does not care about .git/shallow and until someone > steps up to make it happen, let's not publish shallow clones using > dumb protocols. > > Signed-off-by: Nguyễn Thái Ngọc Duy > --- > server-info.c | 9 + > 1 file changed, 9 insertions(+) I doubt that pros-and-cons is in this patch's favor. Without this patch, if a fetch requests commits just on the surface in this shallow repository, the walker would happily get the necessary objects just fine. If the request has to dig deeper and cross the shallow boundary, the walker will get a failure and error out. With this patch, both will error out. So overall, the patch did not make anything safer (e.g. preventing from introducing new corruption on the recipient's end) while breaking a case where it worked just fine, no? Or am I missing something? Not that dumb walkers would matter very much these days, ... > diff --git a/server-info.c b/server-info.c > index 9ec744e..a8df6a5 100644 > --- a/server-info.c > +++ b/server-info.c > @@ -33,6 +33,10 @@ static int update_info_refs(int force) > strcpy(path1 + len, "+"); > > safe_create_leading_directories(path0); > + if (is_repository_shallow()) { > + unlink(path0); > + return error("info/refs not updated for shallow clone"); > + } > info_ref_fp = fopen(path1, "w"); > if (!info_ref_fp) > return error("unable to update %s", path1); > @@ -217,6 +221,11 @@ static int update_info_packs(int force) > strcpy(name, infofile); > strcpy(name + namelen, "+"); > > + if (is_repository_shallow()) { > + unlink(infofile); > + return error("info/packs not updated for shallow clone"); > + } > + > init_pack_info(infofile, force); > > safe_create_leading_directories(name); -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] commit-slab: declare functions "static inline"
Thomas Rast wrote: > This shuts up compiler warnings about unused functions. If that is the only goal, I think it would be cleaner to use #define MAYBE_UNUSED __attribute__((__unused__)) static MAYBE_UNUSED void init_ ... like was done in the vcs-svn/ directory until cba3546 (drop obj_pool, 2010-12-13) et al. I haven't thought carefully about whether encouraging inlining here (or encouraging the reader to think of these functions as inline) is a good or bad change. [...] > @@ -98,4 +98,16 @@ struct slabname { > \ > \ > static int stat_ ##slabname## realloc > > +/* > + * Note that this seemingly redundant second declaration is required > + * to allow a terminating semicolon, which makes instantiations look > + * like function declarations. I.e., the expansion of Micronit: this reads more clearly without the "Note that". That is, the comment can get the reader's attention more easily by going right into what it is about to say without asking for the reader's attention: /* * This seemingly redundant second declaration is required to ... Thanks, Jonathan -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] commit-slab: declare functions "static inline"
Jonathan Nieder writes: > Thomas Rast wrote: > >> This shuts up compiler warnings about unused functions. > > If that is the only goal, I think it would be cleaner to use > > #define MAYBE_UNUSED __attribute__((__unused__)) > > static MAYBE_UNUSED void init_ ... > > like was done in the vcs-svn/ directory until cba3546 (drop obj_pool, > 2010-12-13) et al. > > I haven't thought carefully about whether encouraging inlining here > (or encouraging the reader to think of these functions as inline) is a > good or bad change. Hmm. I actually had this idea after seeing the same trick in khash.h. Is __atribute__((__unused__)) universal? If so, maybe we could apply the same also to khash? If not, I'd rather go with the inline. -- Thomas Rast t...@thomasrast.ch -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Filter log based on paths NOT touched
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 I can't seem to find a way to invert the meaning of a pathspec given to git log in order to find commits touching anything BUT a given path. Does such a thing exist? -BEGIN PGP SIGNATURE- Version: GnuPG v2.0.17 (MingW32) Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/ iQEcBAEBAgAGBQJSk7D5AAoJEJrBOlT6nu75myIIAMoAgihPAhDrCBpRKUHF/X8S B8vjwIg7zALajU+vrz7B/UyxKFHC54sYn0MaAA5htBXKCtd6L0tHrNa1gYbd9qT+ xgTuF7+Unwv90yFBEsoZgEwlSyaLBAVHknMiE4ecxaJrlhBqESbePNrORCwCAuPq ANrYunEETN2KNgYBkNszdEp7Ga9RcP7LWisL/pNV2k+ac7YfqGp1jsN00jLMYqvH c+8Kl154N3xgvk+pGvkKGbO3MavkmEK47lLL929g9iXeP3NkMsrxyEjhnABD9tS3 SxzYZt9G+lpH2Tv8l1/NqMafdNy5P7CEs00C4JZn1EzEcIBqMeYgUhce+WU75qc= =p2gM -END PGP SIGNATURE- -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] commit-slab: declare functions "static inline"
Thomas Rast writes: > Here's a version that has a fat comment instead of the removal. > > Also, since I was rerolling anyway I put a reason why we need this. > In the original motivation I actually created more functions > afterwards, which made it more convincing, but the problem already > exists. Thanks. I considered the bottom one the real declaration (with the top one a forward declaration we need to make the result compile), by the way, so there may be no redundancy anywhere ;-) > commit-slab.h | 24 ++-- > 1 file changed, 18 insertions(+), 6 deletions(-) > > diff --git a/commit-slab.h b/commit-slab.h > index d77aaea..21d54f1 100644 > --- a/commit-slab.h > +++ b/commit-slab.h > @@ -45,8 +45,8 @@ struct slabname { > \ > }; \ > static int stat_ ##slabname## realloc; > \ > \ > -static void init_ ##slabname## _with_stride(struct slabname *s, > \ > - unsigned stride)\ > +static inline void init_ ##slabname## _with_stride(struct slabname *s, > \ > +unsigned stride) \ > {\ > unsigned int elem_size; \ > if (!stride)\ > @@ -58,12 +58,12 @@ struct slabname { > \ > s->slab = NULL; \ > }\ > \ > -static void init_ ##slabname(struct slabname *s) \ > +static inline void init_ ##slabname(struct slabname *s) > \ > {\ > init_ ##slabname## _with_stride(s, 1); \ > }\ > \ > -static void clear_ ##slabname(struct slabname *s)\ > +static inline void clear_ ##slabname(struct slabname *s) \ > {\ > int i; \ > for (i = 0; i < s->slab_count; i++) \ > @@ -73,8 +73,8 @@ struct slabname { > \ > s->slab = NULL; \ > }\ > \ > -static elemtype *slabname## _at(struct slabname *s, \ > - const struct commit *c) \ > +static inline elemtype *slabname## _at(struct slabname *s, \ > +const struct commit *c) \ > {\ > int nth_slab, nth_slot; \ > \ > @@ -98,4 +98,16 @@ struct slabname { > \ > \ > static int stat_ ##slabname## realloc > > +/* > + * Note that this seemingly redundant second declaration is required > + * to allow a terminating semicolon, which makes instantiations look > + * like function declarations. I.e., the expansion of > + * > + *define_commit_slab(indegree, int); > + * > + * ends in 'static int stat_indegreerealloc;'. This would otherwise > + * be a syntax error according (at least) to ISO C. It's hard to > + * catch because GCC silently parses it by default. > + */ > + > #endif /* COMMIT_SLAB_H */ -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] commit-slab: document clear_$slabname()
Thomas Rast wrote: > The clear_$slabname() function was only documented by source code so > far. Write something about it. Good idea. [...] > --- a/commit-slab.h > +++ b/commit-slab.h > @@ -24,6 +24,10 @@ > * to each commit. 'stride' specifies how big each array is. The slab > * that id initialied by the variant without "_with_stride" associates > * each commit with an array of one integer. > + * > + * - void clear_indegree(struct indegree *); > + * > + * Free the slab's data structures. Tense shift (previous descriptions were in the present tense, while this one is in the imperative). More importantly, this doesn't answer the questions I'd have if I were in a hurry, which are what exactly is being freed (has the slab taken ownership of any memory from the user, e.g. when elemtype is a pointer?) and whether the slab needs to be init_ ed again. Maybe something like the following would work? - void clear_indegree(struct indegree *); Empties the slab. The slab can be reused with the same stride without calling init_indegree again or can be reconfigured to a different stride by calling init_indegree_with_stride. Call this function before the slab falls out of scope to avoid leaking memory. Thanks, Jonathan -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] commit-slab: declare functions "static inline"
Jonathan Nieder writes: > Thomas Rast wrote: > >> This shuts up compiler warnings about unused functions. > > If that is the only goal, I think it would be cleaner to use > > #define MAYBE_UNUSED __attribute__((__unused__)) > > static MAYBE_UNUSED void init_ ... > > like was done in the vcs-svn/ directory until cba3546 (drop obj_pool, > 2010-12-13) et al. > > I haven't thought carefully about whether encouraging inlining here > (or encouraging the reader to think of these functions as inline) is a > good or bad change. > > [...] >> @@ -98,4 +98,16 @@ struct slabname { >> \ >> \ >> static int stat_ ##slabname## realloc >> >> +/* >> + * Note that this seemingly redundant second declaration is required >> + * to allow a terminating semicolon, which makes instantiations look >> + * like function declarations. I.e., the expansion of > > Micronit: this reads more clearly without the "Note that". That is, > the comment can get the reader's attention more easily by going right > into what it is about to say without asking for the reader's > attention: > > /* >* This seemingly redundant second declaration is required to ... Hmm, both of these are good points. Thanks. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Filter log based on paths NOT touched
Phillip Susi writes: > I can't seem to find a way to invert the meaning of a pathspec given > to git log in order to find commits touching anything BUT a given > path. Does such a thing exist? Not yet (look for "negative pathspec"). -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] commit-slab: declare functions "static inline"
Thomas Rast wrote: > Jonathan Nieder writes: >> Thomas Rast wrote: >>> This shuts up compiler warnings about unused functions. >> >> If that is the only goal, I think it would be cleaner to use >> >> #define MAYBE_UNUSED __attribute__((__unused__)) >> >> static MAYBE_UNUSED void init_ ... >> >> like was done in the vcs-svn/ directory until cba3546 (drop obj_pool, >> 2010-12-13) et al. >> >> I haven't thought carefully about whether encouraging inlining here >> (or encouraging the reader to think of these functions as inline) is a >> good or bad change. > > Hmm. > > I actually had this idea after seeing the same trick in khash.h. Is > __atribute__((__unused__)) universal? If so, maybe we could apply the > same also to khash? If not, I'd rather go with the inline. The khash functions are very small, so it very well may make sense for them to be inline. git-compat-util.h (or compat/msvc.h) defines __attribute__(x) to an empty sequence of tokens except on HP C and gcc. Attribute unused has existed at least since GCC 2.95. Unfortunately HP C doesn't support attribute __unused__. :( http://h21007.www2.hp.com/portal/download/files/unprot/aCxx/Online_Help/pragmas.htm#Attributes On the bright side, it would be easy to work around using a conditional definition of MAYBE_UNUSED for the sake of HP C. From August 2010 until March 2011 nobody noticed. Jonathan -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] commit-slab: document clear_$slabname()
Thomas Rast writes: > The clear_$slabname() function was only documented by source code so > far. Write something about it. > > Signed-off-by: Thomas Rast > --- > commit-slab.h | 4 > 1 file changed, 4 insertions(+) > > diff --git a/commit-slab.h b/commit-slab.h > index d4c8286..d77aaea 100644 > --- a/commit-slab.h > +++ b/commit-slab.h > @@ -24,6 +24,10 @@ > * to each commit. 'stride' specifies how big each array is. The slab > * that id initialied by the variant without "_with_stride" associates Is that "id" a typo for "is"? > * each commit with an array of one integer. > + * > + * - void clear_indegree(struct indegree *); > + * > + * Free the slab's data structures. > */ > > /* allocate ~512kB at once, allowing for malloc overhead */ -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 0/9] War on #! lines in shell libraries
Hi, This is an old series that I just wanted to flush out of my working directory. It does two things: On one hand, it replaces the line #!/bin/sh at the start of shell libraries with a simple comment # Shell library for to avoid confusing readers into thinking the shell library is going to be run directly with /bin/sh. And on the other hand, it sets the executable bit on actual scripts that are run directly with /bin/sh. (Likewise for some perl scripts, too.) The heart of the series has been well tested in the context of Debian git for a couple of years. Thanks to list denizens for the improvements last time I brought it up[1]. Hopefully this version is more sensible. Anyway, hopefully it can provide some amusement. Thanks, Jonathan Nieder (9): mark Windows build scripts executable mark perl test scripts executable mark contributed hooks executable contrib: remove git-p4import gitk: chmod +x po2msg git-gui: chmod +x po2msg, windows/git-gui.sh test: make FILEMODE a lazy prereq test: replace shebangs with descriptions in shell libraries remove #!interpreter line from shell libraries [1] http://thread.gmane.org/gmane.comp.version-control.git/192582 compat/vcbuild/scripts/clink.pl| 0 compat/vcbuild/scripts/lib.pl | 0 contrib/buildsystems/engine.pl | 0 contrib/buildsystems/generate | 0 contrib/buildsystems/parse.pl | 0 contrib/completion/git-completion.bash | 2 - contrib/completion/git-completion.tcsh | 2 - contrib/hooks/post-receive-email | 1 - contrib/hooks/pre-auto-gc-battery | 1 - contrib/hooks/setgitperms.perl | 0 contrib/hooks/update-paranoid | 0 contrib/p4import/README| 1 - contrib/p4import/git-p4import.py | 365 - contrib/p4import/git-p4import.txt | 167 --- git-gui/po/po2msg.sh | 0 git-gui/windows/git-gui.sh | 0 git-mergetool--lib.sh | 3 +- git-parse-remote.sh| 4 +- git-rebase--am.sh | 3 +- git-rebase--interactive.sh | 9 +- git-rebase--merge.sh | 4 +- git-sh-i18n.sh | 5 +- git-sh-setup.sh| 9 +- gitk-git/po/po2msg.sh | 0 t/Git-SVN/00compile.t | 0 t/Git-SVN/Utils/add_path_to_url.t | 0 t/Git-SVN/Utils/can_compress.t | 0 t/Git-SVN/Utils/canonicalize_url.t | 0 t/Git-SVN/Utils/collapse_dotdot.t | 0 t/Git-SVN/Utils/fatal.t| 0 t/Git-SVN/Utils/join_paths.t | 0 t/gitweb-lib.sh| 3 +- t/lib-bash.sh | 7 +- t/lib-cvs.sh | 2 +- t/lib-diff-alternative.sh | 3 +- t/lib-gettext.sh | 3 +- t/lib-git-daemon.sh| 18 +- t/lib-httpd.sh | 29 ++- t/lib-pack.sh | 2 - t/lib-pager.sh | 2 +- t/lib-prereq-FILEMODE.sh | 11 - t/lib-read-tree.sh | 2 - t/lib-rebase.sh| 2 +- t/lib-terminal.sh | 2 +- t/perf/perf-lib.sh | 4 +- t/t0202/test.pl| 0 t/t3701-add-interactive.sh | 1 - t/t4102-apply-rename.sh| 1 - t/t4120-apply-popt.sh | 1 - t/t4129-apply-samemode.sh | 1 - t/t6031-merge-recursive.sh | 1 - t/t9150/make-svk-dump | 0 t/t9151/make-svnmerge-dump | 0 t/t9200-git-cvsexportcommit.sh | 1 - t/test-lib-functions.sh| 3 +- t/test-lib.sh | 12 +- 56 files changed, 87 insertions(+), 600 deletions(-) mode change 100644 => 100755 compat/vcbuild/scripts/clink.pl mode change 100644 => 100755 compat/vcbuild/scripts/lib.pl mode change 100644 => 100755 contrib/buildsystems/engine.pl mode change 100644 => 100755 contrib/buildsystems/generate mode change 100644 => 100755 contrib/buildsystems/parse.pl mode change 100644 => 100755 contrib/hooks/pre-auto-gc-battery mode change 100644 => 100755 contrib/hooks/setgitperms.perl mode change 100644 => 100755 contrib/hooks/update-paranoid delete mode 100644 contrib/p4import/README delete mode 100644 contrib/p4import/git-p4import.py delete mode 100644 contrib/p4import/git-p4import.txt mode change 100644 => 100755 git-gui/po/po2msg.sh mode change 100644 => 100755 git-gui/windows/git-gui.sh mode change 100644 => 100755 gitk-git/po/po2msg.sh mode change 100644 => 100755 t/Git-SVN/00compile.t mode change 100644 => 100755 t/Git-SVN/Utils/add_path_to_url.t mode change 100644 => 100755 t/Git-SVN/Utils/can_compress.t mode change 100644 => 100755 t/Git-SVN
[PATCH 2/9] mark perl test scripts executable
These scripts are not run directly as part of a normal build, so no one noticed that they did not have the +x bit. Mark them executable to make it more obvious that they can be run directly (when debugging, for example). Signed-off-by: Jonathan Nieder --- t/Git-SVN/00compile.t | 0 t/Git-SVN/Utils/add_path_to_url.t | 0 t/Git-SVN/Utils/can_compress.t | 0 t/Git-SVN/Utils/canonicalize_url.t | 0 t/Git-SVN/Utils/collapse_dotdot.t | 0 t/Git-SVN/Utils/fatal.t| 0 t/Git-SVN/Utils/join_paths.t | 0 t/t0202/test.pl| 0 t/t9150/make-svk-dump | 0 t/t9151/make-svnmerge-dump | 0 10 files changed, 0 insertions(+), 0 deletions(-) mode change 100644 => 100755 t/Git-SVN/00compile.t mode change 100644 => 100755 t/Git-SVN/Utils/add_path_to_url.t mode change 100644 => 100755 t/Git-SVN/Utils/can_compress.t mode change 100644 => 100755 t/Git-SVN/Utils/canonicalize_url.t mode change 100644 => 100755 t/Git-SVN/Utils/collapse_dotdot.t mode change 100644 => 100755 t/Git-SVN/Utils/fatal.t mode change 100644 => 100755 t/Git-SVN/Utils/join_paths.t mode change 100644 => 100755 t/t0202/test.pl mode change 100644 => 100755 t/t9150/make-svk-dump mode change 100644 => 100755 t/t9151/make-svnmerge-dump diff --git a/t/Git-SVN/00compile.t b/t/Git-SVN/00compile.t old mode 100644 new mode 100755 diff --git a/t/Git-SVN/Utils/add_path_to_url.t b/t/Git-SVN/Utils/add_path_to_url.t old mode 100644 new mode 100755 diff --git a/t/Git-SVN/Utils/can_compress.t b/t/Git-SVN/Utils/can_compress.t old mode 100644 new mode 100755 diff --git a/t/Git-SVN/Utils/canonicalize_url.t b/t/Git-SVN/Utils/canonicalize_url.t old mode 100644 new mode 100755 diff --git a/t/Git-SVN/Utils/collapse_dotdot.t b/t/Git-SVN/Utils/collapse_dotdot.t old mode 100644 new mode 100755 diff --git a/t/Git-SVN/Utils/fatal.t b/t/Git-SVN/Utils/fatal.t old mode 100644 new mode 100755 diff --git a/t/Git-SVN/Utils/join_paths.t b/t/Git-SVN/Utils/join_paths.t old mode 100644 new mode 100755 diff --git a/t/t0202/test.pl b/t/t0202/test.pl old mode 100644 new mode 100755 diff --git a/t/t9150/make-svk-dump b/t/t9150/make-svk-dump old mode 100644 new mode 100755 diff --git a/t/t9151/make-svnmerge-dump b/t/t9151/make-svnmerge-dump old mode 100644 new mode 100755 -- 1.8.4.1 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Git issues with submodules
Heiko Voigt writes: > What I think needs fixing here first is that the ignore setting should not > apply to any diffs between HEAD and index. IMO, it should only apply > to the diff between worktree and index. Hmph. How about "git diff $commit", the diff between the worktree and a named commit (which may or may not be HEAD)? > When we have that the user does not see the submodule changed when > normally working. But after doing git add . the change to the submodule > should be shown in status and diff regardless of the configuration. Yes, that sounds sensible. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 3/9] mark contributed hooks executable
The docs in contrib/hooks/pre-auto-gc-battery suggest: For example, if the hook is stored in /usr/share/git-core/contrib/hooks/pre-auto-gc-battery: chmod a+x pre-auto-gc-battery cd /path/to/your/repository.git ln -sf /usr/share/git-core/contrib/hooks/pre-auto-gc-battery \ hooks/pre-auto-gc Unfortunately on multi-user systems most users do not have write access to /usr. Better to mark the sample hooks executable in the first place so users do not have to tweak their permissions to use them by symlinking into .git/hooks/. Reported-by: Olivier Berger Signed-off-by: Jonathan Nieder --- contrib/hooks/post-receive-email | 1 - contrib/hooks/pre-auto-gc-battery | 1 - contrib/hooks/setgitperms.perl| 0 contrib/hooks/update-paranoid | 0 4 files changed, 2 deletions(-) mode change 100644 => 100755 contrib/hooks/pre-auto-gc-battery mode change 100644 => 100755 contrib/hooks/setgitperms.perl mode change 100644 => 100755 contrib/hooks/update-paranoid diff --git a/contrib/hooks/post-receive-email b/contrib/hooks/post-receive-email index 8ee410f..8747b84 100755 --- a/contrib/hooks/post-receive-email +++ b/contrib/hooks/post-receive-email @@ -22,7 +22,6 @@ # For example, on debian the hook is stored in # /usr/share/git-core/contrib/hooks/post-receive-email: # -# chmod a+x post-receive-email # cd /path/to/your/repository.git # ln -sf /usr/share/git-core/contrib/hooks/post-receive-email hooks/post-receive # diff --git a/contrib/hooks/pre-auto-gc-battery b/contrib/hooks/pre-auto-gc-battery old mode 100644 new mode 100755 index 1f914c9..9d0c2d1 --- a/contrib/hooks/pre-auto-gc-battery +++ b/contrib/hooks/pre-auto-gc-battery @@ -13,7 +13,6 @@ # For example, if the hook is stored in # /usr/share/git-core/contrib/hooks/pre-auto-gc-battery: # -# chmod a+x pre-auto-gc-battery # cd /path/to/your/repository.git # ln -sf /usr/share/git-core/contrib/hooks/pre-auto-gc-battery \ # hooks/pre-auto-gc diff --git a/contrib/hooks/setgitperms.perl b/contrib/hooks/setgitperms.perl old mode 100644 new mode 100755 diff --git a/contrib/hooks/update-paranoid b/contrib/hooks/update-paranoid old mode 100644 new mode 100755 -- 1.8.4.1 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 4/9] contrib: remove git-p4import
The git p4import documentation has suggested git p4 as a better alternative for more than 6 years. (According to the mailing list discussion when it was moved to contrib/, git-p4import has serious bugs --- e.g., its incremental mode just doesn't work.) Since then, git p4 has been actively developed and was promoted to a standard git command alongside git svn. Searches on google.com/trends and stackoverflow suggest that no one is looking for git-p4import any more. Remove it. Noticed while considering marking the contrib/p4import/git-p4import.py script executable as part of a wider sweep. Signed-off-by: Jonathan Nieder --- Following up on http://thread.gmane.org/gmane.comp.version-control.git/52580 contrib/p4import/README | 1 - contrib/p4import/git-p4import.py | 365 -- contrib/p4import/git-p4import.txt | 167 - 3 files changed, 533 deletions(-) delete mode 100644 contrib/p4import/README delete mode 100644 contrib/p4import/git-p4import.py delete mode 100644 contrib/p4import/git-p4import.txt diff --git a/contrib/p4import/README b/contrib/p4import/README deleted file mode 100644 index b9892b6..000 --- a/contrib/p4import/README +++ /dev/null @@ -1 +0,0 @@ -Please see contrib/fast-import/git-p4 for a better Perforce importer. diff --git a/contrib/p4import/git-p4import.py b/contrib/p4import/git-p4import.py deleted file mode 100644 index 593d6a0..000 --- a/contrib/p4import/git-p4import.py +++ /dev/null @@ -1,365 +0,0 @@ -#!/usr/bin/env python -# -# This tool is copyright (c) 2006, Sean Estabrooks. -# It is released under the Gnu Public License, version 2. -# -# Import Perforce branches into Git repositories. -# Checking out the files is done by calling the standard p4 -# client which you must have properly configured yourself -# - -import marshal -import os -import sys -import time -import getopt - -if sys.hexversion < 0x0202: - # The behavior of the marshal module changed significantly in 2.2 - sys.stderr.write("git-p4import.py: requires Python 2.2 or later.\n") - sys.exit(1) - -from signal import signal, \ - SIGPIPE, SIGINT, SIG_DFL, \ - default_int_handler - -signal(SIGPIPE, SIG_DFL) -s = signal(SIGINT, SIG_DFL) -if s != default_int_handler: - signal(SIGINT, s) - -def die(msg, *args): -for a in args: -msg = "%s %s" % (msg, a) -print "git-p4import fatal error:", msg -sys.exit(1) - -def usage(): -print "USAGE: git-p4import [-q|-v] [--authors=] [-t ] [//p4repo/path ]" -sys.exit(1) - -verbosity = 1 -logfile = "/dev/null" -ignore_warnings = False -stitch = 0 -tagall = True - -def report(level, msg, *args): -global verbosity -global logfile -for a in args: -msg = "%s %s" % (msg, a) -fd = open(logfile, "a") -fd.writelines(msg) -fd.close() -if level <= verbosity: -print msg - -class p4_command: -def __init__(self, _repopath): -try: -global logfile -self.userlist = {} -if _repopath[-1] == '/': -self.repopath = _repopath[:-1] -else: -self.repopath = _repopath -if self.repopath[-4:] != "/...": -self.repopath= "%s/..." % self.repopath -f=os.popen('p4 -V 2>>%s'%logfile, 'rb') -a = f.readlines() -if f.close(): -raise -except: -die("Could not find the \"p4\" command") - -def p4(self, cmd, *args): -global logfile -cmd = "%s %s" % (cmd, ' '.join(args)) -report(2, "P4:", cmd) -f=os.popen('p4 -G %s 2>>%s' % (cmd,logfile), 'rb') -list = [] -while 1: - try: -list.append(marshal.load(f)) - except EOFError: -break -self.ret = f.close() -return list - -def sync(self, id, force=False, trick=False, test=False): -if force: -ret = self.p4("sync -f %s@%s"%(self.repopath, id))[0] -elif trick: -ret = self.p4("sync -k %s@%s"%(self.repopath, id))[0] -elif test: -ret = self.p4("sync -n %s@%s"%(self.repopath, id))[0] -else: -ret = self.p4("sync%s@%s"%(self.repopath, id))[0] -if ret['code'] == "error": - data = ret['data'].upper() - if data.find('VIEW') > 0: - die("Perforce reports %s is not in client view"% self.repopath) - elif data.find('UP-TO-DATE') < 0: - die("Could not sync files from perforce", self.repopath) - -def changes(self, since=0): -try: -list = [] -for rec in self.p4("changes %s@%s,#head" % (self.repopath, since+1)): -list.append(rec['change']) -list.reverse() -return list -except: -return [] - -def authors(self, filename): -f=open(filename) -for l in f.readlines(): -
[PATCH 5/9 gitk] gitk: chmod +x po2msg
The Makefile only runs it using tclsh, but because the fallback po2msg script has the usual tcl preamble starting with #!/bin/sh it can also be run directly. Signed-off-by: Jonathan Nieder --- po/po2msg.sh | 0 1 file changed, 0 insertions(+), 0 deletions(-) mode change 100644 => 100755 po/po2msg.sh diff --git a/po/po2msg.sh b/po/po2msg.sh old mode 100644 new mode 100755 -- 1.8.4.1 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 6/9 git-gui] git-gui: chmod +x po2msg, windows/git-gui.sh
The Makefile only runs po/po2msg.sh using tclsh, but because the script has the usual tcl preamble starting with #!/bin/sh it can also be run directly. The Windows git-gui wrapper is usable in-place for the same reason. Signed-off-by: Jonathan Nieder --- po/po2msg.sh | 0 windows/git-gui.sh | 0 2 files changed, 0 insertions(+), 0 deletions(-) mode change 100644 => 100755 po/po2msg.sh mode change 100644 => 100755 windows/git-gui.sh diff --git a/po/po2msg.sh b/po/po2msg.sh old mode 100644 new mode 100755 diff --git a/windows/git-gui.sh b/windows/git-gui.sh old mode 100644 new mode 100755 -- 1.8.4.1 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/9] mark Windows build scripts executable
On Windows the convention is to rely on filename extensions to decide whether a file is executable so Windows users are probably not relying on the executable bit of these scripts, but on other platforms it can be useful documentation. Signed-off-by: Jonathan Nieder --- compat/vcbuild/scripts/clink.pl | 0 compat/vcbuild/scripts/lib.pl | 0 contrib/buildsystems/engine.pl | 0 contrib/buildsystems/generate | 0 contrib/buildsystems/parse.pl | 0 5 files changed, 0 insertions(+), 0 deletions(-) mode change 100644 => 100755 compat/vcbuild/scripts/clink.pl mode change 100644 => 100755 compat/vcbuild/scripts/lib.pl mode change 100644 => 100755 contrib/buildsystems/engine.pl mode change 100644 => 100755 contrib/buildsystems/generate mode change 100644 => 100755 contrib/buildsystems/parse.pl diff --git a/compat/vcbuild/scripts/clink.pl b/compat/vcbuild/scripts/clink.pl old mode 100644 new mode 100755 diff --git a/compat/vcbuild/scripts/lib.pl b/compat/vcbuild/scripts/lib.pl old mode 100644 new mode 100755 diff --git a/contrib/buildsystems/engine.pl b/contrib/buildsystems/engine.pl old mode 100644 new mode 100755 diff --git a/contrib/buildsystems/generate b/contrib/buildsystems/generate old mode 100644 new mode 100755 diff --git a/contrib/buildsystems/parse.pl b/contrib/buildsystems/parse.pl old mode 100644 new mode 100755 -- 1.8.4.1 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Git issues with submodules
Jens Lehmann writes: > Looking good to me. Please add tests for "diff.ignoreSubmodules" > and "submodule..ignore", the latter both in .gitmodules and > .git/config. While doing some testing for this thread I found an > inconsistency in git show which currently honors the submodule > specific option only from .git/config and ignores it in the > .gitmodules file ... Sorry, but isn't that what should happen? .git/config is the ultimate source of the truth, and .gitmodules is a hint to prime that when the user does "git submodule init", no? > I'd suggest to also add the --ignore-submodules option in another > patch on top, because the user should be able to override the > configuration either way. And what about having the '-f' option > imply '--ignore-submodules=none'? Yeah, this sudden change of semantics, which I think is going in the right direction in the longer run, does look like it may be robbing from those from the "want specific revision, but not want to see the cruft in the top-level" camp to pay those in the "floating" school. At least, with "add -f", it allows people to add such ignored ones, just like you can "git add -f cruft" when cruft is not tracked and marked as ignored in the .gitignore mechansim. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 7/9] test: make FILEMODE a lazy prereq
This way, test authors don't need to remember to source lib-prereq-FILEMODE.sh before using the FILEMODE prereq to guard tests that rely on the executable bit being honored when checking out files. Signed-off-by: Jonathan Nieder --- t/lib-prereq-FILEMODE.sh | 11 --- t/t3701-add-interactive.sh | 1 - t/t4102-apply-rename.sh| 1 - t/t4120-apply-popt.sh | 1 - t/t4129-apply-samemode.sh | 1 - t/t6031-merge-recursive.sh | 1 - t/t9200-git-cvsexportcommit.sh | 1 - t/test-lib.sh | 4 8 files changed, 4 insertions(+), 17 deletions(-) delete mode 100644 t/lib-prereq-FILEMODE.sh diff --git a/t/lib-prereq-FILEMODE.sh b/t/lib-prereq-FILEMODE.sh deleted file mode 100644 index bce5a4c..000 --- a/t/lib-prereq-FILEMODE.sh +++ /dev/null @@ -1,11 +0,0 @@ -#!/bin/sh -# -# Copyright (c) 2010 Ævar Arnfjörð Bjarmason -# - -if test "$(git config --bool core.filemode)" = false -then - say 'filemode disabled on the filesystem' -else - test_set_prereq FILEMODE -fi diff --git a/t/t3701-add-interactive.sh b/t/t3701-add-interactive.sh index 9dc91d0..24ddd8a 100755 --- a/t/t3701-add-interactive.sh +++ b/t/t3701-add-interactive.sh @@ -2,7 +2,6 @@ test_description='add -i basic tests' . ./test-lib.sh -. "$TEST_DIRECTORY"/lib-prereq-FILEMODE.sh if ! test_have_prereq PERL then diff --git a/t/t4102-apply-rename.sh b/t/t4102-apply-rename.sh index e3ea3d5..49e2d6c 100755 --- a/t/t4102-apply-rename.sh +++ b/t/t4102-apply-rename.sh @@ -7,7 +7,6 @@ test_description='git apply handling copy/rename patch. ' . ./test-lib.sh -. "$TEST_DIRECTORY"/lib-prereq-FILEMODE.sh # setup diff --git a/t/t4120-apply-popt.sh b/t/t4120-apply-popt.sh index c5fecdf..497b628 100755 --- a/t/t4120-apply-popt.sh +++ b/t/t4120-apply-popt.sh @@ -6,7 +6,6 @@ test_description='git apply -p handling.' . ./test-lib.sh -. "$TEST_DIRECTORY"/lib-prereq-FILEMODE.sh test_expect_success setup ' mkdir sub && diff --git a/t/t4129-apply-samemode.sh b/t/t4129-apply-samemode.sh index 0d36ebd..c268298 100755 --- a/t/t4129-apply-samemode.sh +++ b/t/t4129-apply-samemode.sh @@ -3,7 +3,6 @@ test_description='applying patch with mode bits' . ./test-lib.sh -. "$TEST_DIRECTORY"/lib-prereq-FILEMODE.sh test_expect_success setup ' echo original >file && diff --git a/t/t6031-merge-recursive.sh b/t/t6031-merge-recursive.sh index 1cd649e..a953f1b 100755 --- a/t/t6031-merge-recursive.sh +++ b/t/t6031-merge-recursive.sh @@ -2,7 +2,6 @@ test_description='merge-recursive: handle file mode' . ./test-lib.sh -. "$TEST_DIRECTORY"/lib-prereq-FILEMODE.sh test_expect_success 'mode change in one branch: keep changed version' ' : >file1 && diff --git a/t/t9200-git-cvsexportcommit.sh b/t/t9200-git-cvsexportcommit.sh index 3fb3368..812c9cd 100755 --- a/t/t9200-git-cvsexportcommit.sh +++ b/t/t9200-git-cvsexportcommit.sh @@ -5,7 +5,6 @@ test_description='Test export of commits to CVS' . ./test-lib.sh -. "$TEST_DIRECTORY"/lib-prereq-FILEMODE.sh if ! test_have_prereq PERL; then skip_all='skipping git cvsexportcommit tests, perl not available' diff --git a/t/test-lib.sh b/t/test-lib.sh index b25249e..5968157 100644 --- a/t/test-lib.sh +++ b/t/test-lib.sh @@ -830,6 +830,10 @@ test_lazy_prereq SYMLINKS ' ln -s x y && test -h y ' +test_lazy_prereq FILEMODE ' + test "$(git config --bool core.filemode)" = true +' + test_lazy_prereq CASE_INSENSITIVE_FS ' echo good >CamelCase && echo bad >camelcase && -- 1.8.4.1 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 8/9] test: replace shebangs with descriptions in shell libraries
A #! line in these files is misleading, since these scriptlets are meant to be sourced with '.' (using whatever shell sources them) instead of run directly using the interpreter named on the #! line. Removing the #! line shouldn't hurt syntax highlighting since these files have filenames ending with '.sh'. For documentation, add a brief description of how the files are meant to be used in place of the shebang line. Signed-off-by: Jonathan Nieder --- t/gitweb-lib.sh | 3 ++- t/lib-bash.sh | 7 +++ t/lib-cvs.sh | 2 +- t/lib-diff-alternative.sh | 3 ++- t/lib-gettext.sh | 3 ++- t/lib-git-daemon.sh | 18 +- t/lib-httpd.sh| 29 - t/lib-pack.sh | 2 -- t/lib-pager.sh| 2 +- t/lib-read-tree.sh| 2 -- t/lib-rebase.sh | 2 +- t/lib-terminal.sh | 2 +- t/perf/perf-lib.sh| 4 +++- t/test-lib-functions.sh | 3 ++- t/test-lib.sh | 2 +- 15 files changed, 64 insertions(+), 20 deletions(-) diff --git a/t/gitweb-lib.sh b/t/gitweb-lib.sh index 8cf909a..d5dab5a 100644 --- a/t/gitweb-lib.sh +++ b/t/gitweb-lib.sh @@ -1,4 +1,5 @@ -#!/bin/sh +# Initialization and helpers for Gitweb tests, which source this +# shell library instead of test-lib.sh. # # Copyright (c) 2007 Jakub Narebski # diff --git a/t/lib-bash.sh b/t/lib-bash.sh index 11397f7..10b76df 100644 --- a/t/lib-bash.sh +++ b/t/lib-bash.sh @@ -1,7 +1,6 @@ -#!/bin/sh -# -# Ensures that tests are run under Bash; primarily intended for running tests -# of the completion script. +# Shell library sourced instead of ./test-lib.sh by tests that need +# to run under Bash; primary intended for tests of the completion +# script. if test -n "$BASH" && test -z "$POSIXLY_CORRECT"; then # we are in full-on bash mode diff --git a/t/lib-cvs.sh b/t/lib-cvs.sh index 44263ad..5076718 100644 --- a/t/lib-cvs.sh +++ b/t/lib-cvs.sh @@ -1,4 +1,4 @@ -#!/bin/sh +# Shell library sourced instead of ./test-lib.sh by cvsimport tests. . ./test-lib.sh diff --git a/t/lib-diff-alternative.sh b/t/lib-diff-alternative.sh index 75ffd91..8b4dbf2 100644 --- a/t/lib-diff-alternative.sh +++ b/t/lib-diff-alternative.sh @@ -1,4 +1,5 @@ -#!/bin/sh +# Helpers shared by the test scripts for diff algorithms (patience, +# histogram, etc). test_diff_frobnitz() { cat >file1 <<\EOF diff --git a/t/lib-gettext.sh b/t/lib-gettext.sh index ae8883a..eec757f 100644 --- a/t/lib-gettext.sh +++ b/t/lib-gettext.sh @@ -1,4 +1,5 @@ -#!/bin/sh +# Initialization and Icelandic locale for basic git i18n tests, +# which source this scriptlet instead of ./test-lib.sh. # # Copyright (c) 2010 Ævar Arnfjörð Bjarmason # diff --git a/t/lib-git-daemon.sh b/t/lib-git-daemon.sh index 87f0ad8..394b06b 100644 --- a/t/lib-git-daemon.sh +++ b/t/lib-git-daemon.sh @@ -1,4 +1,20 @@ -#!/bin/sh +# Shell library to run git-daemon in tests. Ends the test early if +# GIT_TEST_GIT_DAEMON is not set. +# +# Usage: +# +# . ./test-lib.sh +# . "$TEST_DIRECTORY"/lib-git-daemon.sh +# start_git_daemon +# +# test_expect_success '...' ' +# ... +# ' +# +# test_expect_success ... +# +# stop_git_daemon +# test_done if test -z "$GIT_TEST_GIT_DAEMON" then diff --git a/t/lib-httpd.sh b/t/lib-httpd.sh index ad8f1ef..c470784 100644 --- a/t/lib-httpd.sh +++ b/t/lib-httpd.sh @@ -1,4 +1,31 @@ -#!/bin/sh +# Shell library to run an HTTP server for use in tests. +# Ends the test early if httpd tests should not be run, +# for example because the user has not enabled them. +# +# Usage: +# +# . ./test-lib.sh +# . "$TEST_DIRECTORY"/lib-httpd.sh +# start_httpd +# +# test_expect_success '...' ' +# ... +# ' +# +# test_expect_success ... +# +# stop_httpd +# test_done +# +# Can be configured using the following variables. +# +#GIT_TEST_HTTPD enable HTTPD tests +#LIB_HTTPD_PATH web server path +#LIB_HTTPD_MODULE_PATH web server modules path +#LIB_HTTPD_PORT listening port +#LIB_HTTPD_DAV enable DAV +#LIB_HTTPD_SVN enable SVN +#LIB_HTTPD_SSL enable SSL # # Copyright (c) 2008 Clemens Buchacher # diff --git a/t/lib-pack.sh b/t/lib-pack.sh index b96e125..7509846 100644 --- a/t/lib-pack.sh +++ b/t/lib-pack.sh @@ -1,5 +1,3 @@ -#!/bin/sh -# # Support routines for hand-crafting weird or malicious packs. # # You can make a complete pack like: diff --git a/t/lib-pager.sh b/t/lib-pager.sh index ba03eab..3aa7a3f 100644 --- a/t/lib-pager.sh +++ b/t/lib-pager.sh @@ -1,4 +1,4 @@ -#!/bin/sh +# Helpers for tests of git's choice of pager. test_expect_success 'determine default pager' ' test_might_fail git config --unset core.pager && diff --git a/t/lib-read-tree.sh b/t/lib-read-tree.sh index abc2c6f..6442ae3 100644 --- a/t/lib-read-tree.sh
[PATCH 9/9] remove #!interpreter line from shell libraries
In a shell snippet meant to be sourced by other shell scripts, an opening #! line does more harm than good. The harm: - When the shell library is sourced, the interpreter and options from the #! line are not used. Specifying a particular shell can confuse the reader into thinking it is safe for the shell library to rely on idiosyncrasies of that shell. - Using #! instead of a plain comment drops a helpful visual clue that this is a shell library and not a self-contained script. - Tools such as lintian can use a #! line to tell when an installation script has failed by forgetting to set a script executable. This check does not work if shell libraries also start with a #! line. The good: - Text editors notice the #! line and use it for syntax highlighting if you try to edit the installed scripts (without ".sh" suffix) in place. The use of the #! for file type detection is not needed because Git's shell libraries are meant to be edited in source form (with ".sh" suffix). Replace the opening #! lines with comments. This involves tweaking the test harness's valgrind support to find shell libraries by looking for "# " in the first line instead of "#!" (see v1.7.6-rc3~7, 2011-06-17). Suggested by Russ Allbery through lintian. Thanks to Jeff King and Clemens Buchacher for further analysis. Tested by searching for non-executable scripts with #! line: find . -name .git -prune -o -type f -not -executable | while read file do read line <"$file" case $line in '#!'*) echo "$file" ;; esac done The only remaining scripts found are templates for shell scripts (unimplemented.sh, wrap-for-bin.sh) and sample input used in tests (t/t4034/perl/{pre,post}). Signed-off-by: Jonathan Nieder --- Thanks for reading. contrib/completion/git-completion.bash | 2 -- contrib/completion/git-completion.tcsh | 2 -- git-mergetool--lib.sh | 3 +-- git-parse-remote.sh| 4 +++- git-rebase--am.sh | 3 ++- git-rebase--interactive.sh | 9 +++-- git-rebase--merge.sh | 4 +++- git-sh-i18n.sh | 5 ++--- git-sh-setup.sh| 9 +++-- t/test-lib.sh | 6 ++ 10 files changed, 19 insertions(+), 28 deletions(-) diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash index dba3c15..be61931 100644 --- a/contrib/completion/git-completion.bash +++ b/contrib/completion/git-completion.bash @@ -1,5 +1,3 @@ -#!bash -# # bash/zsh completion support for core Git. # # Copyright (C) 2006,2007 Shawn O. Pearce diff --git a/contrib/completion/git-completion.tcsh b/contrib/completion/git-completion.tcsh index eaacaf0..6104a42 100644 --- a/contrib/completion/git-completion.tcsh +++ b/contrib/completion/git-completion.tcsh @@ -1,5 +1,3 @@ -#!tcsh -# # tcsh completion support for core Git. # # Copyright (C) 2012 Marc Khouzam diff --git a/git-mergetool--lib.sh b/git-mergetool--lib.sh index a280f49..c45a020 100644 --- a/git-mergetool--lib.sh +++ b/git-mergetool--lib.sh @@ -1,5 +1,4 @@ -#!/bin/sh -# git-mergetool--lib is a library for common merge tool functions +# git-mergetool--lib is a shell library for common merge tool functions : ${MERGE_TOOLS_DIR=$(git --exec-path)/mergetools} diff --git a/git-parse-remote.sh b/git-parse-remote.sh index 0e87e09..55fe8d5 100644 --- a/git-parse-remote.sh +++ b/git-parse-remote.sh @@ -1,4 +1,6 @@ -#!/bin/sh +# This is a shell library to calculate the remote repository and +# upstream branch that should be pulled by "git pull" from the current +# branch. # git-ls-remote could be called from outside a git managed repository; # this would fail in that case and would issue an error message. diff --git a/git-rebase--am.sh b/git-rebase--am.sh index 34e3102..a4f683a 100644 --- a/git-rebase--am.sh +++ b/git-rebase--am.sh @@ -1,4 +1,5 @@ -#!/bin/sh +# This shell script fragment is sourced by git-rebase to implement +# its default, fast, patch-based, non-interactive mode. # # Copyright (c) 2010 Junio C Hamano. # diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh index 3c6bed9..43c19e0 100644 --- a/git-rebase--interactive.sh +++ b/git-rebase--interactive.sh @@ -1,11 +1,8 @@ -#!/bin/sh +# This shell script fragment is sourced by git-rebase to implement +# its interactive mode. "git rebase --interactive" makes it easy +# to fix up commits in the middle of a series and rearrange commits. # # Copyright (c) 2006 Johannes E. Schindelin - -# SHORT DESCRIPTION -# -# This script makes it easy to fix up commits in the middle of a series, -# and rearrange commits. # # The original idea comes from Eric W. Biederman, in # http://article.gmane.org/gmane.comp.version-control.git/22407 diff --git a/git-rebase--merge.sh b/git-re
Re: [PATCH v3 06/28] connect.c: teach get_remote_heads to parse "shallow" lines
Nguyễn Thái Ngọc Duy writes: > No callers pass a non-empty pointer as shallow_points at this > stage. As a result, all clients still refuse to talk to shallow > repository on the other end. > > Signed-off-by: Nguyễn Thái Ngọc Duy > --- > builtin/fetch-pack.c | 2 +- > builtin/send-pack.c | 2 +- > connect.c| 12 +++- > remote-curl.c| 2 +- > remote.h | 3 ++- > transport.c | 7 --- > 6 files changed, 20 insertions(+), 8 deletions(-) > > diff --git a/builtin/fetch-pack.c b/builtin/fetch-pack.c > index c8e8582..c1d918f 100644 > --- a/builtin/fetch-pack.c > +++ b/builtin/fetch-pack.c > @@ -150,7 +150,7 @@ int cmd_fetch_pack(int argc, const char **argv, const > char *prefix) > args.verbose ? CONNECT_VERBOSE : 0); > } > > - get_remote_heads(fd[0], NULL, 0, &ref, 0, NULL); > + get_remote_heads(fd[0], NULL, 0, &ref, 0, NULL, NULL); > > ref = fetch_pack(&args, fd, conn, ref, dest, >sought, nr_sought, pack_lockfile_ptr); > diff --git a/builtin/send-pack.c b/builtin/send-pack.c > index 51121f2..bfa9253 100644 > --- a/builtin/send-pack.c > +++ b/builtin/send-pack.c > @@ -233,7 +233,7 @@ int cmd_send_pack(int argc, const char **argv, const char > *prefix) > > memset(&extra_have, 0, sizeof(extra_have)); > > - get_remote_heads(fd[0], NULL, 0, &remote_refs, REF_NORMAL, &extra_have); > + get_remote_heads(fd[0], NULL, 0, &remote_refs, REF_NORMAL, &extra_have, > NULL); > > transport_verify_remote_names(nr_refspecs, refspecs); > > diff --git a/connect.c b/connect.c > index 06e88b0..d0602b0 100644 > --- a/connect.c > +++ b/connect.c > @@ -122,7 +122,8 @@ static void annotate_refs_with_symref_info(struct ref > *ref) > */ > struct ref **get_remote_heads(int in, char *src_buf, size_t src_len, > struct ref **list, unsigned int flags, > - struct extra_have_objects *extra_have) > + struct extra_have_objects *extra_have, > + struct extra_have_objects *shallow_points) The _shape_ of the information you would want to keep track of for the shallow cut-off points may exactly be the same as that is for extra-have endspoints, but it still feels wrong to throw these shallow cut-off points into a structure called "extra have". After all, these are objects the other end does not have, which is the direct opposite of extra-have. Perhaps a preparatory patch needs to rename the structure type to object_name_list or something. And then we can make the variable names, not typenames, responsible for signalling what they mean, i.e. get_remote_heads(... struct list_of_objects *extra_have, struct list_of_objects *shallow_points); when we introduce the new parameter. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 07/28] shallow.c: add remove_reachable_shallow_points()
Nguyễn Thái Ngọc Duy writes: > When we receive a pack and the shallow points from another repository, > we may want to add more shallow points to current repo to make sure no > commits point to nowhere. However we do not want to add unnecessary > shallow points and cut our history short because the other end is a > shallow version of this repo. The output shallow points may or may not > be added to .git/shallow, depending on whether they are actually > reachable in the new pack. > > This function filters such shallow points out, leaving ones that might > potentially be added. A simple has_sha1_file won't do because we may > have incomplete object islands (i.e. not connected to any refs) and > the shallow points are on these islands. In that case we want to keep > the shallow points as candidates until we figure out if the new pack > connects to such object islands. > > Typical cases that use remove_reachable_shallow_points() are: > > - fetch from a repo that has longer history: in this case all remote >shallow roots do not exist in the client repo, this function will >be cheap as it just does a few lookup_commit_graft and >has_sha1_file. It is unclear why. If you fetched from a repository more complete than you, you may deepen your history which may allow you to unplug the shallow points you originally had, and remote "shallow root" (by the way, lets find a single good phrase to represent this thing and stick to it) may want to replace them, no? > - fetch from a repo that has exactly the same shallow root set >(e.g. a clone from a shallow repo): this case may trigger >in_merge_bases_many all the way to roots. An exception is made to >avoid such costly path with a faith that .git/shallow does not >usually points to unreachable commit islands. ... and when the faith is broken, you will end up with a broken repository??? > - push from a shallow repo that has shorter history than the >server's: in_merge_bases_many() is unavoidable, so the longer >history the client has the higher the server cost is. The cost may >be reduced with the help of pack bitmaps, or commit cache, though. > > - push from a shallow clone that has exactly the same shallow root >set: the same as the second fetch case above, .i.e. cheap by >exception. > > The function must be called before the new pack is installed, or we > won't know which objects are ours, which theirs. > > Signed-off-by: Nguyễn Thái Ngọc Duy > --- > commit.h | 3 +++ > connect.c | 2 +- > remote.h | 1 + > shallow.c | 45 + > 4 files changed, 50 insertions(+), 1 deletion(-) > > diff --git a/commit.h b/commit.h > index a879526..98044e6 100644 > --- a/commit.h > +++ b/commit.h > @@ -193,6 +193,7 @@ extern struct commit_list *get_octopus_merge_bases(struct > commit_list *in); > /* largest positive number a signed 32-bit integer can contain */ > #define INFINITE_DEPTH 0x7fff > > +struct extra_have_objects; > extern int register_shallow(const unsigned char *sha1); > extern int unregister_shallow(const unsigned char *sha1); > extern int for_each_commit_graft(each_commit_graft_fn, void *); > @@ -206,6 +207,8 @@ extern void setup_alternate_shallow(struct lock_file > *shallow_lock, > const char **alternate_shallow_file); > extern char *setup_temporary_shallow(void); > extern void advertise_shallow_grafts(int); > +extern void remove_reachable_shallow_points(struct extra_have_objects *out, > + const struct extra_have_objects > *in); > > int is_descendant_of(struct commit *, struct commit_list *); > int in_merge_bases(struct commit *, struct commit *); > diff --git a/connect.c b/connect.c > index d0602b0..80e4360 100644 > --- a/connect.c > +++ b/connect.c > @@ -45,7 +45,7 @@ int check_ref_type(const struct ref *ref, int flags) > return check_ref(ref->name, strlen(ref->name), flags); > } > > -static void add_extra_have(struct extra_have_objects *extra, unsigned char > *sha1) > +void add_extra_have(struct extra_have_objects *extra, unsigned char *sha1) > { > ALLOC_GROW(extra->array, extra->nr + 1, extra->alloc); > hashcpy(&(extra->array[extra->nr][0]), sha1); > diff --git a/remote.h b/remote.h > index 773faa9..ff604ff 100644 > --- a/remote.h > +++ b/remote.h > @@ -145,6 +145,7 @@ extern struct ref **get_remote_heads(int in, char > *src_buf, size_t src_len, >struct ref **list, unsigned int flags, >struct extra_have_objects *have, >struct extra_have_objects *shallow); > +extern void add_extra_have(struct extra_have_objects *extra, unsigned char > *sha1); > > int resolve_remote_symref(struct ref *ref, struct ref *list); > int ref_newer(const unsigned char *new_sha1, const unsigned char *old_sha1); > diff --git a/shallow.c b/shallow.c > index 9552512..a
Re: [PATCH v3 08/28] shallow.c: add mark_new_shallow_refs()
Nguyễn Thái Ngọc Duy writes: > When we receive a pack and the shallow points from another repository, > we may need to add more shallow points to current repo to make sure no > commits point to nowhere. But usually we don't want to do so because > (in future) new shallow points invalidate pack bitmaps and we need to > rebuild them again, which is not cheap. > > So the default way is we allow ref updates that do not introduce new > shallow points and mark the others. If the user is fine with new > shallow point addition, we accept the marked refs. > > But even so we do not blindly accept all shallow points provided. Some > of them might not point to any commits in the new pack. Some might > even do, but those might be unreachable object islands. Only shallow > points that are reachable from old and new refs can stay. > > The way it's implemented is paint down from each ref, attach a bitmap > to each commit where one bit represents one ref. In order to avoid > allocating new bitmap for every commit, we try to reuse the same > bitmap for parent commits if possible. This reduces allocation and > leaks deliberately because it's hard to keep/time consuming track how > many pointers to the same buffer. > > In a typical push or fetch, the new pack should not carry new shallow > roots. If the current repo does not have any commit islands refered by > the sender's shallow roots either, this function is just a few > has_sha1_file(). So quite cheap. > > Once the sender diverts from that path (or the receiver detects > shallow roots attached to commit islands from > remove_reachable_shallow_points), > it'll be a lot more expensive. Pack bitmaps won't help this kind of > commit traversal, but commit cache might. > > Signed-off-by: Nguyễn Thái Ngọc Duy > --- > commit.h | 4 ++ > shallow.c | 219 > ++ > 2 files changed, 223 insertions(+) Hmph. the use of ->util field in this patch feels that it was something commit-slab data structure was invented to solve. > diff --git a/shallow.c b/shallow.c > index a974d2d..c92a1dc 100644 > --- a/shallow.c > +++ b/shallow.c > @@ -4,6 +4,8 @@ > #include "pkt-line.h" > #include "remote.h" > #include "refs.h" > +#include "diff.h" > +#include "revision.h" > > static int is_shallow = -1; > static struct stat shallow_stat; > @@ -280,3 +282,220 @@ void remove_reachable_shallow_points(struct > extra_have_objects *out, > } > free(ca.commits); > } > + > +static int paint_down(const unsigned char *sha1, int id, int nr_bits, int > quick) > +{ > + int hit_bottom = 0; > + unsigned int i, nr; > + struct commit_list *head = NULL; > + int bitmap_nr = (nr_bits + 31) / 32; > + int bitmap_size = bitmap_nr * sizeof(uint32_t); > + uint32_t *tmp = xmalloc(bitmap_size); > + uint32_t *bitmap = xcalloc(bitmap_size, sizeof(uint32_t)); > + bitmap[id / 32] |= (1 << (id % 32)); > + commit_list_insert(lookup_commit(sha1), &head); > + while (head) { > + struct commit_list *p; > + struct commit *c = head->item; > + uint32_t *c_util = c->util; > + > + p = head; > + head = head->next; > + free(p); > + > + if (c->object.flags & (SEEN | UNINTERESTING)) > + continue; > + else > + c->object.flags |= SEEN; > + > + if (c->util == NULL) > + c->util = bitmap; > + else { > + /* > + * Deliberately leak a lot in commit->util > + * because there can be many pointers to the > + * same bitmap. Probably should allocate in a > + * pool and free the whole pool at the end. > + */ ... or perhaps make the bitmap into struct { int refcnt; uint32_t bits[FLEX_ARRAY]; } and refcnt them? > + memcpy(tmp, c_util, bitmap_size); > + for (i = 0; i < bitmap_nr; i++) > + tmp[i] |= bitmap[i]; > + if (memcmp(tmp, c_util, bitmap_size)) { > + c->util = xmalloc(bitmap_size); > + memcpy(c->util, tmp, bitmap_size); > + } > + } > + > + if (c->object.flags & BOTTOM) { > + hit_bottom = 1; > + if (quick) { > + free_commit_list(head); > + break; > + } else > + continue; > + } > + > + if (parse_commit(c)) > + die("unable to parse commit %s", > + sha1_to_hex(c->object.sha1)); > + > + for (p = c->parents; p; p = p->next) { > + if (p->item->object.flags & SEE
Re: [PATCH v3 09/28] shallow.c: extend setup_*_shallow() to accept extra shallow points
Nguyễn Thái Ngọc Duy writes: > Signed-off-by: Nguyễn Thái Ngọc Duy > --- > commit.h | 8 +--- > fetch-pack.c | 5 +++-- > shallow.c | 20 +++- > upload-pack.c | 2 +- > 4 files changed, 24 insertions(+), 11 deletions(-) > > diff --git a/commit.h b/commit.h > index e1fd587..3af4699 100644 > --- a/commit.h > +++ b/commit.h > @@ -203,10 +203,12 @@ extern struct commit_list *get_shallow_commits(struct > object_array *heads, > int depth, int shallow_flag, int not_shallow_flag); > extern void check_shallow_file_for_update(void); > extern void set_alternate_shallow_file(const char *path); > -extern int write_shallow_commits(struct strbuf *out, int use_pack_protocol); > +extern int write_shallow_commits(struct strbuf *out, int use_pack_protocol, > + const struct extra_have_objects *extra); Confusing. Sounds as if you got the extra ".have" and storing them in the .git/shallow file (which of course would not make much sense), which is not what is going on. Also it is unclear how the sanity check the previous step seems to make and the new list of shallow commit names this patch adds to existing functions are designed to interact. I think the whole design needs a better explanation of the flow at the higher level. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 06/28] connect.c: teach get_remote_heads to parse "shallow" lines
Junio C Hamano writes: > Perhaps a preparatory patch needs to rename the structure type to > object_name_list or something. And then we can make the variable > names, not typenames, responsible for signalling what they mean, > i.e. > > get_remote_heads(... > struct list_of_objects *extra_have, > struct list_of_objects *shallow_points); > > when we introduce the new parameter. Yuck, and these are not "list-of-objects", either. They are "list-of-object-names". -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv3] transport: Catch non positive --depth option value
On 22/11/13 02:18, Duy Nguyen wrote: > On Fri, Nov 22, 2013 at 3:18 AM, Junio C Hamano wrote: >> Have you run the tests with this patch? It seems that it breaks >> quite a lot of them, including t5500, t5503, t5510, among others. > > I guess it's caused by builtin/fetch.c:backfill_tags(). And the call > could be replaced with > > transport_set_option(transport, TRANS_OPT_DEPTH, NULL); > Not sure what you mean, https://github.com/git/git/blob/master/t/t5550-http-fetch.sh doesn't call backfill_tags. What do you mean? Thanks -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] bash prompt: add option to disable for a repository
Heikki Hokkanen wrote: > If running git config on each prompt seems too expensive, do you have > any better ideas? Perhaps a GIT_PS1_NOT_FOR_THESE_REPOS=repo1:repo2:repo3 setting would work. __git_ps1 would do the one 'git rev-parse --git-dir --...' to find the repo corresponding to the cwd and then could match against the configured list to decide whether to return early. Hope that helps, Jonathan -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] bash prompt: add option to disable for a repository
On Mon, Nov 25, 2013 at 03:43:44PM -0800, Jonathan Nieder wrote: > Heikki Hokkanen wrote: > > > If running git config on each prompt seems too expensive, do you have > > any better ideas? > > Perhaps a GIT_PS1_NOT_FOR_THESE_REPOS=repo1:repo2:repo3 setting would > work. > > __git_ps1 would do the one 'git rev-parse --git-dir --...' to find the > repo corresponding to the cwd and then could match against the > configured list to decide whether to return early. That would be a better interface, but the implementation must cope with colons in paths on Unixes, case insensitive file systems, and different path representations on Windows (C:\path\to\repo vs. /c/PaTh/To/RePo). I'm not sure we want to go there. Gábor -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] submodule recursion in git-archive
All, My first git patch - so shout out if I’ve got the etiquette wrong! Or of course if I’ve missed something. I googled around looking for solutions to my problem but just came up with a few shell-scripts that didn’t quite get the functionality I needed. The first patch fixes some typos that crept in to existing doc and declarations. It is required for the second which actually implements the changes. All comments gratefully received! Regards Nick Townsend Subject: [PATCH 1/2] submodule: add_submodule_odb() usability Although add_submodule_odb() is documented as being externally usable, it is declared static and also has incorrect documentation. This commit fixes those and makes no changes to existing code using them. All tests still pass. --- Documentation/technical/api-ref-iteration.txt | 4 ++-- submodule.c | 2 +- submodule.h | 1 + 3 files changed, 4 insertions(+), 3 deletions(-) diff --git a/Documentation/technical/api-ref-iteration.txt b/Documentation/technical/api-ref-iteration.txt index aa1c50f..cbee624 100644 --- a/Documentation/technical/api-ref-iteration.txt +++ b/Documentation/technical/api-ref-iteration.txt @@ -50,10 +50,10 @@ submodules object database. You can do this by a code-snippet like this: const char *path = "path/to/submodule" - if (!add_submodule_odb(path)) + if (add_submodule_odb(path)) die("Error submodule '%s' not populated.", path); -`add_submodule_odb()` will return an non-zero value on success. If you +`add_submodule_odb()` will return a zero value on success. If you do not do this you will get an error for each ref that it does not point to a valid object. diff --git a/submodule.c b/submodule.c index 1905d75..1ea46be 100644 --- a/submodule.c +++ b/submodule.c @@ -143,7 +143,7 @@ void stage_updated_gitmodules(void) die(_("staging updated .gitmodules failed")); } -static int add_submodule_odb(const char *path) +int add_submodule_odb(const char *path) { struct strbuf objects_directory = STRBUF_INIT; struct alternate_object_database *alt_odb; diff --git a/submodule.h b/submodule.h index 7beec48..3e3cdca 100644 --- a/submodule.h +++ b/submodule.h @@ -41,5 +41,6 @@ int find_unpushed_submodules(unsigned char new_sha1[20], const char *remotes_nam struct string_list *needs_pushing); int push_unpushed_submodules(unsigned char new_sha1[20], const char *remotes_name); void connect_work_tree_and_git_dir(const char *work_tree, const char *git_dir); +int add_submodule_odb(const char *path); #endif -- 1.8.3.4 (Apple Git-47) Subject: [PATCH 2/2] archive: allow submodule recursion on git-archive When using git-archive to produce a dump of a repository, the existing code does not recurse into a submodule when it encounters it in the tree traversal. These changes add a command line flag that permits this. Note that the submodules must be updated in the repository, otherwise this cannot take place. The feature is disabled for remote repositories as the git_work_tree fails. This is a possible future enhancement. Two additional fields are added to archiver_args: * recurse - a boolean indicator * treepath - the path part of the tree-ish eg. the 'www' in HEAD:www The latter is used within the archive writer to determin the correct path for the submodule .git file. Signed-off-by: Nick Townsend --- Documentation/git-archive.txt | 9 + archive.c | 38 -- archive.h | 2 ++ 3 files changed, 47 insertions(+), 2 deletions(-) diff --git a/Documentation/git-archive.txt b/Documentation/git-archive.txt index b97aaab..b4df735 100644 --- a/Documentation/git-archive.txt +++ b/Documentation/git-archive.txt @@ -11,6 +11,7 @@ SYNOPSIS [verse] 'git archive' [--format=] [--list] [--prefix=/] [] [-o | --output=] [--worktree-attributes] + [--recursive|--recurse-submodules] [--remote= [--exec=]] [...] @@ -51,6 +52,14 @@ OPTIONS --prefix=/:: Prepend / to each filename in the archive. +--recursive:: +--recurse-submodules:: + Archive entries in submodules. Errors occur if the submodules + have not been initialized and updated. + Run `git submodule update --init --recursive` immediately after + the clone is finished to avoid this. + This option is not available with remote repositories. + -o :: --output=:: Write the archive to instead of stdout. diff --git a/archive.c b/archive.c index 346f3b2..f6313c9 100644 --- a/archive.c +++ b/archive.c @@ -5,6 +5,7 @@ #include "archive.h" #include "parse-options.h" #include "unpack-trees.h" +#include "submodule.h" static char const * const archive_usage[] = { N_("git archive [options] [...]"), @@ -131,13 +132,32 @@ static int write_archive_entry(const unsigned char
Re: [PATCHv3] transport: Catch non positive --depth option value
On Tue, Nov 26, 2013 at 6:34 AM, "Andrés G. Aragoneses" wrote: > On 22/11/13 02:18, Duy Nguyen wrote: >> On Fri, Nov 22, 2013 at 3:18 AM, Junio C Hamano wrote: >>> Have you run the tests with this patch? It seems that it breaks >>> quite a lot of them, including t5500, t5503, t5510, among others. >> >> I guess it's caused by builtin/fetch.c:backfill_tags(). And the call >> could be replaced with >> >> transport_set_option(transport, TRANS_OPT_DEPTH, NULL); >> > > Not sure what you mean, > https://github.com/git/git/blob/master/t/t5550-http-fetch.sh doesn't > call backfill_tags. What do you mean? I wrote "I guess" ;-) I did not check what t5550 does. > > Thanks > -- Duy -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Filter log based on paths NOT touched
On Tue, Nov 26, 2013 at 3:35 AM, Junio C Hamano wrote: > Phillip Susi writes: > >> I can't seem to find a way to invert the meaning of a pathspec given >> to git log in order to find commits touching anything BUT a given >> path. Does such a thing exist? > > Not yet (look for "negative pathspec"). There's a difference between "skip commits that touch anything directory foo even if it also touches something outside of foo" and "skip commits that _only_ touches something in foo". Not sure which way Phillip wants here. Negative pathspec only supports the latter. The former needs a new option in rev-list, because the logic to consider a commit not a match if it matches a pathspec is out of tree_entry_interesting()'s control. -- Duy -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Filter log based on paths NOT touched
-BEGIN PGP SIGNED MESSAGE- Hash: SHA512 On 11/25/2013 10:13 PM, Duy Nguyen wrote: > There's a difference between "skip commits that touch anything > directory foo even if it also touches something outside of foo" and > "skip commits that _only_ touches something in foo". Not sure which > way Phillip wants here. Negative pathspec only supports the latter. > The former needs a new option in rev-list, because the logic to > consider a commit not a match if it matches a pathspec is out of > tree_entry_interesting()'s control. I'm looking for the latter. Specifically I'm trying to see what changes to the upstream code a debian package has made so I need to ignore commits that only change things in debian/. -BEGIN PGP SIGNATURE- Version: GnuPG v1.4.14 (GNU/Linux) Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/ iQEcBAEBCgAGBQJSlCTcAAoJEJrBOlT6nu75jUQIAJOybMyXE6lr4y4aX6od02Rn lINvPVC1Xub2E1lsofIB+N4QvZDRWlIJOIcj3GRqirzUcNBaIOUxUW4+UV7KgMQH wmImS8DdBgrkI8Sompe9B0gqd4lmB2mzTutQfOzdEZ/ZOU50935daYboK//X7zM/ vSmumJpRwNfw1BKyuxVpAQ8Ablo9yUu2cswLKJPSKyQkT9d/AJn07LE5/DKZORQN RD3r94kToftgTfsQTJbNpPujJ5nVHwVmiC44Qghdquj54l++ai5Xs2wRU3Za3i+O ts7Tn5Lrw+pFAT5Lnt0v8Yedp7fVgjRhIsNEDIVlgNuHevP/oBOXIxuLJBlCBIE= =AYYO -END PGP SIGNATURE- -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 8/9] test: replace shebangs with descriptions in shell libraries
On Mon, Nov 25, 2013 at 4:03 PM, Jonathan Nieder wrote: > A #! line in these files is misleading, since these scriptlets are > meant to be sourced with '.' (using whatever shell sources them) > instead of run directly using the interpreter named on the #! line. > > Removing the #! line shouldn't hurt syntax highlighting since > these files have filenames ending with '.sh'. For documentation, > add a brief description of how the files are meant to be used in > place of the shebang line. > > Signed-off-by: Jonathan Nieder > --- > diff --git a/t/lib-bash.sh b/t/lib-bash.sh > index 11397f7..10b76df 100644 > --- a/t/lib-bash.sh > +++ b/t/lib-bash.sh > @@ -1,7 +1,6 @@ > -#!/bin/sh > -# > -# Ensures that tests are run under Bash; primarily intended for running tests > -# of the completion script. > +# Shell library sourced instead of ./test-lib.sh by tests that need > +# to run under Bash; primary intended for tests of the completion s/primary/primarily/ > +# script. > > if test -n "$BASH" && test -z "$POSIXLY_CORRECT"; then > # we are in full-on bash mode -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 03/28] clone: prevent --reference to a shallow repository
On Sun, Nov 24, 2013 at 10:55 PM, Nguyễn Thái Ngọc Duy wrote: > If we borrow objects from another repository, we should also pay > attention to their $GIT_DIR/shallow (and even info/grafts). But > current alternates code does not. > > Reject alternate repos that are shallow because we do not do it > right. In future we alternate code may be updated to check s/we/the/ > $GIT_DIR/shallow properly so that this restriction could be lifted. > > Signed-off-by: Nguyễn Thái Ngọc Duy > --- > builtin/clone.c | 6 ++ > 1 file changed, 6 insertions(+) > > diff --git a/builtin/clone.c b/builtin/clone.c > index 874e0fd..900f564 100644 > --- a/builtin/clone.c > +++ b/builtin/clone.c > @@ -252,6 +252,12 @@ static int add_one_reference(struct string_list_item > *item, void *cb_data) > die(_("reference repository '%s' is not a local repository."), > item->string); > > + if (!access(mkpath("%s/shallow", ref_git), F_OK)) > + die(_("reference repository '%s' is shallow"), item->string); > + > + if (!access(mkpath("%s/info/grafts", ref_git), F_OK)) > + die(_("reference repository '%s' is grafted"), item->string); > + > strbuf_addf(&alternate, "%s/objects", ref_git); > add_to_alternates_file(alternate.buf); > strbuf_release(&alternate); > -- > 1.8.2.83.gc99314b -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] bash prompt: add option to disable for a repository
Am 11/26/2013 0:43, schrieb Jonathan Nieder: > Heikki Hokkanen wrote: > >> If running git config on each prompt seems too expensive, do you have >> any better ideas? > > Perhaps a GIT_PS1_NOT_FOR_THESE_REPOS=repo1:repo2:repo3 setting would > work. Yeah, but... I find the wish to show the bash prompt in some, but not all, repositories so uncommon that I doubt that it must be a feature of __git_ps1. There can be a wrapper function that does the repository discovery and calls into __git_ps1 as needed. No current __git_ps1 users need to be burdened. -- Hannes -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html