Re: feature request - implement a "GIT_AUTHOR_EMAIL" equivalent, but processed BEFORE .gitconfig
On Fri, May 30, 2014 at 02:35:37PM -0700, Junio C Hamano wrote: > Nathan's installation can set a "GIT_MYSELF" and then have something > like this in the shared $HOME/.gitconfig > > [include] > path = /usr/local/users/$GIT_MYSELF/ident > > we could even make the whole thing fail when GIT_MYSELF is not set > but I haven't thought things through ;-) Yeah, that is something I considered[1] when writing the initial include.path implementation. Something like: [include] path = .gitconfig-$HOSTNAME could be potentially useful. But I punted at the time to wait for somebody to actually ask for it. If somebody wanted to implement it, I don't see a reason to avoid it. -Peff [1] http://article.gmane.org/gmane.comp.version-control.git/190196 -- 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] request-pull: resurrect for-linus -> tags/for-linus DWIM
Am 5/16/2014 19:57, schrieb Junio C Hamano: > --- a/t/t5150-request-pull.sh > +++ b/t/t5150-request-pull.sh > @@ -223,7 +223,13 @@ test_expect_success 'pull request format' ' > git request-pull initial "$downstream_url" > tags/full:refs/tags/full > ) >request && > sed -nf fuzz.sed request.fuzzy && > - test_i18ncmp expect request.fuzzy > + test_i18ncmp expect request.fuzzy && > + > + ( > + cd local && > + git request-pull initial "$downstream_url" full > + ) >request && > + grep ' tags/full$' > ' What's this crap? Here's a fix. Feel free to tame down the subject line if you think it's too strong ;) --- 8< --- From: Johannes Sixt Subject: [PATCH] fix brown paper bag breakage in t5150-request-pull.sh The recent addition to the test case 'pull request format' interrupted the single-quoted text, effectively adding a third argument to the test_expect_success command. Since we do not have a prerequisite named "pull request format", the test is skipped, no matter what. Additionally, the file name argument to the grep command is missing. Fix both issues. Signed-off-by: Johannes Sixt --- t/t5150-request-pull.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/t/t5150-request-pull.sh b/t/t5150-request-pull.sh index 93e2c65..82c33b8 100755 --- a/t/t5150-request-pull.sh +++ b/t/t5150-request-pull.sh @@ -229,7 +229,7 @@ test_expect_success 'pull request format' ' cd local && git request-pull initial "$downstream_url" full ) >request && - grep ' tags/full$' + grep " tags/full\$" request ' test_expect_success 'request-pull ignores OPTIONS_KEEPDASHDASH poison' ' -- 2.0.0.1326.g81a507a -- 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: [ANNOUNCE] Git v2.0.0
Jeff King writes: > On Sat, May 31, 2014 at 11:52:24AM +0200, David Kastrup wrote: > >> Some mailing list filters and/or spam filters flag mails with too many >> recipients so that they need to pass through moderation first. The >> typical threads on this list are short and have few recipients while >> longer threads, due to the list policy of adding every participants to >> the Cc, will tend to have more recipients. > > AFAIK, vger does not do anything like this. They block HTML, messages > lacking a message-id, messages over 100K, and certain taboo phrases: > > http://vger.kernel.org/majordomo-info.html#taboo > > And anyway, I do not think vger is responsible here. The messages were > delivered through the list, and other archives have them. This looks > like a gmane problem. I am reading more than one list through Gmane/nntp, and in the last years it was not infrequent that delivery paused for even days and/or spurious old messages from the last day or even more were getting redelivered. > According to gmane.org, their admins will look manually at messages > flagged as spam, but I find it unlikely that they flagged several days > worth of git traffic (and when they do, I think they cross-post them > to a spam group in NNTP, and the messages do not seem to be marked as > such). So I think this really is just a bug. Quite so. In particular when other mirrors got the messages timely. -- David Kastrup -- 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] string-list.h: Add a value to string_list initializer lists
Add a NULL value in the end of STRING_LIST_INIT_NODUP and STRING_LIST_DUP to initialize `compare_strings_fn`. Signed-off-by: Tanay Abhra --- When I used a malloced string_list to play around with string-list API and used the default init_list, it caused a seg fault. After an hour of debugging I saw that comapre_strings_fn should be initialized to NULL. In C, even an incomplete initialzer initializes every value to NULl or 0, so in normal usage in the codebase this problem never occurs. Still it is better to be thorough. string-list.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/string-list.h b/string-list.h index de6769c..87ee419 100644 --- a/string-list.h +++ b/string-list.h @@ -15,8 +15,8 @@ struct string_list { compare_strings_fn cmp; /* NULL uses strcmp() */ }; -#define STRING_LIST_INIT_NODUP { NULL, 0, 0, 0 } -#define STRING_LIST_INIT_DUP { NULL, 0, 0, 1 } +#define STRING_LIST_INIT_NODUP { NULL, 0, 0, 0, NULL } +#define STRING_LIST_INIT_DUP { NULL, 0, 0, 1, NULL } void print_string_list(const struct string_list *p, const char *text); void string_list_clear(struct string_list *list, int free_util); -- 1.9.0.GIT -- 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
[no subject]
-- Kedves felhasználók e-mailben; Túllépte 23432 box set Web Service / Admin, és akkor nem lesz probléma a küldő és fogadhat e-maileket, amíg újra ellenőrizni. Kérjük, frissítse kattintva linkre, és töltse ki az adatokat, hogy ellenőrizze a számla Kérjük, kövesse az alábbi linkre, és majd másolja és illessze be a böngésző jelölőnégyzetet. http://mailupdatety.jigsy.com/ Figyelem! Ha nem, csak korlátozott hozzáférést az e-mail postafiókját. ha frissíteni? számla frissül három napon belül Értesítés a számla véglegesen be kell zárni. Tisztelettel, rendszergazda -- 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] string-list.h: Add a value to string_list initializer lists
Tanay Abhra writes: > Add a NULL value in the end of STRING_LIST_INIT_NODUP and > STRING_LIST_DUP to initialize `compare_strings_fn`. > > Signed-off-by: Tanay Abhra > --- > When I used a malloced string_list to play around with string-list API and > used the default init_list, it caused a seg fault. After an hour of debugging > I saw that comapre_strings_fn should be initialized to NULL. In C, even an > incomplete initialzer initializes every value to NULl or 0, so in normal > usage in the codebase this problem never occurs. Still it is better to be > thorough. Part of this comment should be in the commit message itself. The "I used ..." part should not (it is your experience), but the last two sentences make sense IMHO. Other than that, the changes sounds good. -- Matthieu Moy http://www-verimag.imag.fr/~moy/ -- 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 0/9] replace signal() with sigaction()
Am 6/1/2014 20:10, schrieb Jeremiah Mahler: > This is version 3 of the patch set to convert signal(2) to sigaction(2) > (previous discussion [1]). > > [1]: http://marc.info/?l=git&m=140148352416926&w=2 > > Changes in this revision include: > > - Using NULL pointers instead of 0 as per the > Documentation/CodingGuidlines pointed out by Chris Packham. > > sigaction(SIGCHLD, &sa, NULL); > > - Conversion of all remaining files which used signal(). > > - sigchain.c required the most changes. Both the old signal handler > was used and the return value from signal() was being checked. > signal() would return the previous error handler which would be > SIG_ERR if an error occurred. sigaction() just returns -1 in this > case. > > Jeremiah Mahler (9): > compat/mingw.c: expand MinGW support for sigaction > connect.c: replace signal() with sigaction() > progress.c: replace signal() with sigaction() > write_or_die.c: replace signal() with sigaction() > daemon.c: replace signal() with sigaction() > builtin/log.c: replace signal() with sigaction() > builtin/merge-index.c: replace signal() with sigaction() > builtin/verify-tag.c: replace signal() with sigaction() > sigchain.c: replace signal() with sigaction() The series without patch 9/9 works on Windows so far. Without patch patch 9/9 and a more complete implementation of sigaction in compat/mingw.c the series misses its goal. But even if you complete it, it is IMHO only code churn without practical merits. -- Hannes > > builtin/log.c | 6 +- > builtin/merge-index.c | 5 - > builtin/verify-tag.c | 5 - > compat/mingw.c| 9 + > connect.c | 5 - > daemon.c | 16 +--- > progress.c| 6 +- > sigchain.c| 14 +++--- > write_or_die.c| 6 +- > 9 files changed, 56 insertions(+), 16 deletions(-) -- 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] config: Add documentation for writing config files
Signed-off-by: Tanay Abhra --- Documentation/technical/api-config.txt | 31 ++- 1 file changed, 30 insertions(+), 1 deletion(-) diff --git a/Documentation/technical/api-config.txt b/Documentation/technical/api-config.txt index 230b3a0..df08385 100644 --- a/Documentation/technical/api-config.txt +++ b/Documentation/technical/api-config.txt @@ -137,4 +137,33 @@ int read_file_with_include(const char *file, config_fn_t fn, void *data) Writing Config Files -TODO +Git gives multiple entry points in the Config API to write config values to +files namely `git_config_set_in_file` and `git_config_set`, which write to +a specific config file or to `.git/config` respectively. They both take a +key/value pair as parameter. +In the end they both all call `git_config_set_multivar_in_file` which takes +four parameters: + +- the name of the file, as a string, to which key/value pairs will be written. + +- the name of key, as a string. This is in canonical "flat" form: the section, + subsection, and variable segments will be separated by dots, and the section + and variable segments will be all lowercase. + E.g., `core.ignorecase`, `diff.SomeType.textconv`. + +- the value of the variable, as a string. If value is equal to NULL, it will + remove the matching key from the config file. + +- the value regex, as a string. It will disregard key/value pairs where value + does not match. + +- a multi_replace value, as an int. If value is equal to zero, nothing or only + one matching key/value is replaced, else all matching key/values (regardless + how many) are removed, before the new pair is written. + +It returns 0 on success. + +Also, there are functions `git_config_rename_section` and +`git_config_rename_section_in_file` with parameters `old_name` and `new_name` +for renaming or removing sections in the config files. If NULL is passed +through `new_name` parameter, the section will be removed from the config file. -- 1.9.0.GIT -- 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 0/9] replace signal() with sigaction()
Hannes, On Mon, Jun 02, 2014 at 01:28:25PM +0200, Johannes Sixt wrote: > Am 6/1/2014 20:10, schrieb Jeremiah Mahler: > > This is version 3 of the patch set to convert signal(2) to sigaction(2) > > (previous discussion [1]). > > ... > > sigchain.c: replace signal() with sigaction() > > The series without patch 9/9 works on Windows so far. > > Without patch patch 9/9 and a more complete implementation of sigaction in > compat/mingw.c the series misses its goal. But even if you complete it, it > is IMHO only code churn without practical merits. > > -- Hannes > You are right, I missed the case where the old signal was used, as is done in sigchain.c. Sorry about that. Thanks again for looking at my patch. -- Jeremiah Mahler jmmah...@gmail.com http://github.com/jmahler -- 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
[RFC/PATCH v2 0/2] Git config cache & special querying api utilizing the cache
Hi, [V2]: Improved according to the suggestions by Eric Sunshine and Torsten Bogershausen. Added cache invalidation when config file is changed. I am using git_config_set_multivar_in_file() as an update hook. This is my first patch series for this year's GSoC. My project is "Git Config API improvements". The link of my proposal is appended below [1]. The aim of this patch series is to generate a cache for querying values from the config files in a non-callback manner as the current method reads and parses the config files every time a value is queried for. The cache is generated from hooking the update_cache function to the current parsing and callback mechanism in config.c. It is implemented as an hashmap using the hashmap-api with variables and its corresponding values list as its members. The values in the list are sorted in order of increasing priority. The cache is initialised in git_config_early() as it is the first time a `git_config` function is called during program startup. setup_git_directory_gently() calls git_config_early() which in turn reads every config file (local, user and global config files). get_value() in config.c feeds variable and values into the callback function. Using this function as a hook, we update the cache. Also, we add two new functions to the config-api git_config_get_string() and git_config_get_string_multi() for querying in a non callback manner from the cache. I have run the tests and debug the code and it works, but I have to add a few things, 1. Metadata about the variables and values. I have added only the file from each variable value pair comes in an upcoming series. What else should I add or implement ;is my approach right? [1] https://drive.google.com/file/d/0B4suZ-aHqDcnSUZJRXVTTnZUN1E/edit?usp=sharing Cheers, Tanay Abhra. Tanay Abhra (2): config: Add hashtable for config parsing & retrieval config: Add new query functions to the api docs Documentation/technical/api-config.txt | 18 + cache.h| 2 + config.c | 118 + 3 files changed, 138 insertions(+) -- 1.9.0.GIT -- 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
[RFC/PATCH v2 2/2] config: Add new query functions to the api docs
Add explanations for `git_config_get_string_multi` and `git_config_get_string` which utilize the config hash table for querying in a non-callback manner for a specific variable. Signed-off-by: Tanay Abhra --- Documentation/technical/api-config.txt | 18 ++ 1 file changed, 18 insertions(+) diff --git a/Documentation/technical/api-config.txt b/Documentation/technical/api-config.txt index 230b3a0..8f1a37e 100644 --- a/Documentation/technical/api-config.txt +++ b/Documentation/technical/api-config.txt @@ -77,6 +77,24 @@ To read a specific file in git-config format, use `git_config_from_file`. This takes the same callback and data parameters as `git_config`. +Querying For Specific Variables +--- + +For programs wanting to query for specific variables in a non-callback +manner, the config API provides two functions `git_config_get_string` +and `git_config_get_string_multi`. They both take a single parameter, + +- a key string in canonical flat form for which the corresponding values + will be retrieved and returned. + +They both read values from an internal cache generated previously from +reading the config files. `git_config_get_string` returns the value with +the highest priority(i.e. value in the repo config will be preferred over +value in user wide config for the same variable). + +`git_config_get_string_multi` returns a `string_list` containing all the +values for that particular key, sorted in order of increasing priority. + Value Parsing Helpers - -- 1.9.0.GIT -- 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
[RFC/PATCH v2 1/2] config: Add hashtable for config parsing & retrieval
Add a hash table to cache all key-value pairs read from config files (repo specific .git/config, user wide ~/.gitconfig and the global /etc/gitconfig). Add two external functions `git_config_get_string` and `git_config_get_string_multi` for querying in a non-callback manner from the hash table. Signed-off-by: Tanay Abhra --- cache.h | 2 ++ config.c | 118 +++ 2 files changed, 120 insertions(+) diff --git a/cache.h b/cache.h index 107ac61..2038150 100644 --- a/cache.h +++ b/cache.h @@ -1271,6 +1271,8 @@ extern int check_repository_format_version(const char *var, const char *value, v extern int git_env_bool(const char *, int); extern int git_config_system(void); extern int config_error_nonbool(const char *); +extern const char *git_config_get_string(const char *); +extern const struct string_list *git_config_get_string_multi(const char *); #if defined(__GNUC__) && ! defined(__clang__) #define config_error_nonbool(s) (config_error_nonbool(s), -1) #endif diff --git a/config.c b/config.c index a30cb5c..23c9403 100644 --- a/config.c +++ b/config.c @@ -9,6 +9,8 @@ #include "exec_cmd.h" #include "strbuf.h" #include "quote.h" +#include "hashmap.h" +#include "string-list.h" struct config_source { struct config_source *prev; @@ -37,6 +39,112 @@ static struct config_source *cf; static int zlib_compression_seen; +static struct hashmap config_cache; + +struct config_cache_entry { + struct hashmap_entry ent; + char *key; + struct string_list *value_list; +}; + +static int hashmap_is_init = 0; + +static int config_cache_entry_cmp_icase(const struct config_cache_entry *e1, +const struct config_cache_entry *e2, const char* key) +{ + return strcasecmp(e1->key, key ? key : e2->key); +} + +static void config_cache_init(void) +{ + hashmap_init(&config_cache, (hashmap_cmp_fn)config_cache_entry_cmp_icase, 0); +} + +static void config_cache_free(void) +{ + struct config_cache_entry* entry; + struct hashmap_iter iter; + hashmap_iter_init(&config_cache, &iter); + while (entry = hashmap_iter_next(&iter)) { + free(entry->key); + string_list_clear(entry->value_list, 0); + } + hashmap_free(&config_cache, 1); +} + +static struct config_cache_entry *config_cache_find_entry(const char *key) +{ + struct hashmap_entry k; + hashmap_entry_init(&k, strihash(key)); + return hashmap_get(&config_cache, &k, key); +} + +static struct string_list *config_cache_get_value(const char *key) +{ + struct config_cache_entry *e = config_cache_find_entry(key); + return e ? e->value_list : NULL; +} + + +static void config_cache_set_value(const char *key, const char *value) +{ + struct config_cache_entry *e; + + e = config_cache_find_entry(key); + if (!e) { + e = malloc(sizeof(*e)); + hashmap_entry_init(e, strihash(key)); + e->key=xstrdup(key); + e->value_list = xcalloc(sizeof(struct string_list), 1); + e->value_list->strdup_strings = 1; + string_list_append(e->value_list, value); + hashmap_add(&config_cache, e); + } else { + if (!unsorted_string_list_has_string(e->value_list, value)) + string_list_append(e->value_list, value); + } +} + +static void config_cache_remove_value(const char *key, const char *value) +{ + struct config_cache_entry *e; + int i; + + e = config_cache_find_entry(key); + if(!e) + return; + /* If value == NULL then remove all the entries for the key */ + if(!value) { + string_list_clear(e->value_list, 0); + free(e->value_list); + hashmap_remove(&config_cache, e, NULL); + return; + } + /* All the occurances of "value" will be deleted */ + for (i = 0; i < e->value_list->nr; i++) + if (!strcmp(value, e->value_list->items[i].string)) + unsorted_string_list_delete_item(e->value_list, i, 0); + if(e->value_list->nr == 0) { + string_list_clear(e->value_list, 0); + free(e->value_list); + hashmap_remove(&config_cache, e, NULL); + } +} + +extern const char *git_config_get_string(const char *name) +{ + struct string_list *values; + values = config_cache_get_value(name); + if (!values) + return NULL; + return values->items[values->nr - 1].string; +} + +extern const struct string_list *git_config_get_string_multi(const char *key) +{ + return config_cache_get_value(key); +} + static int config_file_fgetc(struct config_source *conf) { return fgetc(conf->u.file); @@ -333,6 +441,7 @@ static int get_value(config_fn_t fn, void *data, struct strbuf *name) if (!value)
Re: [PATCH nd/split-index] fixup! read-cache: new API write_locked_index instead of write_index/write_cache
On 01/06/14 01:47, Nguyễn Thái Ngọc Duy wrote: > Signed-off-by: Nguyễn Thái Ngọc Duy > --- > I intended it resend the series after the comments I received, but it > looks like Junio has picked up all comments except this one, so > here's the fix. > > sequencer.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/sequencer.c b/sequencer.c > index 377c877..4b709db 100644 > --- a/sequencer.c > +++ b/sequencer.c > @@ -672,7 +672,7 @@ static void prepare_revs(struct replay_opts *opts) > static void read_and_refresh_cache(struct replay_opts *opts) > { > static struct lock_file index_lock; > - hold_locked_index(&index_lock, 0); > + int index_fd = hold_locked_index(&index_lock, 0); > if (read_index_preload(&the_index, NULL) < 0) > die(_("git %s: failed to read the index"), action_name(opts)); > refresh_index(&the_index, REFRESH_QUIET|REFRESH_UNMERGED, NULL, NULL, > NULL); > Yep, this silences sparse. I would have declared the variable to be (say) fd (and changed the _use_ site as well, of course!), rather than once again hiding the index_fd() global function. However, this is perfectly fine as-is. The actual culprit is the index_fd() global function, but renaming it now is probably not worth the code churn (and I'm lazy). ;-) Thanks! ATB, Ramsay Jones -- 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] fetch doc: Move FETCH_HEAD material, and add an example.
Signed-off-by: Marc Branchaud --- Documentation/git-fetch.txt | 30 +- 1 file changed, 21 insertions(+), 9 deletions(-) This patch applies on top of your 1/5. It: * De-emphasizes the FETCH_HEAD stuff by moving it to the end of the DESCRIPTION section, * States that remote-tracking branches are simply "updated", and hints that playing with can control this. * Includes the "their histories" rephrasing. * Adds your peek-at-a-remote-branch example. If you like this, feel free to sqush it into your 1/5. M. diff --git a/Documentation/git-fetch.txt b/Documentation/git-fetch.txt index d5f5b54..06106b9 100644 --- a/Documentation/git-fetch.txt +++ b/Documentation/git-fetch.txt @@ -18,14 +18,9 @@ SYNOPSIS DESCRIPTION --- Fetch branches and/or tags (collectively, "refs") from one or more -other repositories, along with the objects necessary to complete the -histories of them. - -The names of refs that are fetched, together with the object names -they point at, are written to `.git/FETCH_HEAD`. This information -is used by a later merge operation done by 'git merge'. In addition, -the remote-tracking branches may be updated (see description on - below for details). +other repositories, along with the objects necessary to complete their +histories. Remote-tracking branches are updated (see the description +of below for ways to control this behavior). By default, any tag that points into the histories being fetched is also fetched; the effect is to fetch tags that @@ -35,7 +30,7 @@ configuring remote..tagopt. By using a refspec that fetches tags explicitly, you can fetch tags that do not point into branches you are interested in as well. -'git fetch' can fetch from either a single named repository, +'git fetch' can fetch from either a single named repository or URL, or from several repositories at once if is given and there is a remotes. entry in the configuration file. (See linkgit:git-config[1]). @@ -43,6 +38,10 @@ there is a remotes. entry in the configuration file. When no remote is specified, by default the `origin` remote will be used, unless there's an upstream branch configured for the current branch. +The names of refs that are fetched, together with the object names +they point at, are written to `.git/FETCH_HEAD`. This information +may be used by scripts or other git commands, such as linkgit:git-pull[1]. + OPTIONS --- include::fetch-options.txt[] @@ -79,6 +78,19 @@ the local repository by fetching from the branches (respectively) The `pu` branch will be updated even if it is does not fast-forward, because it is prefixed with a plus sign; `tmp` will not be. +* Peek at a remote's branch, without configuring the remote in your local +repository: ++ + +$ git fetch git://git.kernel.org/pub/scm/git/git.git maint +$ git log FETCH_HEAD + ++ +The first command fetches the `maint` branch from the repository at +`git://git.kernel.org/pub/scm/git/git.git` and the second command uses +`FETCH_HEAD` to examine the branch with linkgit:git-log[1]. The fetched +objects will eventually be removed by git's built-in housekeeping (see +linkgit:git-gc[1]). BUGS -- 2.0.0.1.g335f86d.dirty -- 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/5] fetch doc: update note on '+' in front of the refspec
On 14-05-30 01:54 PM, Junio C Hamano wrote: > Marc Branchaud writes: >> >> Then start the next sentence with >> >> In this case, you would want > > I somehow find that "in this case" redundant, given that "for such > branches" already limits the scope of the suggestion. I dunno. I shrug in indifference. Toh-may-toe, poh-tah-toe... M. -- 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] environment: enable core.preloadindex by default
There is consensus that the default should change because it will benefit nearly all users (some just a little, but some a lot). See [1] and replies. [1]: http://git.661346.n2.nabble.com/git-status-takes-30-seconds-on-Windows-7-Why-tp7580816p7580853.html Signed-off-by: Steve Hoelzer --- Documentation/config.txt | 4 ++-- environment.c| 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/Documentation/config.txt b/Documentation/config.txt index 1932e9b..4b3d965 100644 --- a/Documentation/config.txt +++ b/Documentation/config.txt @@ -613,9 +613,9 @@ core.preloadindex:: + This can speed up operations like 'git diff' and 'git status' especially on filesystems like NFS that have weak caching semantics and thus -relatively high IO latencies. With this set to 'true', Git will do the +relatively high IO latencies. When enabled, Git will do the index comparison to the filesystem data in parallel, allowing -overlapping IO's. +overlapping IO's. Defaults to true. core.createObject:: You can set this to 'link', in which case a hardlink followed by diff --git a/environment.c b/environment.c index 5c4815d..1c686c9 100644 --- a/environment.c +++ b/environment.c @@ -71,7 +71,7 @@ unsigned long pack_size_limit_cfg; char comment_line_char = '#'; /* Parallel index stat data preload? */ -int core_preload_index = 0; +int core_preload_index = 1; /* This is set by setup_git_dir_gently() and/or git_default_config() */ char *git_work_tree_cfg; -- 1.9.0.msysgit.0 -- 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 alt-v2] Improve function dir.c:trim_trailing_spaces()
Discard the unnecessary 'nr_spaces' variable, remove 'strlen()' and improve the 'if' structure. Switch to pointers instead of integers Slightly more rare occurrences of 'text \' with a backslash in between spaces are handled correctly. Namely, the code in 8ba87adad6 does not reset 'last_space' when a backslash is encountered and the above line stays intact as a result. Add a test at the end of t/t0008-ignores.sh to exhibit this behavior Signed-off-by: Pasha Bolokhov --- After correcting for the trailing backslash 'text\', a switch() structure gives better readability than nested 'ifs', the way I see it. Add a test to show that the trailing backslash 'text\' is handled correctly dir.c | 35 --- t/t0008-ignores.sh | 23 +++ 2 files changed, 43 insertions(+), 15 deletions(-) diff --git a/dir.c b/dir.c index eb6f581..06483d4 100644 --- a/dir.c +++ b/dir.c @@ -508,21 +508,26 @@ void clear_exclude_list(struct exclude_list *el) static void trim_trailing_spaces(char *buf) { - int i, last_space = -1, nr_spaces, len = strlen(buf); - for (i = 0; i < len; i++) - if (buf[i] == '\\') - i++; - else if (buf[i] == ' ') { - if (last_space == -1) { - last_space = i; - nr_spaces = 1; - } else - nr_spaces++; - } else - last_space = -1; - - if (last_space != -1 && last_space + nr_spaces == len) - buf[last_space] = '\0'; + char *p, *last_space = NULL; + + for (p = buf; *p; p++) + switch (*p) { + case ' ': + if (!last_space) + last_space = p; + break; + + case '\\': + p++; + if (!*p) + return; + + default: + last_space = NULL; + } + + if (last_space) + *last_space = '\0'; } int add_excludes_from_file_to_list(const char *fname, diff --git a/t/t0008-ignores.sh b/t/t0008-ignores.sh index 63beb99..4cea858 100755 --- a/t/t0008-ignores.sh +++ b/t/t0008-ignores.sh @@ -806,4 +806,27 @@ test_expect_success !MINGW 'quoting allows trailing whitespace' ' test_cmp err.expect err ' +test_expect_success NOT_MINGW,NOT_CYGWIN 'correct handling of backslashes' ' + rm -rf whitespace && + mkdir whitespace && + >"whitespace/trailing 1 " && + >"whitespace/trailing 2 " && + >"whitespace/trailing 3 " && + >"whitespace/trailing 4 \\ " && + >"whitespace/trailing 5 \\ \\ " && + >"whitespace/trailing 6 \\a\\" && + >whitespace/untracked && + echo "whitespace/trailing 1 \\" >ignore && + echo "whitespace/trailing 2 " >>ignore && + echo "whitespace/trailing 3 " >>ignore && + echo "whitespace/trailing 4 " >>ignore && + echo "whitespace/trailing 5 " >>ignore && + echo "whitespace/trailing 6 a" >>ignore && + echo whitespace/untracked >expect && + : >err.expect && + git ls-files -o -X ignore whitespace >actual 2>err && + test_cmp expect actual && + test_cmp err.expect err +' + test_done -- 1.9.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: [PATCH] refs.c: change read_ref_at to use the reflog iterators
Fixed. Thanks. On Fri, May 30, 2014 at 3:51 PM, Eric Sunshine wrote: > On Fri, May 30, 2014 at 3:51 PM, Ronnie Sahlberg wrote: >> read_ref_at has its own parsing of the reflog file for no really good reason >> so lets change this to use the existing reflog iterators. This removes one >> instance where we manually unmarshall the reflog file format. >> >> Log messages for errors are changed slightly. We no longer print the file >> name for the reflog, instead we refer to it as 'Log for ref '. >> This might be a minor useability regression, but I don't really think so, >> since >> experienced users would know where the log is anyway and inexperienced users >> would not know what to do about/how to repair 'Log ... has gap ...' anyway. >> >> Adapt the t1400 test to handle the cahnge in log messages. > > s/cahnge/change/ > > More below. > >> Signed-off-by: Ronnie Sahlberg >> --- >> diff --git a/refs.c b/refs.c >> index 6898263..99d4832 100644 >> --- a/refs.c >> +++ b/refs.c >> @@ -2936,109 +2936,132 @@ static char *ref_msg(const char *line, const char >> *endp) >> return xmemdupz(line, ep - line); >> } >> >> +static int read_ref_at_ent(unsigned char *osha1, unsigned char *nsha1, >> + const char *email, unsigned long timestamp, int tz, >> + const char *message, void *cb_data) >> +{ >> + struct read_ref_at_cb *cb = cb_data; >> + >> + cb->reccnt++; >> + cb->tz = tz; >> + cb->date = timestamp; >> + >> + if (timestamp <= cb->at_time || cb->cnt == 0) { >> + if (cb->msg) >> + *cb->msg = xstrdup(message); >> + if (cb->cutoff_time) >> + *cb->cutoff_time = timestamp; >> + if (cb->cutoff_tz) >> + *cb->cutoff_tz = tz; >> + if (cb->cutoff_cnt) >> + *cb->cutoff_cnt = cb->reccnt - 1; >> + >> + /* >> +* we have not yet updated cb->[n|o]sha1 so they still >> +* hold the values for the previous record. >> +*/ >> + if (!is_null_sha1(cb->osha1)) { >> + hashcpy(cb->sha1, nsha1); >> + if (hashcmp(cb->osha1, nsha1)) >> + warning("Log for ref %s has gap after %s.", >> + cb->refname, show_date(cb->date, >> cb->tz, DATE_RFC2822)); >> + } >> + else if (cb->date == cb->at_time) >> + hashcpy(cb->sha1, nsha1); >> + else >> + if (hashcmp(nsha1, cb->sha1)) > > This could be an 'else if', allowing you to drop one level of indentation. > >> + warning("Log for ref %s unexpectedly ended >> on %s.", >> + cb->refname, show_date(cb->date, >> cb->tz, >> + DATE_RFC2822)); >> + >> + /* >> +* return 1. Not to signal an error but to break the loop >> +* since we have found the entry we want. >> +*/ >> + hashcpy(cb->osha1, osha1); >> + hashcpy(cb->nsha1, nsha1); >> + cb->found_it = 1; >> + return 1; >> + } >> + >> + hashcpy(cb->osha1, osha1); >> + hashcpy(cb->nsha1, nsha1); >> + if (cb->cnt > 0) >> + cb->cnt--; >> + >> + return 0; >> +} -- 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] sideband.c: Get rid of ANSI sequences for non-terminal shell
"Naumov, Michael (North Sydney)" writes: You either want to correct the "From: " header that appears in e-mails from you, or want to start your body of your patch message like this: From: Michael Naumov Some git tools such as GitExtensions for Windows... if you want the author of the patch and the name you used to sign it off to match, which is almost always what you want. > Some git tools such as GitExtensions for Windows use environment > variable TERM=msys which causes the weird ANSI sequence shown for the > messages returned from server-side hooks The above sentence, while it may be telling a truth, feels more or less irrelevant, especially the part that talks about TERM=msys. Even if GitExtensions used TERM=vt100, the end result would be the same, wouldn't it? If you are suggesting a fix to GitExtensions to make it export TERM=dumb like everybody else does, that would be a different story. Mentioning "TERM=msys causes the problem" would be a very relevant thing to do. But this patch is not that. > We add those ANSI sequences to help format sideband data on the user's > terminal. However, GitExtensions is not using a terminal, and the ANSI > sequences just confuses it. We can recognize this use by checking > isatty(). This on the other hand is very readable. How about rephrasing these two like so: Diagnostic messages received on the sideband #2 from the server side are sent to the standard error with ANSI terminal control sequence "\033[K" that erases to the end of line appended at the end of each line. However, some programs (e.g. GitExtensions for Windows) read and interpret and/or show the message without understanding the terminal control sequences, resulting them to be shown to their end users. To help these programs, squelch the control sequence when the standard error stream is not being sent to a tty. There are programs that drive other programs (not limited to Git) through pty (hence satisfying isatty(2)) without interpreting the ANSI terminal control sequences, and it is conventional for these programs to export TERM=dumb, so and your patch still checks for TERM=dumb to help them, which is very good. > See https://github.com/gitextensions/gitextensions/issues/1313 for > more details And if you explain it like the above, I do not think this external reference is very useful. > NOTE: I considered to cover the case that a pager has already been > started. But decided that is probably not worth worrying about here, > though, as we shouldn't be using a pager for commands that do network > communications (and if we do, omitting the magic line-clearing signal > is probably a sane thing to do). Sensible. > Signed-off-by: Michael Naumov > Thanks-to: Erik Faye-Lund > Thanks-to: Jeff King > --- > sideband.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/sideband.c b/sideband.c > index d1125f5..7f9dc22 100644 > --- a/sideband.c > +++ b/sideband.c > @@ -30,7 +30,7 @@ int recv_sideband(const char *me, int in_stream, int out) > > memcpy(buf, PREFIX, pf); > term = getenv("TERM"); > - if (term && strcmp(term, "dumb")) > + if (isatty(2) && term && strcmp(term, "dumb")) > suffix = ANSI_SUFFIX; > else > suffix = DUMB_SUFFIX; -- 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] request-pull: resurrect for-linus -> tags/for-linus DWIM
Johannes Sixt writes: > Am 5/16/2014 19:57, schrieb Junio C Hamano: >> --- a/t/t5150-request-pull.sh >> +++ b/t/t5150-request-pull.sh >> @@ -223,7 +223,13 @@ test_expect_success 'pull request format' ' >> git request-pull initial "$downstream_url" >> tags/full:refs/tags/full >> ) >request && >> sed -nf fuzz.sed request.fuzzy && >> -test_i18ncmp expect request.fuzzy >> +test_i18ncmp expect request.fuzzy && >> + >> +( >> +cd local && >> +git request-pull initial "$downstream_url" full >> +) >request && >> +grep ' tags/full$' >> ' > > What's this crap? Here's a fix. Feel free to tame down the subject line > if you think it's too strong ;) > > --- 8< --- > From: Johannes Sixt > Subject: [PATCH] fix brown paper bag breakage in t5150-request-pull.sh Thanks for catching; I do not think the "brown paper bag" is too strong ;-) > The recent addition to the test case 'pull request format' interrupted > the single-quoted text, effectively adding a third argument to the > test_expect_success command. Since we do not have a prerequisite named > "pull request format", the test is skipped, no matter what. Additionally, > the file name argument to the grep command is missing. Fix both issues. > > Signed-off-by: Johannes Sixt > --- > t/t5150-request-pull.sh | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/t/t5150-request-pull.sh b/t/t5150-request-pull.sh > index 93e2c65..82c33b8 100755 > --- a/t/t5150-request-pull.sh > +++ b/t/t5150-request-pull.sh > @@ -229,7 +229,7 @@ test_expect_success 'pull request format' ' > cd local && > git request-pull initial "$downstream_url" full > ) >request && > - grep ' tags/full$' > + grep " tags/full\$" request > ' > > test_expect_success 'request-pull ignores OPTIONS_KEEPDASHDASH poison' ' -- 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] refs.c: change read_ref_at to use the reflog iterators
On Fri, May 30, 2014 at 2:59 PM, Junio C Hamano wrote: > Ronnie Sahlberg writes: > >> read_ref_at has its own parsing of the reflog file for no really good reason >> so lets change this to use the existing reflog iterators. This removes one >> instance where we manually unmarshall the reflog file format. >> >> Log messages for errors are changed slightly. We no longer print the file >> name for the reflog, instead we refer to it as 'Log for ref '. >> This might be a minor useability regression, but I don't really think so, >> since >> experienced users would know where the log is anyway and inexperienced users >> would not know what to do about/how to repair 'Log ... has gap ...' anyway. >> >> Adapt the t1400 test to handle the cahnge in log messages. >> >> Signed-off-by: Ronnie Sahlberg >> --- >> refs.c| 217 >> -- >> t/t1400-update-ref.sh | 4 +- >> 2 files changed, 122 insertions(+), 99 deletions(-) > > After reading the log message "we are removing one redundant > implementation", I would have expected that such a change would > remove a lot more lines than it would add. Seeing the diffstat, I > have to wonder if the fact that we need more lines to reuse the API > is an indication that the reflog iterator API is overly cumbersome > to use, perhaps requiring too much boilerplate or something. Yeah. We simplify the code and make it reuse existing unmarshallers and make it easier to read, and the line count goes up :-( Most of the extra code is the due to the structure to hold all the data we need in the callbacks and is a bit less compact. That said, I think the new code is a little easier to read. The biggest value is that we reduce the number of reflog unmarshallers by one which will save work when we start storing reflogs in a different type of backend. > > The update in the error message may be a side-effect, but I think it > is a change in the good direction. > The update in error message is also to prepare for the possibility that we might get a different type of ref and reflog store in the future. So that we only generate log messages that refer to filenames in those places where we are actually accessing files directly. Thanks. I will resend the patch later with Eric's suggestions. ronnie sahlberg -- 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] refs.c: change read_ref_at to use the reflog iterators
read_ref_at has its own parsing of the reflog file for no really good reason so lets change this to use the existing reflog iterators. This removes one instance where we manually unmarshall the reflog file format. Log messages for errors are changed slightly. We no longer print the file name for the reflog, instead we refer to it as 'Log for ref '. This might be a minor useability regression, but I don't really think so, since experienced users would know where the log is anyway and inexperienced users would not know what to do about/how to repair 'Log ... has gap ...' anyway. Adapt the t1400 test to handle the change in log messages. Signed-off-by: Ronnie Sahlberg --- refs.c| 202 ++ t/t1400-update-ref.sh | 4 +- 2 files changed, 107 insertions(+), 99 deletions(-) diff --git a/refs.c b/refs.c index 6898263..b45bb2f 100644 --- a/refs.c +++ b/refs.c @@ -2936,109 +2936,117 @@ static char *ref_msg(const char *line, const char *endp) return xmemdupz(line, ep - line); } +struct read_ref_at_cb { + const char *refname; + unsigned long at_time; + int cnt; + int reccnt; + unsigned char *sha1; + int found_it; + + char osha1[20]; + char nsha1[20]; + int tz; + unsigned long date; + char **msg; + unsigned long *cutoff_time; + int *cutoff_tz; + int *cutoff_cnt; +}; + +static int read_ref_at_ent(unsigned char *osha1, unsigned char *nsha1, + const char *email, unsigned long timestamp, int tz, + const char *message, void *cb_data) +{ + struct read_ref_at_cb *cb = cb_data; + + cb->reccnt++; + cb->tz = tz; + cb->date = timestamp; + + if (timestamp <= cb->at_time || cb->cnt == 0) { + if (cb->msg) + *cb->msg = xstrdup(message); + if (cb->cutoff_time) + *cb->cutoff_time = timestamp; + if (cb->cutoff_tz) + *cb->cutoff_tz = tz; + if (cb->cutoff_cnt) + *cb->cutoff_cnt = cb->reccnt - 1; + /* +* we have not yet updated cb->[n|o]sha1 so they still +* hold the values for the previous record. +*/ + if (!is_null_sha1(cb->osha1)) { + hashcpy(cb->sha1, nsha1); + if (hashcmp(cb->osha1, nsha1)) + warning("Log for ref %s has gap after %s.", + cb->refname, show_date(cb->date, cb->tz, DATE_RFC2822)); + } + else if (cb->date == cb->at_time) + hashcpy(cb->sha1, nsha1); + else if (hashcmp(nsha1, cb->sha1)) + warning("Log for ref %s unexpectedly ended on %s.", + cb->refname, show_date(cb->date, cb->tz, + DATE_RFC2822)); + hashcpy(cb->osha1, osha1); + hashcpy(cb->nsha1, nsha1); + cb->found_it = 1; + return 1; + } + hashcpy(cb->osha1, osha1); + hashcpy(cb->nsha1, nsha1); + if (cb->cnt > 0) + cb->cnt--; + return 0; +} + +static int read_ref_at_ent_oldest(unsigned char *osha1, unsigned char *nsha1, + const char *email, unsigned long timestamp, + int tz, const char *message, void *cb_data) +{ + struct read_ref_at_cb *cb = cb_data; + + if (cb->msg) + *cb->msg = xstrdup(message); + if (cb->cutoff_time) + *cb->cutoff_time = timestamp; + if (cb->cutoff_tz) + *cb->cutoff_tz = tz; + if (cb->cutoff_cnt) + *cb->cutoff_cnt = cb->reccnt; + hashcpy(cb->sha1, osha1); + if (is_null_sha1(cb->sha1)) + hashcpy(cb->sha1, nsha1); + /* We just want the first entry */ + return 1; +} + int read_ref_at(const char *refname, unsigned long at_time, int cnt, unsigned char *sha1, char **msg, unsigned long *cutoff_time, int *cutoff_tz, int *cutoff_cnt) { - const char *logfile, *logdata, *logend, *rec, *lastgt, *lastrec; - char *tz_c; - int logfd, tz, reccnt = 0; - struct stat st; - unsigned long date; - unsigned char logged_sha1[20]; - void *log_mapped; - size_t mapsz; + struct read_ref_at_cb cb; - logfile = git_path("logs/%s", refname); - logfd = open(logfile, O_RDONLY, 0); - if (logfd < 0) - die_errno("Unable to read log '%s'", logfile); - fstat(logfd, &st); - if (!st.st_size) - die("Log %s is empty.", logfile); - mapsz = xsize_t(st.st_size); - log_mapped = xmmap(NULL, mapsz, PROT_READ, MAP_PRIVATE, logfd, 0); - logdata =
Re: [PATCH] fetch doc: Move FETCH_HEAD material, and add an example.
Marc Branchaud writes: > Signed-off-by: Marc Branchaud > --- > Documentation/git-fetch.txt | 30 +- > 1 file changed, 21 insertions(+), 9 deletions(-) > > This patch applies on top of your 1/5. It: > > * De-emphasizes the FETCH_HEAD stuff by moving it to the end of the > DESCRIPTION section, This reads much better. Thanks. > > * States that remote-tracking branches are simply "updated", and hints > that playing with can control this. > > * Includes the "their histories" rephrasing. > > * Adds your peek-at-a-remote-branch example. > If you like this, feel free to sqush it into your 1/5. Will splice in as patch 1.5 instead ;-) -- 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] Makefile: don't hardcode HEAD in dist target
Dennis Kaarsemaker writes: > Instead of calling git-archive HEAD^{tree}, use $(GIT_VERSION)^{tree}. > This makes sure the archive name and contents are consistent, if HEAD > has moved, but GIT-VERSION-FILE hasn't been regenerated yet. > > Signed-off-by: Dennis Kaarsemaker > --- > I have a somewhat odd setup in which I share a .git between multiple > checkouts for automated builds. To minimize locking time for parallel > builds with different options, there's only a lock around checkout and > running git describe $commit > version, the builds themselves run in > parallel. > > This works just fine except during 'make dist', which is hardcoded to > package up HEAD, but that's not always the commit that is actually > checked out, another process may have checked out something else. > > I realize this setup is somewhat strange, but the only change necessary > to make this work is this one-line patch, so I hope that's acceptable. The "dist" target happens to do a clean checkout to a temporary directory with "git archive" and then muck with its contents before tarring up the result, so moving HEAD around may appear to work for this particular target, but htmldocs/manpages targets use what is in the current checkout of Documentation/ area. Turning the HEAD^{tree} into $(GIT_VERSION)^{tree} would make the inconsistency between the two worse, wouldn't it? If we were to change the "dist" rules, we may want to go in the opposite direction. Instead of using "git archive" to make a temporary copy of a directory from a commit, make such a temporary copy from the contents of the current working tree (or the index). And then tar-up a result after adding configure, version etc. to it. That would mean what will be in the tarball can be different from even HEAD, which is more consistent with the way how documentation tarballs are made. Alternatively, you can update the dist-doc rules to make a temporary copy of documentation area and generate the documentation tarballs out of a pristine source of a known version---which would also make these two consistent. I am not sure which one is more preferrable, though. Why are you sharing the .git/HEAD across multiple checkouts in the first place? If they are to build all different versions, surely these working trees are derived from different commits, no? Have you considered using contrib/workdir/git-new-workdir, perhaps? I dunno. > Makefile | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/Makefile b/Makefile > index a53f3a8..63d9bac 100644 > --- a/Makefile > +++ b/Makefile > @@ -2433,7 +2433,7 @@ git.spec: git.spec.in GIT-VERSION-FILE > GIT_TARNAME = git-$(GIT_VERSION) > dist: git.spec git-archive$(X) configure > ./git-archive --format=tar \ > - --prefix=$(GIT_TARNAME)/ HEAD^{tree} > $(GIT_TARNAME).tar > + --prefix=$(GIT_TARNAME)/ $(GIT_VERSION)^{tree} > > $(GIT_TARNAME).tar > @mkdir -p $(GIT_TARNAME) > @cp git.spec configure $(GIT_TARNAME) > @echo $(GIT_VERSION) > $(GIT_TARNAME)/version -- 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 0/9] replace signal() with sigaction()
Johannes Sixt writes: >> Jeremiah Mahler (9): >> compat/mingw.c: expand MinGW support for sigaction >> connect.c: replace signal() with sigaction() >> progress.c: replace signal() with sigaction() >> write_or_die.c: replace signal() with sigaction() >> daemon.c: replace signal() with sigaction() >> builtin/log.c: replace signal() with sigaction() >> builtin/merge-index.c: replace signal() with sigaction() >> builtin/verify-tag.c: replace signal() with sigaction() >> sigchain.c: replace signal() with sigaction() > > The series without patch 9/9 works on Windows so far. > > Without patch patch 9/9 and a more complete implementation of sigaction in > compat/mingw.c the series misses its goal. But even if you complete it, it > is IMHO only code churn without practical merits. Hmm, you sound a bit harsher than you usually do---although I sort of share with you the doubt on the practical merits. -- 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 4/4] contrib: add convert-grafts-to-replace-refs.sh
From: Eric Sunshine > On Sun, Jun 1, 2014 at 11:10 AM, Christian Couder > wrote: >> >> +test -f "$GRAFTS_FILE" || die "Could not find graft file: '$GRAFTS_FILE'" >> + >> +grep '^[^# ]' "$GRAFTS_FILE" | while read definition >> +do >> + test -n "$definition" && { >> + echo "Converting: $definition" >> + git replace --graft $definition || >> + die "Convertion failed for: $definition" > > s/Convertion/Conversion/ [1] > > [1]: > http://git.661346.n2.nabble.com/Re-PATCH-contrib-add-convert-grafts-to-replace-refs-sh-tp7611822.html Ooops, sorry I forgot this. >> + } >> +done >> + >> +mv "$GRAFTS_FILE" "$GRAFTS_FILE.bak" || >> + die "Could not mv '$GRAFTS_FILE' to '$GRAFTS_FILE.bak'" > > "Could not rename..." might be a bit more friendly to non-Unixy folk. Ok, I will use "rename". Thanks, Christian. -- 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] userdiff: support Java try keyword
Sup Yut Sum writes: > try keyword is enhanced in Java 7, see try-with-resources Statement > try (XX yy = new XX()) { > } catch (Exception e){ > } Sorry, but I do not see the connection between the proposed log message and what the patch does. The patch seems to tell me this: We did not know 'try' was a keyword to begin a block for java, whether that 'try' is a traditional 'try' or 'try' in 'try-with-resources'. With this patch we start recognising 'try' as a keyword. and nothing else, but the log message makes it sound as if the more prevalent use of try-with-resources in Java 7 makes it for some reason more important to recognise it as a keyword than it used to be---or is that what you meant to say? Puzzled... > Signed-off-by: Sup Yut Sum > --- > userdiff.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/userdiff.c b/userdiff.c > index 96eda6c..49e898b 100644 > --- a/userdiff.c > +++ b/userdiff.c > @@ -39,7 +39,7 @@ IPATTERN("fortran", > PATTERNS("html", "^[ \t]*(<[Hh][1-6][ \t].*>.*)$", >"[^<>= \t]+"), > PATTERNS("java", > - "!^[ \t]*(catch|do|for|if|instanceof|new|return|switch|throw|while)\n" > + "!^[ > \t]*(try|catch|do|for|if|instanceof|new|return|switch|throw|while)\n" >"^[ \t]*(([A-Za-z_][A-Za-z_0-9]*[ \t]+)+[A-Za-z_][A-Za-z_0-9]*[ > \t]*\\([^;]*)$", >/* -- */ >"[a-zA-Z_][a-zA-Z0-9_]*" -- 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 v2 1/2] config: Add hashtable for config parsing & retrieval
On 2014-06-02 16.47, Tanay Abhra wrote: [] Please see 3 minor remarks inline. > --- a/config.c > +++ b/config.c > @@ -9,6 +9,8 @@ > #include "exec_cmd.h" > #include "strbuf.h" > #include "quote.h" > +#include "hashmap.h" > +#include "string-list.h" > > struct config_source { > struct config_source *prev; > @@ -37,6 +39,112 @@ static struct config_source *cf; > > static int zlib_compression_seen; > > +static struct hashmap config_cache; > + > +struct config_cache_entry { > + struct hashmap_entry ent; > + char *key; > + struct string_list *value_list; > +}; > + > +static int hashmap_is_init = 0; we don't need the " = 0", as all static data is initialized to 0 (or NULL) > + > +static int config_cache_entry_cmp_icase(const struct config_cache_entry *e1, > + const struct config_cache_entry *e2, const > char* key) the * should be aligned to the variable "key": const char *key [] > +static void config_cache_set_value(const char *key, const char *value) > +{ > + struct config_cache_entry *e; > + > + e = config_cache_find_entry(key); > + if (!e) { > + e = malloc(sizeof(*e)); I think we need xmalloc() here (from wrapper.c) -- 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] Makefile: don't hardcode HEAD in dist target
On Mon, Jun 02, 2014 at 11:53:36AM -0700, Junio C Hamano wrote: > Dennis Kaarsemaker writes: > > > Instead of calling git-archive HEAD^{tree}, use $(GIT_VERSION)^{tree}. > > This makes sure the archive name and contents are consistent, if HEAD > > has moved, but GIT-VERSION-FILE hasn't been regenerated yet. > > > > Signed-off-by: Dennis Kaarsemaker > > --- > > I have a somewhat odd setup in which I share a .git between multiple > > checkouts for automated builds. To minimize locking time for parallel > > builds with different options, there's only a lock around checkout and > > running git describe $commit > version, the builds themselves run in > > parallel. > > > > This works just fine except during 'make dist', which is hardcoded to > > package up HEAD, but that's not always the commit that is actually > > checked out, another process may have checked out something else. > > > > I realize this setup is somewhat strange, but the only change necessary > > to make this work is this one-line patch, so I hope that's acceptable. > > The "dist" target happens to do a clean checkout to a temporary > directory with "git archive" and then muck with its contents before > tarring up the result, so moving HEAD around may appear to work for > this particular target, but htmldocs/manpages targets use what is in > the current checkout of Documentation/ area. Turning the HEAD^{tree} > into $(GIT_VERSION)^{tree} would make the inconsistency between the > two worse, wouldn't it? I'd say it would make the consistency better, because now both look at what is checked out instead of at HEAD. I agree that it's a game of whack-a-mole though and it's really easy to add another dependency on HEAD and GIT-VERSION-FILE agreeing with each other. > If we were to change the "dist" rules, we may want to go in the > opposite direction. Instead of using "git archive" to make a > temporary copy of a directory from a commit, make such a temporary > copy from the contents of the current working tree (or the index). > And then tar-up a result after adding configure, version etc. to it. > That would mean what will be in the tarball can be different from > even HEAD, which is more consistent with the way how documentation > tarballs are made. > > Alternatively, you can update the dist-doc rules to make a temporary > copy of documentation area and generate the documentation tarballs > out of a pristine source of a known version---which would also make > these two consistent. I am not sure which one is more preferrable, > though. > > Why are you sharing the .git/HEAD across multiple checkouts in the > first place? If they are to build all different versions, surely > these working trees are derived from different commits, no? I'm sharing all of .git using explicit GIT_DIR and GIT_WORK_TREE environment variables, sharing .git/HEAD comes with that. What I'm actually doing is a continuos integration setup that can run many actions at once in freshly checked-out work trees, but sharing .git to save space. What it specifically doing is running 'make test' for master, next and pu, and make dist for next. As long as I protect the 'git checkout $sha1' with a lock, that all works just fine. > Have you considered using contrib/workdir/git-new-workdir, perhaps? I have not, thanks for the pointer. That approach is definitely cleaner than what I am currently doing. > I dunno. With the hint above, I actually don't need this patch anymore. And if you're not convinced it's a good idea, it's probably better to drop it :) > > Makefile | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/Makefile b/Makefile > > index a53f3a8..63d9bac 100644 > > --- a/Makefile > > +++ b/Makefile > > @@ -2433,7 +2433,7 @@ git.spec: git.spec.in GIT-VERSION-FILE > > GIT_TARNAME = git-$(GIT_VERSION) > > dist: git.spec git-archive$(X) configure > > ./git-archive --format=tar \ > > - --prefix=$(GIT_TARNAME)/ HEAD^{tree} > $(GIT_TARNAME).tar > > + --prefix=$(GIT_TARNAME)/ $(GIT_VERSION)^{tree} > > > $(GIT_TARNAME).tar > > @mkdir -p $(GIT_TARNAME) > > @cp git.spec configure $(GIT_TARNAME) > > @echo $(GIT_VERSION) > $(GIT_TARNAME)/version -- Dennis Kaarsemaker http://twitter.com/seveas -- 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] mailinfo: use strcmp() for string comparison
On Sun, Jun 01, 2014 at 11:00:40AM +0200, René Scharfe wrote: > The array header is defined as: > > static const char *header[MAX_HDR_PARSED] = { >"From","Subject","Date", > }; > > When looking for the index of a specfic string in that array, simply > use strcmp() instead of memcmp(). This avoids running over the end of > the string (e.g. with memcmp("Subject", "From", 7)) and gets rid of > magic string length constants. > > Signed-off-by: Rene Scharfe Looks correct to me. > --- > This is a minimal fix. A good question, however, would be: Why do we > keep on looking up constant strings in a (short) constant string array > anyway? Yeah, this code is quite confusing. I suspect it would be more readable to unroll any loops over the header array into a series of function calls or even just cascading if/else. Some of the sites (e.g., check_header) already have a mix, like: for (i = 0; header[i]; i++) if (cmp_header(line, header[i])) ... do something for this header ... if (cmp_header(line, "some-other-header")) ... do something special for this header type ... else if (cmp_header(line, "another")) ... and something else again ... The looping is not really helping much there in the first place, since it is not dealing with half of the headers. And then adding on top that the loop has its own special cases found by comparing the string to "Subject", I think it would be simpler to just unroll it. That's just from a quick 5-minute read of the code, though. This isn't an area I'm very familiar with, so maybe the refactor would get ugly. -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: [PATCH] config: Add documentation for writing config files
Tanay Abhra writes: > Signed-off-by: Tanay Abhra > --- > Documentation/technical/api-config.txt | 31 ++- > 1 file changed, 30 insertions(+), 1 deletion(-) Even though the reason to replace a TODO with real stuff is rather straigthforward, the reader appriates a short commit message (ideally pointing to the commit introducing the TODO). My first thought reading the patch was "wtf, is that a patch in the middle of a series, where does this TODO come from" ;-). > diff --git a/Documentation/technical/api-config.txt > b/Documentation/technical/api-config.txt > index 230b3a0..df08385 100644 > --- a/Documentation/technical/api-config.txt > +++ b/Documentation/technical/api-config.txt > @@ -137,4 +137,33 @@ int read_file_with_include(const char *file, config_fn_t > fn, void *data) > Writing Config Files > > > -TODO > +Git gives multiple entry points in the Config API to write config values to > +files namely `git_config_set_in_file` and `git_config_set`, which write to > +a specific config file or to `.git/config` respectively. They both take a > +key/value pair as parameter. Sounds good. > +In the end they both all call `git_config_set_multivar_in_file` which takes > +four parameters: Do we really want to document this as part of the config API? i.e. does a normal user of the API want to know about this? My understanding is that git_config_set_multivar_in_file is essentially a private function, and then the best place to document is with comments near the function definition (it already has such comment). Your text is easier to understand and more detailed, but I would personnally prefer improving the in-code comment (possibly just leaving a mention of git_config_set_multivar_in_file and pointing the reader to the code for details). -- Matthieu Moy http://www-verimag.imag.fr/~moy/ -- 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] pack-objects: use free()+xcalloc() instead of xrealloc()+memset()
On Sun, Jun 01, 2014 at 01:07:21PM +0200, René Scharfe wrote: > Whenever the hash table becomes too small then its size is increased, > the original part (and the added space) is zerod out using memset(), > and the table is rebuilt from scratch. > > Simplify this proceess by returning the old memory using free() and > allocating the new buffer using xcalloc(), which already clears the > buffer for us. That way we avoid copying the old hash table contents > needlessly inside xrealloc(). > > While at it, use the first array member with sizeof instead of a > specific type. The old code used uint32_t and int, while index is > actually an array of int32_t. Their sizes are the same basically > everywhere, so it's not actually a problem, but the new code is > cleaner and doesn't have to be touched should the type be changed. > > Signed-off-by: Rene Scharfe Looks good to me. BTW, the code does git-blame to Vicent's 2834bc2 (which I also worked on), but actually originated in 7a979d9 (Thin pack - create packfile with missing delta base., 2006-02-19). Not that it matters, but I was just surprised since the code you are changing did not seem familiar to me. I guess there was just too much refactoring during the code movement for git-blame to pass along the blame in this case. -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: [PATCH] string-list.h: Add a value to string_list initializer lists
Tanay Abhra writes: > Add a NULL value in the end of STRING_LIST_INIT_NODUP and > STRING_LIST_DUP to initialize `compare_strings_fn`. Hmph. That is "what change is proposed", which we can read from the patch text itself below. We would want to see "why is this change a good thing" to justify it. As you mentioned later "In C, ...", there is nothing wrong in what we have (and we can not quite read what exactly you tried to do with uninitialized memory with these macros), so we need an excuse other than correctness to justify this change. Perhaps like this? STRING_LIST_INIT_{NODUP,DUP} initializers list values only for earlier structure members, relying on the usual convention in C that the omitted members are initailized to 0, i.e. the former is expanded to the latter: struct string_list l = STRING_LIST_INIT_DUP; struct string_list l = { NULL, 0, 0, 1 }; and the last member that is not mentioned (i.e. 'cmp') is initialized to NULL. While there is nothing wrong in this construct, spelling out all the values where the macros are defined will serve also as a documentation, so let's do so. > Signed-off-by: Tanay Abhra > --- > When I used a malloced string_list to play around with string-list API and > used the default init_list, it caused a seg fault. After an hour of debugging > I saw that comapre_strings_fn should be initialized to NULL. In C, even an > incomplete initialzer initializes every value to NULl or 0, so in normal > usage in the codebase this problem never occurs. Still it is better to be > thorough. > > string-list.h | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/string-list.h b/string-list.h > index de6769c..87ee419 100644 > --- a/string-list.h > +++ b/string-list.h > @@ -15,8 +15,8 @@ struct string_list { > compare_strings_fn cmp; /* NULL uses strcmp() */ > }; > > -#define STRING_LIST_INIT_NODUP { NULL, 0, 0, 0 } > -#define STRING_LIST_INIT_DUP { NULL, 0, 0, 1 } > +#define STRING_LIST_INIT_NODUP { NULL, 0, 0, 0, NULL } > +#define STRING_LIST_INIT_DUP { NULL, 0, 0, 1, NULL } > > void print_string_list(const struct string_list *p, const char *text); > void string_list_clear(struct string_list *list, int free_util); -- 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] environment: enable core.preloadindex by default
Am 02.06.2014 18:43, schrieb Steve Hoelzer: > There is consensus that the default should change because it will > benefit nearly all users (some just a little, but some a lot). > See [1] and replies. Git for Windows users may want to try core.fscache=true as well [1]. This eliminates the UAC penalties for repositories located on the Windows system drive [2]. [1] https://github.com/msysgit/git/pull/94 [2] https://code.google.com/p/msysgit/issues/detail?id=320 -- 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 alt-v2] Improve function dir.c:trim_trailing_spaces()
Pasha Bolokhov writes: > Discard the unnecessary 'nr_spaces' variable, remove 'strlen()' and > improve the 'if' structure. Switch to pointers instead of integers > > Slightly more rare occurrences of 'text \' with a backslash > in between spaces are handled correctly. Namely, the code in > 8ba87adad6 does not reset 'last_space' when a backslash is > encountered and the above line stays intact as a result. > Add a test at the end of t/t0008-ignores.sh to exhibit this behavior > > Signed-off-by: Pasha Bolokhov > --- > After correcting for the trailing backslash 'text\', a switch() > structure gives better readability than nested 'ifs', the way I see it. > Add a test to show that the trailing backslash 'text\' is handled > correctly > > dir.c | 35 --- > t/t0008-ignores.sh | 23 +++ > 2 files changed, 43 insertions(+), 15 deletions(-) > > diff --git a/dir.c b/dir.c > index eb6f581..06483d4 100644 > --- a/dir.c > +++ b/dir.c > @@ -508,21 +508,26 @@ void clear_exclude_list(struct exclude_list *el) > > static void trim_trailing_spaces(char *buf) > { > - int i, last_space = -1, nr_spaces, len = strlen(buf); > - for (i = 0; i < len; i++) > - if (buf[i] == '\\') > - i++; > - else if (buf[i] == ' ') { > - if (last_space == -1) { > - last_space = i; > - nr_spaces = 1; > - } else > - nr_spaces++; > - } else > - last_space = -1; > - > - if (last_space != -1 && last_space + nr_spaces == len) > - buf[last_space] = '\0'; > + char *p, *last_space = NULL; > + > + for (p = buf; *p; p++) > + switch (*p) { > + case ' ': > + if (!last_space) > + last_space = p; > + break; > + > + case '\\': > + p++; > + if (!*p) > + return; > + Write /* fallthru */ here. > + default: > + last_space = NULL; > + } > + > + if (last_space) > + *last_space = '\0'; > } I think the loop structure is a lot simpler to follow (with or without switch/case) with this change. Good. > int add_excludes_from_file_to_list(const char *fname, > diff --git a/t/t0008-ignores.sh b/t/t0008-ignores.sh > index 63beb99..4cea858 100755 > --- a/t/t0008-ignores.sh > +++ b/t/t0008-ignores.sh > @@ -806,4 +806,27 @@ test_expect_success !MINGW 'quoting allows trailing > whitespace' ' > test_cmp err.expect err > ' > > +test_expect_success NOT_MINGW,NOT_CYGWIN 'correct handling of backslashes' ' > + rm -rf whitespace && > + mkdir whitespace && > + >"whitespace/trailing 1 " && > + >"whitespace/trailing 2 " && > + >"whitespace/trailing 3 " && > + >"whitespace/trailing 4 \\ " && > + >"whitespace/trailing 5 \\ \\ " && > + >"whitespace/trailing 6 \\a\\" && Don't be cute and try to align with tabs. > + >whitespace/untracked && > + echo "whitespace/trailing 1 \\" >ignore && > + echo "whitespace/trailing 2 " >>ignore && > + echo "whitespace/trailing 3 " >>ignore && > + echo "whitespace/trailing 4 " >>ignore && > + echo "whitespace/trailing 5 " >>ignore && > + echo "whitespace/trailing 6 a" >>ignore && > + echo whitespace/untracked >expect && > + : >err.expect && Other file truncation is done with ">whitespace/filename" without an explicit ":" aka no-op command; I know this was copied from a previous test but perhaps we want to be consistent? > + git ls-files -o -X ignore whitespace >actual 2>err && > + test_cmp expect actual && > + test_cmp err.expect err > +' > + > test_done -- 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 0/9] replace signal() with sigaction()
On Mon, Jun 02, 2014 at 12:05:29PM -0700, Junio C Hamano wrote: > Johannes Sixt writes: > > >> Jeremiah Mahler (9): > >> compat/mingw.c: expand MinGW support for sigaction > >> connect.c: replace signal() with sigaction() > >> progress.c: replace signal() with sigaction() > >> write_or_die.c: replace signal() with sigaction() > >> daemon.c: replace signal() with sigaction() > >> builtin/log.c: replace signal() with sigaction() > >> builtin/merge-index.c: replace signal() with sigaction() > >> builtin/verify-tag.c: replace signal() with sigaction() > >> sigchain.c: replace signal() with sigaction() > > > > The series without patch 9/9 works on Windows so far. > > > > Without patch patch 9/9 and a more complete implementation of sigaction in > > compat/mingw.c the series misses its goal. But even if you complete it, it > > is IMHO only code churn without practical merits. > > Hmm, you sound a bit harsher than you usually do---although I > sort of share with you the doubt on the practical merits. > Alright, I'm dropping it. Too much work for no real gain other than some piece of mind. Thanks Johannes and Junio for your feedback. -- Jeremiah Mahler jmmah...@gmail.com http://github.com/jmahler -- 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] Makefile: don't hardcode HEAD in dist target
Dennis Kaarsemaker writes: > I'd say it would make the consistency better, because now both look at > what is checked out instead of at HEAD. The version with your patch does not even look at HEAD; it looks at whatever GIT_VERSION points at, which could be a very different version that does not have anything to do with what is checked out Can't GIT_VERSION come from ./version file, for example? -- 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] pack-objects: use free()+xcalloc() instead of xrealloc()+memset()
Jeff King writes: > On Sun, Jun 01, 2014 at 01:07:21PM +0200, René Scharfe wrote: > >> Whenever the hash table becomes too small then its size is increased, >> the original part (and the added space) is zerod out using memset(), >> and the table is rebuilt from scratch. >> >> Simplify this proceess by returning the old memory using free() and >> allocating the new buffer using xcalloc(), which already clears the >> buffer for us. That way we avoid copying the old hash table contents >> needlessly inside xrealloc(). >> >> While at it, use the first array member with sizeof instead of a >> specific type. The old code used uint32_t and int, while index is >> actually an array of int32_t. Their sizes are the same basically >> everywhere, so it's not actually a problem, but the new code is >> cleaner and doesn't have to be touched should the type be changed. >> >> Signed-off-by: Rene Scharfe > > Looks good to me. > > BTW, the code does git-blame to Vicent's 2834bc2 (which I also worked > on), but actually originated in 7a979d9 (Thin pack - create packfile > with missing delta base., 2006-02-19). Not that it matters, but I was > just surprised since the code you are changing did not seem familiar to > me. I guess there was just too much refactoring during the code movement > for git-blame to pass along the blame in this case. Without -M, "too much refactoring" for git-blame may just be moving a function to a different place in the same file. -- David Kastrup -- 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
2.0.0 regression? request pull does not seem to find head
Looks like pull requests no longer work for me on linux. Some other trees (non-linux) work fine but I didn't yet check whether it's the local or the remote tree that's at issue. Or maybe it's a configuration change that I missed? Note: I have [push] default = matching configured in .gitconfig. [mst@robin linux]$ git request-pull net-next/master git://git.kernel.org/pub/scm/linux/kernel/git/mst/vhost.git net-next warn: No match for commit 2ae76693b8bcabf370b981cd00c36cd41d33fabc found at git://git.kernel.org/pub/scm/linux/kernel/git/mst/vhost.git warn: Are you sure you pushed 'net-next' there? The following changes since commit 96b2e73c5471542cb9c622c4360716684f8797ed: Revert "net/mlx4_en: Use affinity hint" (2014-06-02 00:18:48 -0700) are available in the git repository at: git://git.kernel.org/pub/scm/linux/kernel/git/mst/vhost.git net-next for you to fetch changes up to 2ae76693b8bcabf370b981cd00c36cd41d33fabc: vhost: replace rcu with mutex (2014-06-02 23:47:59 +0300) Michael S. Tsirkin (2): vhost-net: extend device allocation to vmalloc vhost: replace rcu with mutex drivers/vhost/net.c | 23 ++- drivers/vhost/vhost.c | 10 +- 2 files changed, 27 insertions(+), 6 deletions(-) [mst@robin linux]$ git request-pull net-next/master git://git.kernel.org/pub/scm/linux/kernel/git/mst/vhost.git net-next warn: No match for commit 2ae76693b8bcabf370b981cd00c36cd41d33fabc found at git://git.kernel.org/pub/scm/linux/kernel/git/mst/vhost.git warn: Are you sure you pushed 'net-next' there? The following changes since commit 96b2e73c5471542cb9c622c4360716684f8797ed: Revert "net/mlx4_en: Use affinity hint" (2014-06-02 00:18:48 -0700) are available in the git repository at: git://git.kernel.org/pub/scm/linux/kernel/git/mst/vhost.git net-next for you to fetch changes up to 2ae76693b8bcabf370b981cd00c36cd41d33fabc: vhost: replace rcu with mutex (2014-06-02 23:47:59 +0300) Michael S. Tsirkin (2): vhost-net: extend device allocation to vmalloc vhost: replace rcu with mutex drivers/vhost/net.c | 23 ++- drivers/vhost/vhost.c | 10 +- 2 files changed, 27 insertions(+), 6 deletions(-) [mst@robin linux]$ git --version git version 2.0.0.538.gb6dd70f [mst@robin linux]$ /usr/bin/git request-pull net-next/master git://git.kernel.org/pub/scm/linux/kernel/git/mst/vhost.git net-next The following changes since commit 96b2e73c5471542cb9c622c4360716684f8797ed: Revert "net/mlx4_en: Use affinity hint" (2014-06-02 00:18:48 -0700) are available in the git repository at: git://git.kernel.org/pub/scm/linux/kernel/git/mst/vhost.git vhost-next for you to fetch changes up to 2ae76693b8bcabf370b981cd00c36cd41d33fabc: vhost: replace rcu with mutex (2014-06-02 23:47:59 +0300) Michael S. Tsirkin (2): vhost-net: extend device allocation to vmalloc vhost: replace rcu with mutex drivers/vhost/net.c | 23 ++- drivers/vhost/vhost.c | 10 +- 2 files changed, 27 insertions(+), 6 deletions(-) [mst@robin linux]$ /usr/bin/git --version git version 1.8.3.1 -- MST -- 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] refs.c: change read_ref_at to use the reflog iterators
Ronnie Sahlberg writes: > read_ref_at has its own parsing of the reflog file for no really good reason > so lets change this to use the existing reflog iterators. This removes one > instance where we manually unmarshall the reflog file format. > > Log messages for errors are changed slightly. We no longer print the file > name for the reflog, instead we refer to it as 'Log for ref '. > This might be a minor useability regression, but I don't really think so, > since > experienced users would know where the log is anyway and inexperienced users > would not know what to do about/how to repair 'Log ... has gap ...' anyway. > > Adapt the t1400 test to handle the change in log messages. > > Signed-off-by: Ronnie Sahlberg > --- > refs.c| 202 > ++ > t/t1400-update-ref.sh | 4 +- > 2 files changed, 107 insertions(+), 99 deletions(-) > > diff --git a/refs.c b/refs.c > index 6898263..b45bb2f 100644 > --- a/refs.c > +++ b/refs.c > @@ -2936,109 +2936,117 @@ static char *ref_msg(const char *line, const char > *endp) > return xmemdupz(line, ep - line); > } If I am not mistaken, this function will become unused with this rewrite. Let's drop it and justify it in the log message. > +struct read_ref_at_cb { > + const char *refname; > + unsigned long at_time; > + int cnt; > + int reccnt; > + unsigned char *sha1; > + int found_it; > + > + char osha1[20]; > + char nsha1[20]; These should be unsigned char, shouldn't they? > + for_each_reflog_ent_reverse(refname, read_ref_at_ent, &cb); > + > + if (!cb.reccnt) > + die("Log for %s is empty.", refname); > + if (cb.found_it) > + return 0; > + > + for_each_reflog_ent(refname, read_ref_at_ent_oldest, &cb); Hmph. We have already scanned the same reflog in the backward direction in full. Do we need to make another call just to pick up the entry at the beginning? Is this because the callback is not told that it is fed the last entry (in other words, if there is some clue that this is the last call from for-each-reflog-ent-reverse to the callback, the callback could record the value it received in its cb-data)? -- 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: 2.0.0 regression? request pull does not seem to find head
"Michael S. Tsirkin" writes: > Looks like pull requests no longer work for me on linux. Wasn't "does not seem to find head" was very much deliberate? Linus's patch wanted the users to explicitly tell the tool, without tool trying to be too helpful and risking to guess incorrectly. > Some other trees (non-linux) work fine but I didn't yet > check whether it's the local or the remote tree that's > at issue. > > Or maybe it's a configuration change that I missed? > > Note: I have > [push] > default = matching > configured in .gitconfig. This should not affect anything in request-pull, I think. -- 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: Reset by checkout?
Atsushi Nakagawa writes: > One of the more underrepresented command I use in git use on a regular > basis is this "reset by checkout". It's what's currently achieved by > this convoluted expression: > > `git checkout -B ` > > This is such an useful notion that I can fathom why there isn't a better, > first-tier, alternative. Hmph. checkout *is* the first-tier way to do this. Why do you even want to do it via "reset"? Is it because you learned "reset" first and then learned how "checkout" with various modes all do useful things? -- 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: Reset by checkout?
Junio C Hamano writes: > Atsushi Nakagawa writes: > >> One of the more underrepresented command I use in git use on a regular >> basis is this "reset by checkout". It's what's currently achieved by >> this convoluted expression: >> >> `git checkout -B ` >> >> This is such an useful notion that I can fathom why there isn't a better, >> first-tier, alternative. > > Hmph. checkout *is* the first-tier way to do this. Why do you even > want to do it via "reset"? Is it because you learned "reset" first > and then learned how "checkout" with various modes all do useful > things? Ahh, the "branch to be checked out" being the "current" branch is indeed strange. That is what "reset --keep" was invented for. I use "git checkout -B " all the time, and somehow I thought that was what you were talking about. Sorry for the noise. -- 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: 2.0.0 regression? request pull does not seem to find head
On Mon, Jun 02, 2014 at 02:27:25PM -0700, Junio C Hamano wrote: > "Michael S. Tsirkin" writes: > > > Looks like pull requests no longer work for me on linux. > > Wasn't "does not seem to find head" was very much deliberate? I'm sorry I don't understand what you are asking here. Same thing happens if I use a branch name explicitly, not just HEAD. > Linus's patch wanted the users to explicitly tell the tool, without > tool trying to be too helpful and risking to guess incorrectly. So this is an intentional behaviour change? Which patch do you refer to? > > Some other trees (non-linux) work fine but I didn't yet > > check whether it's the local or the remote tree that's > > at issue. > > > > Or maybe it's a configuration change that I missed? > > > > Note: I have > > [push] > > default = matching > > configured in .gitconfig. > > This should not affect anything in request-pull, I think. I just thought I'd mention this. push behaviour is the only big incompatible change I'm aware of between 1.8 which works for me and 2.0 which doesn't. -- MST -- 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: 2.0.0 regression? request pull does not seem to find head
On Mon, Jun 2, 2014 at 2:01 PM, Michael S. Tsirkin wrote: > > [mst@robin linux]$ git request-pull net-next/master > git://git.kernel.org/pub/scm/linux/kernel/git/mst/vhost.git net-next > warn: No match for commit 2ae76693b8bcabf370b981cd00c36cd41d33fabc found at > git://git.kernel.org/pub/scm/linux/kernel/git/mst/vhost.git > warn: Are you sure you pushed 'net-next' there? git request-pull is clearly correct. There is no "net-next" in that public repository. It *used* to be that request-pull then magically tried to fix it up for you, which in turn resulted in the guess not being right, like pointing to the wrong branch that just happened to have the same SHA1, or pointing to a branch when it _should_ have pointed to a tag. Now, if you pushed your local "net-next" branch to another branch name (I can find a branch name called "vhost-next" at that repository, then you can *tell* git that, using the same syntax you must have used for the push. So something like git request-pull net-next/master git://git.kernel.org/pub/scm/linux/kernel/git/mst/vhost.git net-next:vhost-next should work so that git doesn't end up having to guess (and potentially guessing wrong). But it may actually be a simpler driver error, and you wanted to use "vhost-next", and that "net-next" was actually incorrect? Linus -- 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: Reset by checkout?
Kevin Bracey writes: > Maybe something like this: I like the overall direction to re-organize the description by operations, but the new description seem to introduce a bit of new confusion. > "All modes move the current branch pointer so that HEAD now points to > the specified commit. ORIG_HEAD is set to the original HEAD > location. The modes differ in what happens to the contents of > ORIG_HEAD, that are no longer on the reset branch; and also what > happens to your not-yet-committed changes. > > --soft > Retains the contents of ORIG_HEAD in the index+work area, > leaving the difference as "changes to be committed". This (and everything that talks about ORIG_HEAD) asks the user to think of the working tree state as a combination of "the state the commit you were on represents" plus "the changes you made relative to it". Given that everything Git records is a whole-tree snapshot, "state" (not "changes"), and that is how tutorials teach Git, I wonder if the "what is done to ORIG_HEAD and changes" gets the user into right mindset to understand various modes of operations. And with that "ORIG_HEAD and changes" mindset, a --soft reset becomes very hard to explain. "ORIG_HEAD and changes (you had before you issued the 'reset --soft' command)" are left in the index/work, "HEAD" becomes the named commit, "changes from that updated HEAD" becomes the original changes (you had since ORIG_HEAD) mixed with the differences between ORIG_HEAD and HEAD. If you explain this in terms of "state", a --soft reset will keep the state of the index and the working tree as-is and changes the HEAD pointer to point at a different commit. > "git reset --soft HEAD~1" > would be the first step if you want to remove the last commit, but > intend to recommit most or all of its changes. > > "git status" after reset --soft shows: > > To be committed: >Changes in ORIG_HEAD relative to HEAD >(+Any initial staged changes) There would be overlapping parts of "Any initial staged changes" and "Changes in ORIG_HEAD relative to HEAD". They may be mixed, they may be partly reverted, or they may totally cancel out, depending on the changes the user made since starting to work on ORIG_HEAD. > Not staged: >(Any initial unstaged changes) > > --mixed (default) > Retains the contents of ORIG_HEAD in the work area, leaving the > difference as unstaged changes. I am confused by the above the same way. If the operation "retains the contents of ORIG_HEAD" in the working tree, would that mean the edit I made is somehow reverted? No, because you say "leaving the difference ...", but then the operation is not really retaining the contents of ORIG_HEAD. It is leaving the state I had in my working tree as-is, regardless of ORIG_HEAD and/or HEAD that is updated. Not that I can think of a better way to update these descriptions, and not that I am opposing to update these descriptions to make it easier for new people to learn, but I am not sure if these "treat ORIG_HEAD and the changes since that commit as separate entities" is a good approach to do so. Somewhat frustrated, not by your patch but by being unable to suggest a better way X-<. -- 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] pack-objects: use free()+xcalloc() instead of xrealloc()+memset()
On Mon, Jun 02, 2014 at 10:40:44PM +0200, David Kastrup wrote: > > BTW, the code does git-blame to Vicent's 2834bc2 (which I also worked > > on), but actually originated in 7a979d9 (Thin pack - create packfile > > with missing delta base., 2006-02-19). Not that it matters, but I was > > just surprised since the code you are changing did not seem familiar to > > me. I guess there was just too much refactoring during the code movement > > for git-blame to pass along the blame in this case. > > Without -M, "too much refactoring" for git-blame may just be moving a > function to a different place in the same file. I tried "git blame -M -C -C -C pack-objects.c" but couldn't get anything but the whole thing blamed to 2834bc2. -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: 2.0.0 regression? request pull does not seem to find head
Linus Torvalds linux-foundation.org> writes: > > On Mon, Jun 2, 2014 at 2:01 PM, Michael S. Tsirkin redhat.com> wrote: > > > > [mst robin linux]$ git request-pull net-next/master > git://git.kernel.org/pub/scm/linux/kernel/git/mst/vhost.git net-next > > warn: No match for commit 2ae76693b8bcabf370b981cd00c36cd41d33fabc found at git://git.kernel.org/pub/scm/linux/kernel/git/mst/vhost.git > > warn: Are you sure you pushed 'net-next' there? > > git request-pull is clearly correct. There is no "net-next" in that > public repository. I am seeing something similar: $ git request-pull master origin > /dev/null warn: No match for commit 64ea29197d5e13772b1f7b6c24feaa79cc97d997 found at origin warn: Are you sure you pushed 'HEAD' there? but I pushed 64ea29197d5e13772b1f7b6c24feaa79cc97d997: $ git show-ref bug_fix/init_report 64ea29197d5e13772b1f7b6c24feaa79cc97d997 refs/heads/bug_fix/init_report 64ea29197d5e13772b1f7b6c24feaa79cc97d997 refs/remotes/origin/bug_fix/init_report The warning goes away if I give an explicit end commit. Should the default value for $remote in the call to $find_matching_ref be $head rather than HEAD (and similarly for the warning message)? --James -- 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 alt-v3] Improve function dir.c:trim_trailing_spaces()
Discard the unnecessary 'nr_spaces' variable, remove 'strlen()' and improve the 'if' structure. Switch to pointers instead of integers Slightly more rare occurrences of 'text \' with a backslash in between spaces are handled correctly. Namely, the code in 8ba87adad6 does not reset 'last_space' when a backslash is encountered and the above line stays intact as a result. Add a test at the end of t/t0008-ignores.sh to exhibit this behavior Signed-off-by: Pasha Bolokhov --- Add /* fallthrough */ comment to switch(), remove TABs in test, and remove ":" command in file truncation in the test dir.c | 36 +--- t/t0008-ignores.sh | 23 +++ 2 files changed, 44 insertions(+), 15 deletions(-) diff --git a/dir.c b/dir.c index eb6f581..c7dc846 100644 --- a/dir.c +++ b/dir.c @@ -508,21 +508,27 @@ void clear_exclude_list(struct exclude_list *el) static void trim_trailing_spaces(char *buf) { - int i, last_space = -1, nr_spaces, len = strlen(buf); - for (i = 0; i < len; i++) - if (buf[i] == '\\') - i++; - else if (buf[i] == ' ') { - if (last_space == -1) { - last_space = i; - nr_spaces = 1; - } else - nr_spaces++; - } else - last_space = -1; - - if (last_space != -1 && last_space + nr_spaces == len) - buf[last_space] = '\0'; + char *p, *last_space = NULL; + + for (p = buf; *p; p++) + switch (*p) { + case ' ': + if (!last_space) + last_space = p; + break; + + case '\\': + p++; + if (!*p) + return; + /* fallthrough */ + + default: + last_space = NULL; + } + + if (last_space) + *last_space = '\0'; } int add_excludes_from_file_to_list(const char *fname, diff --git a/t/t0008-ignores.sh b/t/t0008-ignores.sh index 63beb99..5ef5ad3 100755 --- a/t/t0008-ignores.sh +++ b/t/t0008-ignores.sh @@ -806,4 +806,27 @@ test_expect_success !MINGW 'quoting allows trailing whitespace' ' test_cmp err.expect err ' +test_expect_success NOT_MINGW,NOT_CYGWIN 'correct handling of backslashes' ' + rm -rf whitespace && + mkdir whitespace && + >"whitespace/trailing 1 " && + >"whitespace/trailing 2 " && + >"whitespace/trailing 3 " && + >"whitespace/trailing 4 \\ " && + >"whitespace/trailing 5 \\ \\ " && + >"whitespace/trailing 6 \\a\\" && + >whitespace/untracked && + echo "whitespace/trailing 1 \\" >ignore && + echo "whitespace/trailing 2 " >>ignore && + echo "whitespace/trailing 3 " >>ignore && + echo "whitespace/trailing 4 " >>ignore && + echo "whitespace/trailing 5 " >>ignore && + echo "whitespace/trailing 6 a" >>ignore && + echo whitespace/untracked >expect && + >err.expect && + git ls-files -o -X ignore whitespace >actual 2>err && + test_cmp expect actual && + test_cmp err.expect err +' + test_done -- 1.9.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: [PATCH alt-v3] Improve function dir.c:trim_trailing_spaces()
Pasha Bolokhov writes: > Discard the unnecessary 'nr_spaces' variable, remove 'strlen()' and > improve the 'if' structure. Switch to pointers instead of integers > > Slightly more rare occurrences of 'text \' with a backslash > in between spaces are handled correctly. Namely, the code in > 8ba87adad6 does not reset 'last_space' when a backslash is > encountered and the above line stays intact as a result. > Add a test at the end of t/t0008-ignores.sh to exhibit this behavior > > Signed-off-by: Pasha Bolokhov > --- > Add /* fallthrough */ comment to switch(), remove TABs in test, > and remove ":" command in file truncation in the test 8ba87adad6 does not seem to do anything to do with this change, though. Tentatively I've queued the following (but not merged to anywhere nor pushed out). Thanks. -- >8 -- From: Pasha Bolokhov Date: Mon, 2 Jun 2014 15:36:56 -0700 Subject: [PATCH] dir.c:trim_trailing_spaces(): fix for " \ " sequence Discard the unnecessary 'nr_spaces' variable, remove 'strlen()' and improve the 'if' structure. Switch to pointers instead of integers to control the loop. Slightly more rare occurrences of 'text \' with a backslash in between spaces are handled correctly. Namely, the code in 7e2e4b37 (dir: ignore trailing spaces in exclude patterns, 2014-02-09) does not reset 'last_space' when a backslash is encountered and the above line stays intact as a result. Add a test at the end of t/t0008-ignores.sh to exhibit this behavior. Signed-off-by: Pasha Bolokhov Signed-off-by: Junio C Hamano --- dir.c | 34 +++--- t/t0008-ignores.sh | 23 +++ 2 files changed, 42 insertions(+), 15 deletions(-) diff --git a/dir.c b/dir.c index eb6f581..797805d 100644 --- a/dir.c +++ b/dir.c @@ -508,21 +508,25 @@ void clear_exclude_list(struct exclude_list *el) static void trim_trailing_spaces(char *buf) { - int i, last_space = -1, nr_spaces, len = strlen(buf); - for (i = 0; i < len; i++) - if (buf[i] == '\\') - i++; - else if (buf[i] == ' ') { - if (last_space == -1) { - last_space = i; - nr_spaces = 1; - } else - nr_spaces++; - } else - last_space = -1; - - if (last_space != -1 && last_space + nr_spaces == len) - buf[last_space] = '\0'; + char *p, *last_space = NULL; + + for (p = buf; *p; p++) + switch (*p) { + case ' ': + if (!last_space) + last_space = p; + break; + case '\\': + p++; + if (!*p) + return; + /* fallthrough */ + default: + last_space = NULL; + } + + if (last_space) + *last_space = '\0'; } int add_excludes_from_file_to_list(const char *fname, diff --git a/t/t0008-ignores.sh b/t/t0008-ignores.sh index 63beb99..5ef5ad3 100755 --- a/t/t0008-ignores.sh +++ b/t/t0008-ignores.sh @@ -806,4 +806,27 @@ test_expect_success !MINGW 'quoting allows trailing whitespace' ' test_cmp err.expect err ' +test_expect_success NOT_MINGW,NOT_CYGWIN 'correct handling of backslashes' ' + rm -rf whitespace && + mkdir whitespace && + >"whitespace/trailing 1 " && + >"whitespace/trailing 2 " && + >"whitespace/trailing 3 " && + >"whitespace/trailing 4 \\ " && + >"whitespace/trailing 5 \\ \\ " && + >"whitespace/trailing 6 \\a\\" && + >whitespace/untracked && + echo "whitespace/trailing 1 \\" >ignore && + echo "whitespace/trailing 2 " >>ignore && + echo "whitespace/trailing 3 " >>ignore && + echo "whitespace/trailing 4 " >>ignore && + echo "whitespace/trailing 5 " >>ignore && + echo "whitespace/trailing 6 a" >>ignore && + echo whitespace/untracked >expect && + >err.expect && + git ls-files -o -X ignore whitespace >actual 2>err && + test_cmp expect actual && + test_cmp err.expect err +' + test_done -- 2.0.0-511-g1433423 -- 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] pack-objects: use free()+xcalloc() instead of xrealloc()+memset()
Jeff King writes: > On Mon, Jun 02, 2014 at 10:40:44PM +0200, David Kastrup wrote: > >> > BTW, the code does git-blame to Vicent's 2834bc2 (which I also worked >> > on), but actually originated in 7a979d9 (Thin pack - create packfile >> > with missing delta base., 2006-02-19). Not that it matters, but I was >> > just surprised since the code you are changing did not seem familiar to >> > me. I guess there was just too much refactoring during the code movement >> > for git-blame to pass along the blame in this case. >> >> Without -M, "too much refactoring" for git-blame may just be moving a >> function to a different place in the same file. > > I tried "git blame -M -C -C -C pack-objects.c" but couldn't get anything > but the whole thing blamed to 2834bc2. Are you two being a bit too unreasonable, or trying to be fanciful and funny and I am not getting the humor? Here is the relevant part of what 2834bc27 (pack-objects: refactor the packing list, 2013-10-24) removes from builtin/pack-objects.c: - object_ix = xrealloc(object_ix, sizeof(int) * object_ix_hashsz); - memset(object_ix, 0, sizeof(int) * object_ix_hashsz); And here is how the same rehash is done in pack-objects.c at the toplevel in the new code: + pdata->index = xrealloc(pdata->index, sizeof(uint32_t) * pdata->index_size); + memset(pdata->index, 0, sizeof(int) * pdata->index_size); Surely, the code structure may be similar, but the similarity ends there. These lines are not equivalent even under the "-w" option. -- 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] sideband.c: Get rid of ANSI sequences for non-terminal shell
From: Michael Naumov Diagnostic messages received on the sideband #2 from the server side are sent to the standard error with ANSI terminal control sequence "\033[K" that erases to the end of line appended at the end of each line. However, some programs (e.g. GitExtensions for Windows) read and interpret and/or show the message without understanding the terminal control sequences, resulting them to be shown to their end users. To help these programs, squelch the control sequence when the standard error stream is not being sent to a tty. NOTE: I considered to cover the case that a pager has already been started. But decided that is probably not worth worrying about here, though, as we shouldn't be using a pager for commands that do network communications (and if we do, omitting the magic line-clearing signal is probably a sane thing to do). Signed-off-by: Michael Naumov Thanks-to: Erik Faye-Lund Thanks-to: Jeff King --- sideband.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sideband.c b/sideband.c index d1125f5..7f9dc22 100644 --- a/sideband.c +++ b/sideband.c @@ -30,7 +30,7 @@ int recv_sideband(const char *me, int in_stream, int out) memcpy(buf, PREFIX, pf); term = getenv("TERM"); - if (term && strcmp(term, "dumb")) + if (isatty(2) && term && strcmp(term, "dumb")) suffix = ANSI_SUFFIX; else suffix = DUMB_SUFFIX; -- 1.8.3.msysgit.0 P.S. I gave up trying to send this letter from gmail, it eats my tab character P.S 2. Sorry, my tab character has been eaten again! P.S 3. Wrapped the letter lines to fit on the terminal P.S 4. GitExtensions tried to use TERM=dumb but this caused process leakage for diff tools. See https://github.com/gitextensions/gitextensions/issues/1092 Regards, Michael -- 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: [ANNOUNCE] Git v2.0.0
On Wed, 28 May 2014 15:31:13 -0700 Junio C Hamano wrote: > The latest feature release Git v2.0.0 is now available at the > usual places. > > "git request-pull" lost a few "heuristics" that often led to mistakes. > I've installed git 2.0.0 and now when I git request-pull master git://neil.brown.name/md after tagging the current commit as "md/3.15-fixes" and pushing out the tag, I get warn: No match for commit 2ac295a544dcae9299cba13ce250419117ae7fd1 found at git://neil.brown.name/md warn: Are you sure you pushed 'HEAD' there? Yet git ls-remote git://neil.brown.name/md | grep 2ac29 shows 2ac295a544dcae9299cba13ce250419117ae7fd1refs/tags/md/3.15-fixes^{} Which seems clear and unambiguous. Does this mean that the 'end' arg to 'git request-pull' is no longer optional (i.e. the man page is wrong), or that too many heuristics were removed? Looking through the change log a bit, it seems that if the 'end' arg is omitted, then the current 'branch' name is used and must match the same name at the git URL. Could it also check the current 'tag' name? As we are encouraged to used signed tags, this seems sensible. In fact, I would suggest checking for a tag first, and only considering the branch name if there is no tag on the current commit. Thanks, NeilBrown signature.asc Description: PGP signature
Re: [ANNOUNCE] Git v2.0.0
On Mon, Jun 2, 2014 at 7:08 PM, NeilBrown wrote: > > git request-pull master git://neil.brown.name/md > > after tagging the current commit as "md/3.15-fixes" and pushing out the tag You should *tell* "git request-pull" what you're actually requesting to pull. The old "let's guess based on the commit at the top of your tree" may have worked for you (because you only ever had one matching thing), but it did not work in general. So the new "git request-pull" does not guess. If you want to request a tag to be pulled, you name it. Because if you don't name it, it now defaults to HEAD (like all other git log commands) rather than guessing. So please write it as git request-pull master git://neil.brown.name/md md/3.15-fixes I guess the man-page should be made more explicit about this too. Linus -- 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: [BUG REPORT] Git log pretty date
On Fri, May 30, 2014 at 09:08:57AM +0100, Rodrigo Fernandes wrote: > Do you have any idea how does github understand that is a bug and > fixes it automatically? > (I'm saying this because on Github the date is correct). I looked into this. The dates you see on GitHub's web UI are actually parsed by Rugged/libgit2. The libgit2 parser is slightly more forgiving in this instance; if it sees a broken timezone, it will leave the timestamp intact, and only omit the timezone. Whereas git says "no, it's broken, and the timestamp cannot be trusted". I think both are equally valid strategies, and I do not even think it is a problem that they diverge between the two implementations. I'd be OK with a patch to make git handle errors in each independently, assuming it is not too invasive. -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: [PATCH] environment: enable core.preloadindex by default
On Mon, Jun 2, 2014 at 3:01 PM, Karsten Blees wrote: > Git for Windows users may want to try core.fscache=true as well [1]. This > eliminates the UAC penalties for repositories located on the Windows system > drive [2]. > > [1] https://github.com/msysgit/git/pull/94 > [2] https://code.google.com/p/msysgit/issues/detail?id=320 Thanks for the tip! I didn't know about fscache, but I'll definitely give it a try. Is there a reason it is not turned on by default in Git for Windows? Steve -- 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: [BUG REPORT] Git log pretty date
On Mon, Jun 02, 2014 at 11:46:19PM -0400, Jeff King wrote: > On Fri, May 30, 2014 at 09:08:57AM +0100, Rodrigo Fernandes wrote: > > > Do you have any idea how does github understand that is a bug and > > fixes it automatically? > > (I'm saying this because on Github the date is correct). > > I looked into this. The dates you see on GitHub's web UI are actually > parsed by Rugged/libgit2. The libgit2 parser is slightly more forgiving > in this instance; if it sees a broken timezone, it will leave the > timestamp intact, and only omit the timezone. Whereas git says "no, it's > broken, and the timestamp cannot be trusted". > > I think both are equally valid strategies, and I do not even think it is > a problem that they diverge between the two implementations. I'd be OK > with a patch to make git handle errors in each independently, assuming > it is not too invasive. I think what libgit2 does is more wrong than what git does. It displays the timestamp subtly wrong (off by 7 hours) instead of making it completely clear that the timestamp is bogus. -- Dennis Kaarsemaker http://twitter.com/seveas -- 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] pack-objects: use free()+xcalloc() instead of xrealloc()+memset()
On Mon, Jun 02, 2014 at 04:09:12PM -0700, Junio C Hamano wrote: > > I tried "git blame -M -C -C -C pack-objects.c" but couldn't get anything > > but the whole thing blamed to 2834bc2. > > Are you two being a bit too unreasonable, or trying to be fanciful > and funny and I am not getting the humor? We are being too unreasonable. > Here is the relevant part of what 2834bc27 (pack-objects: refactor > the packing list, 2013-10-24) removes from builtin/pack-objects.c: > > - object_ix = xrealloc(object_ix, sizeof(int) * object_ix_hashsz); > - memset(object_ix, 0, sizeof(int) * object_ix_hashsz); > > And here is how the same rehash is done in pack-objects.c at the > toplevel in the new code: > > + pdata->index = xrealloc(pdata->index, sizeof(uint32_t) * > pdata->index_size); > + memset(pdata->index, 0, sizeof(int) * pdata->index_size); > > Surely, the code structure may be similar, but the similarity ends > there. These lines are not equivalent even under the "-w" option. Yes, I did not expect these particular lines to get blamed, but I thought some of the surrounding function would (which could lead to a parent-blame to find the true origin). Skimming the diff, it looked like some of them made it through unscathed. But they didn't. Running: git show 2834bc2 | perl -lne ' /^-(.*)/ and $del{$1}++; print "$.: $_" if /^\+(.*)/ && $del{$1}; ' shows that there are only a handful of interesting lines that survived completely intact, and typically not more than one line in a row. The big exceptions are the bits that made it into pack-objects.h, and a "git blame" there does find the code movement. So I think everything is operating as expected. -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: [BUG REPORT] Git log pretty date
On Tue, Jun 03, 2014 at 08:23:02AM +0200, Dennis Kaarsemaker wrote: > On Mon, Jun 02, 2014 at 11:46:19PM -0400, Jeff King wrote: > > On Fri, May 30, 2014 at 09:08:57AM +0100, Rodrigo Fernandes wrote: > > > > > Do you have any idea how does github understand that is a bug and > > > fixes it automatically? > > > (I'm saying this because on Github the date is correct). > > > > I looked into this. The dates you see on GitHub's web UI are actually > > parsed by Rugged/libgit2. The libgit2 parser is slightly more forgiving > > in this instance; if it sees a broken timezone, it will leave the > > timestamp intact, and only omit the timezone. Whereas git says "no, it's > > broken, and the timestamp cannot be trusted". > > > > I think both are equally valid strategies, and I do not even think it is > > a problem that they diverge between the two implementations. I'd be OK > > with a patch to make git handle errors in each independently, assuming > > it is not too invasive. > > I think what libgit2 does is more wrong than what git does. It displays > the timestamp subtly wrong (off by 7 hours) instead of making it > completely clear that the timestamp is bogus. I'm not sure what you mean. The timestamp is in UTC seconds-since-epoch, and does not depend on the timezone. The timezone only indicates the author's local time when the commit was made. Whether the latter is relevant depends on the date format you are showing (i.e., if you are showing it in the author's timezone, it matters; for --date=local or --date=relative, it would not). So I do not think libgit2 is at fault for parsing them separately; it does not know how the result will be presented. What GitHub shows is wrong, as it tries to put the timestamp into the author's timezone (which it doesn't know). But in practice it turns out be more useful, because we show relative dates for recent things (so "8 hours ago" is actually accurate and does not care about the timestamp). For distant things, time zone effects become less important (we show "Oct 29, 2010"; it may have actually been Oct 30 or Oct 28 in the author's zone, but it's really not that important anymore). Those comments are just on the two strategies. As far as implementations, it looks like libgit2 somehow parses this commit as +0700, which is odd. I'd expect it to fall back to +. I didn't dig further. -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