Re: Question on for-each-ref and --contains
St. Blind st.s.blind at gmail.com writes: The git-branch --contains and --merged are not very handy too, because the output is not really flexible, and the --merged works on HEAD only. `git branch --merged foo` will list branches that are merged in the history of 'foo'. And the equivalent is true for `--contains`. Not sure if that will solve everything, though. Øsse -- 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] is_hfs_dotgit: loosen over-eager match of \u{..47}
Our is_hfs_dotgit function relies on the hackily-implemented next_hfs_char to give us the next character that an HFS+ filename comparison would look at. It's hacky because it doesn't implement the full case-folding table of HFS+; it gives us just enough to see if the path matches .git. At the end of next_hfs_char, we use tolower() to convert our 32-bit code point to lowercase. Our tolower() implementation only takes an 8-bit char, though; it throws away the upper 24 bits. This means we can't have any false negatives for is_hfs_dotgit. We only care about matching 7-bit ASCII characters in .git, and we will correctly process 'G' or 'g'. However, we _can_ have false positives. Because we throw away the upper bits, code point \u{0147} (for example) will look like 'G' and get downcased to 'g'. It's not known whether a sequence of code points whose truncation ends up as .git is meaningful in any language, but it does not hurt to be more accurate here. We can just pass out the full 32-bit code point, and compare it manually to the upper and lowercase characters we care about. Signed-off-by: Jeff King p...@peff.net --- I saw Linus ask about this on G+. I had done the no false negative analysis when writing the patch, but didn't consider the false positive. Another way of accomplishing the same thing is for next_hfs_char to continue folding case, but _only_ do so for 8-bit code points. Like: return (out 0xff00) ? out : tolower(out); I think the what's below is more obvious (and is actually how I originally wrote it; I switched to using tolower() during development to try to make it more readable). t/t1450-fsck.sh | 16 utf8.c | 17 - 2 files changed, 28 insertions(+), 5 deletions(-) diff --git a/t/t1450-fsck.sh b/t/t1450-fsck.sh index 1f04b8a..3f5883d 100755 --- a/t/t1450-fsck.sh +++ b/t/t1450-fsck.sh @@ -349,6 +349,22 @@ dot-backslash-case ..GITfoobar dotgit-case-backslash .gitfoobar EOF +test_expect_success 'fsck allows .Ňit' ' + ( + git init not-dotgit + cd not-dotgit + echo content file + git add file + git commit -m base + blob=$(git rev-parse :file) + printf 100644 blob $blob\t.\\305\\207it tree + tree=$(git mktree tree) + git fsck 2err + cat err + ! test -s err + ) +' + # create a static test repo which is broken by omitting # one particular object ($1, which is looked up via rev-parse # in the new repository). diff --git a/utf8.c b/utf8.c index 9a3f4ad..34a779e 100644 --- a/utf8.c +++ b/utf8.c @@ -606,7 +606,7 @@ static ucs_char_t next_hfs_char(const char **in) * but this is enough to catch anything that will convert * to .git */ - return tolower(out); + return out; } } @@ -614,10 +614,17 @@ int is_hfs_dotgit(const char *path) { ucs_char_t c; - if (next_hfs_char(path) != '.' || - next_hfs_char(path) != 'g' || - next_hfs_char(path) != 'i' || - next_hfs_char(path) != 't') + c = next_hfs_char(path); + if (c != '.') + return 0; + c = next_hfs_char(path); + if (c != 'g' c != 'G') + return 0; + c = next_hfs_char(path); + if (c != 'i' c != 'I') + return 0; + c = next_hfs_char(path); + if (c != 't' c != 'T') return 0; c = next_hfs_char(path); if (c !is_dir_sep(c)) -- 2.2.1.376.gec59d43 -- 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: Question on for-each-ref and --contains
On Tue, Dec 23, 2014 at 09:50:50AM +0200, St. Blind wrote: In order to make some analyses here I had to build certain report lists of our remote refs, based on various containing and merged rules. We have more than hundred such refs at a time usually. So my poor script which tries to make decisions with the help of rev-list for each ref on each arbitrary commit is not really fast. The git-branch --contains and --merged are not very handy too, because the output is not really flexible, and the --merged works on HEAD only. Do you think is a good idea to have the option --contains in for-each-ref too (and/or in rev-list directly)? If yes, then probably also the --(no-)merged options, but hopefully with the arbitrary commit this time? Yes, it's absolutely a good idea to teach for-each-ref these options. This is something I've been meaning to work on for a while. This is at least the second time it has been mentioned in the past week or so, too. Maybe I'll try to move it forward soon. The last attempt got stuck on factoring out a single universal --contains traversal (we have 2 implementations): http://thread.gmane.org/gmane.comp.version-control.git/252472 I don't think there was any particular blocker besides time to look at and think about it. We need to be very sure of the termination requirements for the traversal (there was some discussion in the thread, but I think I convinced myself that some of the optimization suggestions there could return wrong answers for some cases). -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
Question about installing git from source
Hello All, Trying to install git from source, executing: make configure ./configure --prefix=/usr make --prefix=/usr all sudo make install DISTDIR=/usr And getting following: install -d -m 755 '/usr/usr/bin' Copying scripts to /usr/bin cp: omitting directory ‘autom4te.cache’ cp: omitting directory ‘bin-wrappers’ cp: omitting directory ‘block-sha1’ cp: omitting directory ‘builtin’ cp: omitting directory ‘compat’ cp: omitting directory ‘contrib’ cp: omitting directory ‘Documentation’ cp: omitting directory ‘ewah’ cp: omitting directory ‘git-gui’ cp: omitting directory ‘gitk-git’ cp: omitting directory ‘gitweb’ cp: omitting directory ‘mergetools’ cp: omitting directory ‘perl’ cp: omitting directory ‘po’ cp: omitting directory ‘ppc’ cp: omitting directory ‘t’ cp: omitting directory ‘templates’ cp: omitting directory ‘vcs-svn’ cp: omitting directory ‘xdiff’ Done What's problem can be here? Thank you. -- 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 16/18] fsck: support demoting errors to warnings
Hi Junio, On Mon, 22 Dec 2014, Junio C Hamano wrote: Johannes Schindelin johannes.schinde...@gmx.de writes: On Mon, 22 Dec 2014, Junio C Hamano wrote: Johannes Schindelin johannes.schinde...@gmx.de writes: Of course you can say that! ;-) The problem these ugly messages try to solve is to give the user a hint which setting to change if they want to override the default behavior, though... Ahh, OK, then dashed form would not work as a configuration variable names, so missingTaggerEntry would be the only usable option. Except that cunning me has made it so that both missing-tagger-entry *and* missingTaggerEntry work... Then the missing-tagger-entry side needs to be dropped. The naming does not conform to the way how we name our officially supported configuration variables. But it does conform with the way we do our command-line parameters, and it would actually cause *more* work (and more complicated code) to have two separate parsers, or even to force the parser to accept only one way to specify settings... Should I really introduce more complexity just to disallow non-camelCased config variables? Ciao, Dscho -- 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 04/18] Offer a function to demote fsck errors to warnings
Hi Junio, On Mon, 22 Dec 2014, Junio C Hamano wrote: Johannes Schindelin johannes.schinde...@gmx.de writes: For example, try to spot the error here: ... F(ALMOST_HAPPY, INFO) \ F(CANNOT_RECOVER, ERROR) \ F(COFFEE_IS_EMPTY, WARN) \ F(JUST_BEING_CHATTY, INFO) \ F(LIFE_IS_GOOD, INFO) \ F(MISSING_SOMETHING_VITAL, FATAL_ERROR) \ F(NEED_TO_SLEEP, WARN) \ F(SOMETHING_WENT_WRONG, ERROR) \ ... But that is not what is being suggested at all. I already said that FIRST_SOMETHING is fine as a measure to initialize, didn't I? I am only saying that if you have a place to store customized level, you should initialize that part with default levels and always look it up from that place at runtime. It is perfectly fine for the initialization step to take advantage of the ordering and FIRST_SOMETHING constants. Thanks for clarifying, I was worried that you wanted to encode the severity levels explicitly (F(ID, LEVEL) instead of F(ID) in the correct order). The DRY principle also suggests that we should not encode the severity level in two ways (which would leave the door open for inconsistencies). That means that we should not initialize a static array of severity levels, but initialize the array using a loop. Okay, now that we have established that the initial ordering by severity makes sense, let's examine the initialization step. Basically, our approaches differ only in *when* to initialize that array of severity levels: you want to initialize it always, and I want to initialize it only when the severity levels are customized by the caller. Now, let's have a look how the fsck_options are currently initialized. My code follows the convention established with strbufs, argv_arrays, etc: there is a preprocessor definition (_DEFAULT, imitating the _INIT definitions) that allows us to initialize such structs very conveniently. Please note that no loop is required, and certainly no extra code has to be called to initialize the struct. We get away with initializing that array lazily in the fsck_strict_mode() function when we detect that it needs to be initialized, being populated by the very same function that provides the default settings before customization. This is a very robust setup as the knowledge about, say, the size of that array is confined strictly to fsck.c. However, if we had to change the lookup such that it uses an array always, we would have to introduce a function to initialize the struct, always, in particular we would have to find a place to call that initialization function in, say, builtin/fsck.c (actually, in every code path that calls into the fsck machinery). Arguably, the code would get more complex – introducing new call paths just to initialize the fsck_options struct – and I would argue further that there is no gain from an elegance, readability and maintenance point of view: whether the array is initialized lazily or not, it will be initialized the exact same way. All it means is that we have to introduce separate code paths because we would separate explicitly the initialization from the configuration step. Therefore, I do not believe that introducing an fsck_options_init() is what you would really want. An alternative would be to initialize the array at compile time – we would have to violate the DRY principle for that, repeating the severity levels many times over, and we could no longer confine the visibility to the message IDs to fsck.c because not only the size of the array of severity levels would have to be known to every user of fsck.h, but the exact default severity levels themselves, to be able to initialize the struct. But we could initialize the struct with a known set of settings via the _DEFAULTS definition that way. However, you already expressed slight disagreement with the preprocessor magic needed to initialize both the enum and the array of message ID strings from the same list in a way that lets the compiler ensure consistency; I am afraid that if I were to modify _DEFAULTS to populate the entire severity level array, the resulting code would find your utter contempt. I believe, therefore, that this is also not what you want. So that leaves only one alternative: to initialize a global array with the default severity levels at *some* stage. I have no idea what that stage would be, therefore we would have to either establish ugly, and DRY-violating, compile time initialization, or we would have to call a function before using any of the fsck machinery – but that is fragile: it is too easy to forget one call path and access an uninitialized array! Worse, even if we *had* a fully initialized array of default severity levels, we would still have to have an on-demand copy (i.e. lazy initialization) of said array lest we modify the global defaults in fsck_strict_mode()! Essentially, we would just *add* complexity to the current solution, not replace it with anything simpler. Therefore, I believe
[PATCH v3 3/4] pack-objects: use --objects-edge-aggressive only for shallow repos
Using --objects-edge-aggressive is important for shallow repos, as it can result in a much smaller pack (in some cases, 10% of the size). However, it performs poorly on large non-shallow repositories with many refs. Since shallow repositories are less likely to have many refs (due to having less history), the smaller pack size is advantageous there. Adjust pack-objects to use --objects-edge-aggressive only for shallow repositories. Signed-off-by: brian m. carlson sand...@crustytoothpaste.net --- Documentation/rev-list-options.txt | 3 ++- builtin/pack-objects.c | 4 +++- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/Documentation/rev-list-options.txt b/Documentation/rev-list-options.txt index 8cb6f92..2984f40 100644 --- a/Documentation/rev-list-options.txt +++ b/Documentation/rev-list-options.txt @@ -659,7 +659,8 @@ These options are mostly targeted for packing of Git repositories. --objects-edge-aggressive:: Similar to `--objects-edge`, but it tries harder to find excluded - commits at the cost of increased time. + commits at the cost of increased time. This is used instead of + `--objects-edge` to build ``thin'' packs for shallow repositories. --unpacked:: Only useful with `--objects`; print the object IDs that are not diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c index 3f9f5c7..f3ba861 100644 --- a/builtin/pack-objects.c +++ b/builtin/pack-objects.c @@ -2711,7 +2711,9 @@ int cmd_pack_objects(int argc, const char **argv, const char *prefix) argv_array_push(rp, pack-objects); if (thin) { use_internal_rev_list = 1; - argv_array_push(rp, --objects-edge); + argv_array_push(rp, is_repository_shallow() + ? --objects-edge-aggressive + : --objects-edge); } else argv_array_push(rp, --objects); -- 2.2.1.209.g41e5f3a -- 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 v3 1/4] Documentation: add missing article in rev-list-options.txt
Add the missing article a. Signed-off-by: brian m. carlson sand...@crustytoothpaste.net --- Documentation/rev-list-options.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Documentation/rev-list-options.txt b/Documentation/rev-list-options.txt index afccfdc..2277fcb 100644 --- a/Documentation/rev-list-options.txt +++ b/Documentation/rev-list-options.txt @@ -653,7 +653,7 @@ These options are mostly targeted for packing of Git repositories. --objects-edge:: Similar to `--objects`, but also print the IDs of excluded commits prefixed with a ``-'' character. This is used by - linkgit:git-pack-objects[1] to build ``thin'' pack, which records + linkgit:git-pack-objects[1] to build a ``thin'' pack, which records objects in deltified form based on objects contained in these excluded commits to reduce network traffic. -- 2.2.1.209.g41e5f3a -- 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 v3 0/4] Improve push performance with lots of refs
This series contains patches to address a significant push performance regression in repositories with large amounts of refs. It avoids performing expensive edge marking unless the repository is shallow. The first patch in the series is a fix for a minor typo I discovered when editing the documentation. The second patch implements git rev-list --objects-edge-aggressive, and the third patch ensures it's used for shallow repos only. The final patch ensures that sending packs to shallow clients use --object-edge-aggressive as well. The only change from v2 is the addition of a fourth patch, which fixes t5500. It's necessary because the test wants packs for fetches to shallow clones to be minimal. I'm not especially thrilled with having to provide a --shallow command line argument, but the alternative is to buffer a potentially large amount of data in order to determine whether the remote side is shallow. The original fix was suggested by Duy Nguyen. brian m. carlson (4): Documentation: add missing article in rev-list-options.txt rev-list: add an option to mark fewer edges as uninteresting pack-objects: use --objects-edge-aggressive only for shallow repos upload-pack: use --objects-edge-aggressive for shallow fetches Documentation/git-pack-objects.txt | 7 ++- Documentation/git-rev-list.txt | 3 ++- Documentation/rev-list-options.txt | 7 ++- builtin/pack-objects.c | 7 ++- list-objects.c | 4 ++-- revision.c | 6 ++ revision.h | 1 + upload-pack.c | 4 +++- 8 files changed, 32 insertions(+), 7 deletions(-) -- 2.2.1.209.g41e5f3a -- 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 v3 2/4] rev-list: add an option to mark fewer edges as uninteresting
In commit fbd4a70 (list-objects: mark more commits as edges in mark_edges_uninteresting - 2013-08-16), we marked an increasing number of edges uninteresting. This change, and the subsequent change to make this conditional on --objects-edge, are used by --thin to make much smaller packs for shallow clones. Unfortunately, they cause a significant performance regression when pushing non-shallow clones with lots of refs (23.322 seconds vs. 4.785 seconds with 22400 refs). Add an option to git rev-list, --objects-edge-aggressive, that preserves this more aggressive behavior, while leaving --objects-edge to provide more performant behavior. Signed-off-by: brian m. carlson sand...@crustytoothpaste.net --- Documentation/git-rev-list.txt | 3 ++- Documentation/rev-list-options.txt | 4 list-objects.c | 4 ++-- revision.c | 6 ++ revision.h | 1 + 5 files changed, 15 insertions(+), 3 deletions(-) diff --git a/Documentation/git-rev-list.txt b/Documentation/git-rev-list.txt index fd7f8b5..5b11922 100644 --- a/Documentation/git-rev-list.txt +++ b/Documentation/git-rev-list.txt @@ -46,7 +46,8 @@ SYNOPSIS [ \--extended-regexp | -E ] [ \--fixed-strings | -F ] [ \--date=(local|relative|default|iso|iso-strict|rfc|short) ] -[ [\--objects | \--objects-edge] [ \--unpacked ] ] +[ [ \--objects | \--objects-edge | \--objects-edge-aggressive ] + [ \--unpacked ] ] [ \--pretty | \--header ] [ \--bisect ] [ \--bisect-vars ] diff --git a/Documentation/rev-list-options.txt b/Documentation/rev-list-options.txt index 2277fcb..8cb6f92 100644 --- a/Documentation/rev-list-options.txt +++ b/Documentation/rev-list-options.txt @@ -657,6 +657,10 @@ These options are mostly targeted for packing of Git repositories. objects in deltified form based on objects contained in these excluded commits to reduce network traffic. +--objects-edge-aggressive:: + Similar to `--objects-edge`, but it tries harder to find excluded + commits at the cost of increased time. + --unpacked:: Only useful with `--objects`; print the object IDs that are not in packs. diff --git a/list-objects.c b/list-objects.c index 2910bec..2a139b6 100644 --- a/list-objects.c +++ b/list-objects.c @@ -157,7 +157,7 @@ void mark_edges_uninteresting(struct rev_info *revs, show_edge_fn show_edge) if (commit-object.flags UNINTERESTING) { mark_tree_uninteresting(commit-tree); - if (revs-edge_hint !(commit-object.flags SHOWN)) { + if (revs-edge_hint_aggressive !(commit-object.flags SHOWN)) { commit-object.flags |= SHOWN; show_edge(commit); } @@ -165,7 +165,7 @@ void mark_edges_uninteresting(struct rev_info *revs, show_edge_fn show_edge) } mark_edge_parents_uninteresting(commit, revs, show_edge); } - if (revs-edge_hint) { + if (revs-edge_hint_aggressive) { for (i = 0; i revs-cmdline.nr; i++) { struct object *obj = revs-cmdline.rev[i].item; struct commit *commit = (struct commit *)obj; diff --git a/revision.c b/revision.c index 75dda92..753dd2f 100644 --- a/revision.c +++ b/revision.c @@ -1853,6 +1853,12 @@ static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg revs-tree_objects = 1; revs-blob_objects = 1; revs-edge_hint = 1; + } else if (!strcmp(arg, --objects-edge-aggressive)) { + revs-tag_objects = 1; + revs-tree_objects = 1; + revs-blob_objects = 1; + revs-edge_hint = 1; + revs-edge_hint_aggressive = 1; } else if (!strcmp(arg, --verify-objects)) { revs-tag_objects = 1; revs-tree_objects = 1; diff --git a/revision.h b/revision.h index 9cb5adc..033a244 100644 --- a/revision.h +++ b/revision.h @@ -93,6 +93,7 @@ struct rev_info { blob_objects:1, verify_objects:1, edge_hint:1, + edge_hint_aggressive:1, limited:1, unpacked:1, boundary:2, -- 2.2.1.209.g41e5f3a -- 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 v3 4/4] upload-pack: use --objects-edge-aggressive for shallow fetches
When fetching into a shallow repository, we want to aggressively mark edges as uninteresting, since this decreases the pack size. However, is_shallow_repository() returns false on the server side, since the server is not shallow. As the server, we get an indication of whether the client is shallow based on stdin; however, by the time we get to parsing stdin, we already will have called setup_revisions and won't be able to change our decision anymore. Teach pack-objects a --shallow option to indicate that the remote side is shallow and use it in upload-pack. Signed-off-by: brian m. carlson sand...@crustytoothpaste.net --- Documentation/git-pack-objects.txt | 7 ++- builtin/pack-objects.c | 5 - upload-pack.c | 4 +++- 3 files changed, 13 insertions(+), 3 deletions(-) diff --git a/Documentation/git-pack-objects.txt b/Documentation/git-pack-objects.txt index d2d8f47..c2f76fb 100644 --- a/Documentation/git-pack-objects.txt +++ b/Documentation/git-pack-objects.txt @@ -13,7 +13,7 @@ SYNOPSIS [--no-reuse-delta] [--delta-base-offset] [--non-empty] [--local] [--incremental] [--window=n] [--depth=n] [--revs [--unpacked | --all]] [--stdout | base-name] - [--keep-true-parents] object-list + [--shallow] [--keep-true-parents] object-list DESCRIPTION @@ -190,6 +190,11 @@ required objects and is thus unusable by Git without making it self-contained. Use `git index-pack --fix-thin` (see linkgit:git-index-pack[1]) to restore the self-contained property. +--shallow:: + Optimize a pack that will be provided to a client with a shallow + repository. This option, combined with \--thin, can result in a + smaller pack at the cost of speed. + --delta-base-offset:: A packed archive can express the base object of a delta as either a 20-byte object name or as an offset in the diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c index f3ba861..6bfbd3d 100644 --- a/builtin/pack-objects.c +++ b/builtin/pack-objects.c @@ -2613,6 +2613,7 @@ int cmd_pack_objects(int argc, const char **argv, const char *prefix) { int use_internal_rev_list = 0; int thin = 0; + int shallow = 0; int all_progress_implied = 0; struct argv_array rp = ARGV_ARRAY_INIT; int rev_list_unpacked = 0, rev_list_all = 0, rev_list_reflog = 0; @@ -2677,6 +2678,8 @@ int cmd_pack_objects(int argc, const char **argv, const char *prefix) PARSE_OPT_OPTARG, option_parse_unpack_unreachable }, OPT_BOOL(0, thin, thin, N_(create thin packs)), + OPT_BOOL(0, shallow, shallow, +N_(create packs suitable for shallow fetches)), OPT_BOOL(0, honor-pack-keep, ignore_packed_keep, N_(ignore packs that have companion .keep file)), OPT_INTEGER(0, compression, pack_compression_level, @@ -2711,7 +2714,7 @@ int cmd_pack_objects(int argc, const char **argv, const char *prefix) argv_array_push(rp, pack-objects); if (thin) { use_internal_rev_list = 1; - argv_array_push(rp, is_repository_shallow() + argv_array_push(rp, is_repository_shallow() || shallow ? --objects-edge-aggressive : --objects-edge); } else diff --git a/upload-pack.c b/upload-pack.c index ac9ac15..b531a32 100644 --- a/upload-pack.c +++ b/upload-pack.c @@ -86,7 +86,7 @@ static void create_pack_file(void) corruption on the remote side.; int buffered = -1; ssize_t sz; - const char *argv[12]; + const char *argv[13]; int i, arg = 0; FILE *pipe_fd; @@ -100,6 +100,8 @@ static void create_pack_file(void) argv[arg++] = --thin; argv[arg++] = --stdout; + if (shallow_nr) + argv[arg++] = --shallow; if (!no_progress) argv[arg++] = --progress; if (use_ofs_delta) -- 2.2.1.209.g41e5f3a -- 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: [msysGit] Re: Announcing Git for Windows 1.9.5
On 2014-12-23 10.48, Pat Tressel wrote: Bugfixes - Safeguards against bogus file names on NTFS (CVE-2014-9390). Apologies, but...is it possible to get back an old version? I installed this version, and it no longer recognizes non-English characters that appear in some file names we have in our repo. I'm only guessing that this has to do with the vuln fix. (If you want, I can file a bug report -- this should be easy to reproduce, as all that's needed should be to clone our repo (https://github.com/flavour/eden) and do git status -- two files will appear as deleted, but they're not. Hmm, it may be necessary to do the git clone with an old version, if the new version is unable to handle those file names.) Thanks! -- Pat You may consider to convert the repo into unicode, and update all clients accessing this repo. https://github.com/kblees/git/wiki -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 2/2] Add a regression test for 'git remote add existing same-url'
Signed-off-by: Johannes Schindelin johannes.schinde...@gmx.de --- t/t5505-remote.sh | 5 + 1 file changed, 5 insertions(+) diff --git a/t/t5505-remote.sh b/t/t5505-remote.sh index ac79dd9..17c6330 100755 --- a/t/t5505-remote.sh +++ b/t/t5505-remote.sh @@ -1113,4 +1113,9 @@ test_extra_arg set-url origin newurl oldurl # prune takes any number of args # update takes any number of args +test_expect_success 'add remote matching the insteadOf URL' ' + git config url@example.com.insteadOf backup + git remote add backup x...@example.com +' + test_done -- 2.0.0.rc3.9669.g840d1f9 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 0/2] Let `git remote add` play nicer with url.url.insteadOf
Anastas Dancha reported that it is not possible to add a remote when there is already a url.url.insteadOf = nick setting in $HOME/.gitconfig. While it makes sense to prevent surprises when a user adds a remote and it fetches from somewhere completely different, it makes less sense to prevent adding a remote when it is actually the same that was specified in the config. Therefore we add just another check that let's `git remote add` continue when the existing remote's URL is identical to the specified one. Interdiff below the diffstat (the commit message was also touched up to stop pretending that we allow adding a remote twice when the URL did not change). Signed-off-by: Johannes Schindelin johannes.schinde...@gmx.de Johannes Schindelin (2): git remote: allow adding remotes agreeing with url.insteadOf Add a regression test for 'git remote add existing same-url' builtin/remote.c | 4 +++- t/t5505-remote.sh | 5 + 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/builtin/remote.c b/builtin/remote.c index 9168c83..b4ff468 100644 --- a/builtin/remote.c +++ b/builtin/remote.c @@ -180,7 +180,8 @@ static int add(int argc, const char **argv) url = argv[1]; remote = remote_get(name); - if (remote (remote-url_nr 1 || (strcmp(name, remote-url[0]) + if (remote (remote-url_nr 1 || + (strcmp(name, remote-url[0]) strcmp(url, remote-url[0])) || remote-fetch_refspec_nr)) die(_(remote %s already exists.), name); -- 2.0.0.rc3.9669.g840d1f9 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 1/2] git remote: allow adding remotes agreeing with url.....insteadOf
When adding a remote, we make sure that the remote does not exist already. However, this test was not quite correct: when the url.insteadOf config variable was set to the remote name to be added, the code would assume that the remote exists already. Let's allow adding remotes when there is a url.insteadOf setting when both the name and the URL agree with the remote to be added. It might seem like a mistake to compare against remote-url[0] without verifying that remote-url_nr =1, but at this point a missing URL has been filled by the name already, therefore url_nr cannot be zero. Noticed by Anastas Dancha. Signed-off-by: Johannes Schindelin johannes.schinde...@gmx.de --- builtin/remote.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/builtin/remote.c b/builtin/remote.c index 46ecfd9..b4ff468 100644 --- a/builtin/remote.c +++ b/builtin/remote.c @@ -180,7 +180,9 @@ static int add(int argc, const char **argv) url = argv[1]; remote = remote_get(name); - if (remote (remote-url_nr 1 || strcmp(name, remote-url[0]) || + if (remote (remote-url_nr 1 || + (strcmp(name, remote-url[0]) + strcmp(url, remote-url[0])) || remote-fetch_refspec_nr)) die(_(remote %s already exists.), name); -- 2.0.0.rc3.9669.g840d1f9 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] git remote add: allow re-adding remotes with the same URL
Hi Junio, On Mon, 22 Dec 2014, Junio C Hamano wrote: Johannes Schindelin johannes.schinde...@gmx.de writes: When adding a remote, we make sure that the remote does not exist already. For convenience, we allow re-adding remotes with the same URLs. This also handles the case that there is an [url ...] insteadOf setting in the config. It might seem like a mistake to compare against remote-url[0] without verifying that remote-url_nr =1, but at this point a missing URL has been filled by the name already, therefore url_nr cannot be zero. Noticed by Anastas Dancha. Signed-off-by: Johannes Schindelin johannes.schinde...@gmx.de --- builtin/remote.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/builtin/remote.c b/builtin/remote.c index 46ecfd9..9168c83 100644 --- a/builtin/remote.c +++ b/builtin/remote.c @@ -180,7 +180,8 @@ static int add(int argc, const char **argv) url = argv[1]; remote = remote_get(name); - if (remote (remote-url_nr 1 || strcmp(name, remote-url[0]) || + if (remote (remote-url_nr 1 || (strcmp(name, remote-url[0]) + strcmp(url, remote-url[0])) || remote-fetch_refspec_nr)) die(_(remote %s already exists.), name); When we need to fold an overlong line, it is easier to read if the line is folded at an operator with higher precedence, i.e. this line if (A (B || (C D) || E)) folded like this if (A (B || (C D) || E)) is harder to read than when folded like this if (A (B || (C D) || E)) So, it is an error if we have remote and if (1) URL for the remote is defined already twice or more; or (2) we are adding a nickname (i.e. not a URL) and it is different from what we already have; or (3) we already have fetch_refspec The way I read the log message's rationale was that this is to allow replacing an existing remote's URL; wouldn't checking the existence of fetch_refspec go against that goal? Puzzled. Either the code is wrong or I am mislead by the explanation in the log. I hope v2 addresses your concerns. Ciao, Dscho -- 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] is_hfs_dotgit: loosen over-eager match of \u{..47}
On 2014-12-23 09.45, Jeff King wrote: Our is_hfs_dotgit function relies on the hackily-implemented next_hfs_char to give us the next character that an HFS+ filename comparison would look at. It's hacky because it doesn't implement the full case-folding table of HFS+; it gives us just enough to see if the path matches .git. At the end of next_hfs_char, we use tolower() to convert our 32-bit code point to lowercase. Our tolower() implementation only takes an 8-bit char, though; it throws away the upper 24 bits. This means we can't have any false negatives for is_hfs_dotgit. We only care about matching 7-bit ASCII characters in .git, and we will correctly process 'G' or 'g'. However, we _can_ have false positives. Because we throw away the upper bits, code point \u{0147} (for example) will look like 'G' and get downcased to 'g'. It's not known whether a sequence of code points whose truncation ends up as .git is meaningful in any language, but it does not hurt to be more accurate here. We can just pass out the full 32-bit code point, and compare it manually to the upper and lowercase characters we care about. Signed-off-by: Jeff King p...@peff.net --- I saw Linus ask about this on G+. I had done the no false negative analysis when writing the patch, but didn't consider the false positive. Another way of accomplishing the same thing is for next_hfs_char to continue folding case, but _only_ do so for 8-bit code points. Like: Don't we have the same possible problem under NTFS? Under Linux + VFAT ? Under all OS + VFAT ? And would it make sense to turn this return (out 0xff00) ? out : tolower(out); into this: static ucs_char_t unicode_tolower(ucs_char_t ch) { return (ch 0xff00) ? ch : tolower(ch); } And what happens if I export NTFS to Mac OS X? (Other combinations possible) Shouldn't fsck under all OS warn for NTFS and hfs possible attacks ? -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: git update-ref --stdin : too many open files
Stefan Beller stefanbel...@gmail.com writes: Sounds reasonable. Though by closing the file we're giving up again a bit of safety. If we close the file everyone could tamper with the lock file. (Sure they are not supposed to touch it, but they could) There are locking primitives (SysV mandatory locking) that require you to keep the file you have lock on open for you to retain the ownership of the lock, and that kind of lock does prevent random other processes from simultaneously accessing the locked file. But that is not what our locks are designed around; our locks rely only on open(O_EXCL|O_CREAT) fails if it already exists. And between keeping a lockfile open and closing but not removing a lockfile, there is no difference how the lockfile that still exists prevents open(O_EXCL|O_CREAT) by other processes from succeeding. So we are not giving up any safety at all, which is a good thing ;-). -- 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 04/18] Offer a function to demote fsck errors to warnings
Johannes Schindelin johannes.schinde...@gmx.de writes: However, if we had to change the lookup such that it uses an array always, we would have to introduce a function to initialize the struct, always, in particular we would have to find a place to call that initialization function in, say, builtin/fsck.c (actually, in every code path that calls into the fsck machinery). You would need to call a function to initialize the table if you support customization by reading the configuration files anyway. I am not sure why you think finding such a place is hard. Puzzled. Also I suspect that you can tell the compiler to initialize the array in place with default values, perhaps like this? -- 8 -- #include stdio.h /* sorted by the default severity (lowest impact first) */ #define EVENT_LIST(F) \ F(EVENT_A), \ F(EVENT_B), \ F(EVENT_C), \ F(EVENT_D) #define ID_(event) ID_ ## event enum event_id { EVENT_LIST(ID_) }; enum severity_level { severity_info, severity_warn, severity_error }; /* below this one are INFO */ #define FIRST_WARN_EVENT_ID ID_EVENT_B /* below this one are WARN */ #define FIRST_ERROR_EVENT_IDID_EVENT_C #define STRING_(s) #s #define DESC_(event) \ { \ ID_ ## event, \ STRING_(event), \ (ID_ ## event FIRST_WARN_EVENT_ID \ ? severity_info \ : ID_ ## event FIRST_ERROR_EVENT_ID \ ? severity_warn \ : severity_error) \ } struct event_config { enum event_id id; const char * name; enum severity_level level; } event[] = { EVENT_LIST(DESC_) }; int main(int ac, char **av) { int i; for (i = 0; i sizeof(event) / sizeof(event[0]); i++) { printf(%d, %s, %d\n, event[i].id, event[i].name, event[i].level); } 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 04/18] Offer a function to demote fsck errors to warnings
Hi Junio, On Tue, 23 Dec 2014, Junio C Hamano wrote: Johannes Schindelin johannes.schinde...@gmx.de writes: However, if we had to change the lookup such that it uses an array always, we would have to introduce a function to initialize the struct, always, in particular we would have to find a place to call that initialization function in, say, builtin/fsck.c (actually, in every code path that calls into the fsck machinery). You would need to call a function to initialize the table if you support customization by reading the configuration files anyway. Yes, this is the config machinery. But I need employ that only if I want to let the caller customize the severity levels. However, the fsck machinery is also called from places where such a customization is not offered. They would now need to be changed, too. Also I suspect that you can tell the compiler to initialize the array in place with default values, perhaps like this? -- 8 -- #include stdio.h /* sorted by the default severity (lowest impact first) */ #define EVENT_LIST(F) \ F(EVENT_A), \ F(EVENT_B), \ F(EVENT_C), \ F(EVENT_D) #define ID_(event) ID_ ## event enum event_id { EVENT_LIST(ID_) }; enum severity_level { severity_info, severity_warn, severity_error }; /* below this one are INFO */ #define FIRST_WARN_EVENT_ID ID_EVENT_B /* below this one are WARN */ #define FIRST_ERROR_EVENT_ID ID_EVENT_C #define STRING_(s) #s #define DESC_(event) \ { \ ID_ ## event, \ STRING_(event), \ (ID_ ## event FIRST_WARN_EVENT_ID \ ? severity_info \ : ID_ ## event FIRST_ERROR_EVENT_ID \ ? severity_warn \ : severity_error) \ } This is exactly the ugly, ugly preprocessor construct I thought you would meet with contempt. I mean, compared to this, my FUNC() hack is outright pretty ;-) And *still*, this is *just* a global table with defaults. I would *still* need to copy-on-write when the first customization of the severity level takes place because I cannot allow the global defaults to be modified by one caller (that would defeat the whole purpose of having per-caller settings bundled in the fsck_options struct). You see, I still would need to have a lazy initialization, the complexity in that part would not be reduced at all. So I am afraid that this approach really adds complexity rather than replacing it with something simpler than my current code. Ciao, Dscho -- 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 16/18] fsck: support demoting errors to warnings
Johannes Schindelin johannes.schinde...@gmx.de writes: Of course you can say that! ;-) The problem these ugly messages try to solve is to give the user a hint which setting to change if they want to override the default behavior, though... Ahh, OK, then dashed form would not work as a configuration variable names, so missingTaggerEntry would be the only usable option. Except that cunning me has made it so that both missing-tagger-entry *and* missingTaggerEntry work... Then the missing-tagger-entry side needs to be dropped. The naming does not conform to the way how we name our officially supported configuration variables. But it does conform with the way we do our command-line parameters, Hmmm What is the expected user interaction? The way I read the series was ($MISSING_TAGGER stands for the token we choose to show): (1) The user runs fsck without customization, and may see a warning (or error): $ git fsck error in tag d6602ec5194c87b0fc87103ca4d67251c76f233a: $MISSING_TAGGER (2) The user demotes it to warning and runs fsck again: $ git config fsck.$MISSING_TAGGER warn $ git fsck warning: tag d6602ec5194c87b0fc87103ca4d67251c76f233a: $MISSING_TAGGER I suspect that it would be much better if the configuration variables were organized the other way around, e.g. $ git config fsck.warn missingTagger,someOtherKindOfError Then a one-shot override would make sense and easier to give as command line option, e.g. $ git fsck --warn=missingTagger,someOtherKindOfError But the proposed organization to use one variable per questionable event type (as opposed to one variable per severity level) would lead to a one-shot override of this form, e.g. $ git fsck --missing-tagger=warn --some-other-kind-of-error=warn which I think is insane to require us to support unbound number of dashed options. Or are you saying that we allow git config core.file-mode true from the command line to set core.fileMode configuration? Puzzled... -- 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
mangled file names in git checkout-index --temp output when run in repo subdirectory
I am using git checkout-index --temp to obtain copies of files from the index, but it does not always print valid file names unless run from the repository root. git checkout-index --temp prints names of files in the index interpreted relative to the current directory below the repository root. If you have a git repo in /tmp/gitbug, you are in /tmp/gitbug/dir1, and you run git checkout-index --temp /tmp/gitbug/dir1/file1, the file listing prints just file1. So far so good. However, this file name shortening appears to assume that all file names being printed will be within the subtree rooted at the current directory, and it just skips over the first N characters in the name, where N is the length of the current directory name relative to the repo root (in the above example, N = len(dir1/) = 5). That is, if you have a git repo in /tmp/gitbug, you are in /tmp/gitbug/dir1, and you run git checkout-index --temp /tmp/gitbug/longdir2/file2, the file listing prints ir2/file2, not ../longdir2/file2. If you arrange things just right you can get as the file name. I've never seen arbitrary binary garbage, so (without inspecting the source code) it does appear that something is keeping the printed string pointer within the original string, even in the case when N the length of the string being printed. I have seen this in a few different versions of Git. I just downloaded and built Git 2.2.1 from sources and confirmed that it has the buggy behavior. Below is a shell script demonstrating the bug and an annotated transcript of the execution of the script on my system. The workaround seems to be that, until the bug is fixed, git checkout-index must be run in the repository root. Thanks. Russ % cat /tmp/gitbugdemo #!/bin/bash git=${1:-git} set -e rm -rf /tmp/gitbug mkdir /tmp/gitbug cd /tmp/gitbug $git init . mkdir dir1 longdir2 veryveryverylongdir3 echo hello world dir1/file1 echo hello world longdir2/file2 echo hello world veryveryverylongdir3/file3 files=/tmp/gitbug/dir1/file1 /tmp/gitbug/longdir2/file2 /tmp/gitbug/veryveryverylongdir3/file3 relfiles=$(echo $files | sed 's;/tmp/gitbug/;../;g') rootfiles=$(echo $relfiles | sed 's;\.\./;;g') set -x $git version cd /tmp/gitbug $git add . $git checkout-index --temp $files $git checkout-index --temp $rootfiles cd /tmp/gitbug/dir1 $git checkout-index --temp $files $git checkout-index --temp $relfiles cd /tmp/gitbug/longdir2 $git checkout-index --temp $files $git checkout-index --temp $relfiles cd /tmp/gitbug/veryveryverylongdir3 $git checkout-index --temp $files $git checkout-index --temp $relfiles % % /tmp/gitbugdemo git2.2.1 warning: templates not found /usr/local/share/git-core/templates Initialized empty Git repository in /private/tmp/gitbug/.git/ + git2.2.1 version git version 2.2.1 + cd /tmp/gitbug + git2.2.1 add . ### In root directory, all is well. + git2.2.1 checkout-index --temp /tmp/gitbug/dir1/file1 /tmp/gitbug/longdir2/file2 /tmp/gitbug/veryveryverylongdir3/file3 .merge_file_TRqU2e dir1/file1 .merge_file_7uajP4 longdir2/file2 .merge_file_L4OD1U veryveryverylongdir3/file3 + git2.2.1 checkout-index --temp dir1/file1 longdir2/file2 veryveryverylongdir3/file3 .merge_file_77EMra dir1/file1 .merge_file_DpKgMu longdir2/file2 .merge_file_w8yfxA veryveryverylongdir3/file3 ### In dir1/, loses first 5 chars of file paths outside current directory. ### Both absolute and relative command-line arguments trigger bug. + cd /tmp/gitbug/dir1 + git2.2.1 checkout-index --temp /tmp/gitbug/dir1/file1 /tmp/gitbug/longdir2/file2 /tmp/gitbug/veryveryverylongdir3/file3 .merge_file_2SwQmt file1 .merge_file_g0rOvr ir2/file2 .merge_file_3uDW54 eryverylongdir3/file3 + git2.2.1 checkout-index --temp ../dir1/file1 ../longdir2/file2 ../veryveryverylongdir3/file3 .merge_file_BwUlNt file1 .merge_file_gQji59 ir2/file2 .merge_file_hNNjc9 eryverylongdir3/file3 ### In longdir2/, loses first 9 chars of file paths outside current directory. ### Both absolute and relative command-line arguments trigger bug. + cd /tmp/gitbug/longdir2 + git2.2.1 checkout-index --temp /tmp/gitbug/dir1/file1 /tmp/gitbug/longdir2/file2 /tmp/gitbug/veryveryverylongdir3/file3 .merge_file_ljy0oC 1 .merge_file_ANLgpx file2 .merge_file_1lDbMp erylongdir3/file3 + git2.2.1 checkout-index --temp ../dir1/file1 ../longdir2/file2 ../veryveryverylongdir3/file3 .merge_file_ugqOsC 1 .merge_file_reoXBo file2 .merge_file_p4tQn9 erylongdir3/file3 ### In veryveryverylongdir3/, inconsistent truncation of file paths outside current directory. ### Absolute command-line arguments trigger bug but relative ones do not (!). + cd /tmp/gitbug/veryveryverylongdir3 + git2.2.1 checkout-index --temp /tmp/gitbug/dir1/file1 /tmp/gitbug/longdir2/file2 /tmp/gitbug/veryveryverylongdir3/file3 .merge_file_VvT0uT 1 .merge_file_UfAT1x file2 .merge_file_Fon1Ex file3 + git2.2.1 checkout-index --temp ../dir1/file1 ../longdir2/file2 ../veryveryverylongdir3/file3 .merge_file_YDHkDe ../dir1/file1 .merge_file_F7M27m ../longdir2/file2 .merge_file_ZkkXnQ
Re: [PATCH 16/18] fsck: support demoting errors to warnings
Hi Junio, On Tue, 23 Dec 2014, Junio C Hamano wrote: Johannes Schindelin johannes.schinde...@gmx.de writes: Of course you can say that! ;-) The problem these ugly messages try to solve is to give the user a hint which setting to change if they want to override the default behavior, though... Ahh, OK, then dashed form would not work as a configuration variable names, so missingTaggerEntry would be the only usable option. Except that cunning me has made it so that both missing-tagger-entry *and* missingTaggerEntry work... Then the missing-tagger-entry side needs to be dropped. The naming does not conform to the way how we name our officially supported configuration variables. But it does conform with the way we do our command-line parameters, Hmmm What is the expected user interaction? The way I read the series was ($MISSING_TAGGER stands for the token we choose to show): (1) The user runs fsck without customization, and may see a warning (or error): $ git fsck error in tag d6602ec5194c87b0fc87103ca4d67251c76f233a: $MISSING_TAGGER (2) The user demotes it to warning and runs fsck again: $ git config fsck.$MISSING_TAGGER warn $ git fsck warning: tag d6602ec5194c87b0fc87103ca4d67251c76f233a: $MISSING_TAGGER The intended use case is actually when receive.fsckObjects = true and you call `git push`, seeing 'remote: error: $MULTIPLE_AUTHORS: ...'. Now, the $MULTIPLE_AUTHORS *config* setting is parsed by `git receive-pack`, but that is not the command that needs to customize the fsck call: it is either `git index-pack` or `git unpack-objects`. So what `git receive-pack` does is to pass the config options as command-line options to the called command. For consistency with the rest of Git, the command-line options were *not* camel-cased, but lower-case, dash-separated. The parser I wrote actually accepts both versions, allowing me to skip the tedious step to convert the camelCased config setting into a lower-case-dashed version to pass to `index-pack` or `unpack-objects`, only to be parsed by the same parser as `fsck` would use directly. So I am rather happy with the fact that the parser handles both camelCased and lower-case-dashed versions. I suspect that it would be much better if the configuration variables were organized the other way around, e.g. $ git config fsck.warn missingTagger,someOtherKindOfError I had something similar in an earlier version of my patch series, but it was shot down rightfully: if you want to allow inheriting defaults from $HOME/.gitconfig, you have to configure the severity levels individually. (The current solution also sidesteps the problematic situation when both fsck.warn *and* fsck.error contain, say, missingTagger.) Then a one-shot override would make sense and easier to give as command line option, e.g. $ git fsck --warn=missingTagger,someOtherKindOfError Yep, my first implementation actually used `--strict=missing-tagger,-some-demoted-error`. But as I mentioned above, that approach is not as flexible as the current one. But the proposed organization to use one variable per questionable event type (as opposed to one variable per severity level) would lead to a one-shot override of this form, e.g. $ git fsck --missing-tagger=warn --some-other-kind-of-error=warn which I think is insane to require us to support unbound number of dashed options. The intended use case is actually *not* the command-line, but the config file, in particular allowing /etc/gitconfig, $HOME/.gitconfig *and* .git/config to customize the settings. Or are you saying that we allow git config core.file-mode true from the command line to set core.fileMode configuration? I do not understand this reference. I did not suggest to change `git config`, did I? If I did, I apologize; it was definitely *not* my intention to change long-standing customs. Ciao, Dscho -- 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: Question about installing git from source
Alexander Kuleshov kuleshovm...@gmail.com writes: Trying to install git from source, executing: make configure ./configure --prefix=/usr make --prefix=/usr all sudo make install DISTDIR=/usr That does not seem to match any of the ways how INSTALL tells us to build and install. Excerpts from INSTALL (1) ... If you want to do a global install, you can do $ make prefix=/usr all doc info ;# as yourself # make prefix=/usr install install-doc install-html install-info ;# as root Note how prefix is spelled. (2) Alternatively you can use autoconf generated ./configure script to set up install paths (via config.mak.autogen), so you can write instead $ make configure ;# as yourself $ ./configure --prefix=/usr ;# as yourself $ make all doc ;# as yourself # make install install-doc install-html;# as root Note how make does not have any prefix. Also when you install to a temporary directory so that you can tar up the resulting hierarchy, the variable to use is spelled DESTDIR, e.g. make DESTDIR=/var/tmp/git-2.2.1 install install-doc ... What's problem can be here? Hopefully the above would be a good start to help you figure that out. -- 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 16/18] fsck: support demoting errors to warnings
Junio C Hamano gits...@pobox.com writes: I suspect that it would be much better if the configuration variables were organized the other way around, e.g. $ git config fsck.warn missingTagger,someOtherKindOfError By the way, I think I like this organization is much better than the other way around, i.e. fsck.missingTagger=warn, as we do not want the namespace under fsck.* for variables that control the behaviour of fsck that are *NOT* kinds of questionable conditions fsck can find. -- 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: Question about installing git from source
Hell Junio, Look I download git from master, executed following: make configure make prefix=/usr all sudo make prefix=/usr install And got the same during installing: ~/dev/git $ sudo make prefix=/usr install GEN perl/PM.stamp SUBDIR perl /usr/bin/perl Makefile.PL PREFIX='/usr' INSTALL_BASE='' --localedir='/usr/share/locale' Generating a Unix-style perl.mak Writing perl.mak for Git Writing MYMETA.yml and MYMETA.json GEN git-add--interactive GEN git-difftool GEN git-archimport GEN git-cvsexportcommit GEN git-cvsimport GEN git-cvsserver GEN git-relink GEN git-send-email GEN git-svn GEN git-p4 SUBDIR gitweb SUBDIR ../ make[2]: 'GIT-VERSION-FILE' is up to date. GEN git-instaweb SUBDIR git-gui SUBDIR gitk-git SUBDIR perl /usr/bin/perl -pe s\Q++LOCALEDIR++\E/usr/share/locale Git/SVN/Prompt.pm blib/lib/Git/SVN/Prompt.pm /usr/bin/perl -pe s\Q++LOCALEDIR++\E/usr/share/locale Git/SVN/Editor.pm blib/lib/Git/SVN/Editor.pm /usr/bin/perl -pe s\Q++LOCALEDIR++\E/usr/share/locale Git/SVN/Fetcher.pm blib/lib/Git/SVN/Fetcher.pm /usr/bin/perl -pe s\Q++LOCALEDIR++\E/usr/share/locale Git/SVN/Utils.pm blib/lib/Git/SVN/Utils.pm /usr/bin/perl -pe s\Q++LOCALEDIR++\E/usr/share/locale Git/SVN/Log.pm blib/lib/Git/SVN/Log.pm /usr/bin/perl -pe s\Q++LOCALEDIR++\E/usr/share/locale Git/SVN/Memoize/YAML.pm blib/lib/Git/SVN/Memoize/YAML.pm /usr/bin/perl -pe s\Q++LOCALEDIR++\E/usr/share/locale Git/SVN.pm blib/lib/Git/SVN.pm /usr/bin/perl -pe s\Q++LOCALEDIR++\E/usr/share/locale Git/SVN/Migration.pm blib/lib/Git/SVN/Migration.pm /usr/bin/perl -pe s\Q++LOCALEDIR++\E/usr/share/locale Git/IndexInfo.pm blib/lib/Git/IndexInfo.pm /usr/bin/perl -pe s\Q++LOCALEDIR++\E/usr/share/locale Git/SVN/GlobSpec.pm blib/lib/Git/SVN/GlobSpec.pm /usr/bin/perl -pe s\Q++LOCALEDIR++\E/usr/share/locale Git/SVN/Ra.pm blib/lib/Git/SVN/Ra.pm /usr/bin/perl -pe s\Q++LOCALEDIR++\E/usr/share/locale Git.pm blib/lib/Git.pm /usr/bin/perl -pe s\Q++LOCALEDIR++\E/usr/share/locale Git/I18N.pm blib/lib/Git/I18N.pm SUBDIR templates install -d -m 755 '/usr/bin' Copying scripts to /usr/bin cp: omitting directory ‘bin-wrappers’ cp: omitting directory ‘block-sha1’ cp: omitting directory ‘builtin’ cp: omitting directory ‘compat’ cp: omitting directory ‘contrib’ cp: omitting directory ‘Documentation’ cp: omitting directory ‘ewah’ cp: omitting directory ‘git-gui’ cp: omitting directory ‘gitk-git’ ... 2014-12-23 23:03 GMT+06:00 Junio C Hamano gits...@pobox.com: Alexander Kuleshov kuleshovm...@gmail.com writes: Trying to install git from source, executing: make configure ./configure --prefix=/usr make --prefix=/usr all sudo make install DISTDIR=/usr That does not seem to match any of the ways how INSTALL tells us to build and install. Excerpts from INSTALL (1) ... If you want to do a global install, you can do $ make prefix=/usr all doc info ;# as yourself # make prefix=/usr install install-doc install-html install-info ;# as root Note how prefix is spelled. (2) Alternatively you can use autoconf generated ./configure script to set up install paths (via config.mak.autogen), so you can write instead $ make configure ;# as yourself $ ./configure --prefix=/usr ;# as yourself $ make all doc ;# as yourself # make install install-doc install-html;# as root Note how make does not have any prefix. Also when you install to a temporary directory so that you can tar up the resulting hierarchy, the variable to use is spelled DESTDIR, e.g. make DESTDIR=/var/tmp/git-2.2.1 install install-doc ... What's problem can be here? Hopefully the above would be a good start to help you figure that out. -- _ 0xAX -- 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 16/18] fsck: support demoting errors to warnings
Johannes Schindelin johannes.schinde...@gmx.de writes: The parser I wrote actually accepts both versions, allowing me to skip the tedious step to convert the camelCased config setting into a lower-case-dashed version to pass to `index-pack` or `unpack-objects`, only to be parsed by the same parser as `fsck` would use directly. So I am rather happy with the fact that the parser handles both camelCased and lower-case-dashed versions. That is myopic view of the world that ignores maintainability and teachability, doing disservice to our user base. What message does it send to an unsuspecting new user that fsck.random-error is silently accepted (because we will never document it) as an alias for fsck.randomError, while most of the configuration variables will not take such an alias? I suspect that it would be much better if the configuration variables were organized the other way around, e.g. $ git config fsck.warn missingTagger,someOtherKindOfError I had something similar in an earlier version of my patch series, but it was shot down rightfully: if you want to allow inheriting defaults from $HOME/.gitconfig, you have to configure the severity levels individually. Hmmm. What's wrong with fsck.warn -missingTagger that overrides the earlier one, or even fsck.info missingTagger after having fsck.warn other,missingTagger,yetanother, with the usual last one wins rule? Whoever shot it down rightfully is wrong here, I would think. But the proposed organization to use one variable per questionable event type (as opposed to one variable per severity level) would lead to a one-shot override of this form, e.g. $ git fsck --missing-tagger=warn --some-other-kind-of-error=warn which I think is insane to require us to support unbound number of dashed options. The intended use case is actually *not* the command-line, but the config file, in particular allowing /etc/gitconfig, $HOME/.gitconfig *and* .git/config to customize the settings. But we do need to worry about one-shot override from the command line. A configuration that sticks without a way to override is a no-no. Or are you saying that we allow git config core.file-mode true from the command line to set core.fileMode configuration? I do not understand this reference. I was puzzled by your command line and wondering if you meant from the command line, aVariable can be spelled a-variable. I did not suggest to change `git config`, did I? If I did, I apologize; it was definitely *not* my intention to change long-standing customs. Then fsck.missing-tagger is definitely out. Long standing customs is that a multi-word token at the first and the last level is not dashed-multi-word. -- 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 04/18] Offer a function to demote fsck errors to warnings
Johannes Schindelin johannes.schinde...@gmx.de writes: And *still*, this is *just* a global table with defaults. I would *still* need to copy-on-write when the first customization of the severity level takes place because I cannot allow the global defaults to be modified by one caller (that would defeat the whole purpose of having per-caller settings bundled in the fsck_options struct). If you allocate a per-invocation fsck_options struct, then the initialization the default with code is dead easy---you can just do it immediately after you x[cm]alloc()---no? What am I missing? -- 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 04/18] Offer a function to demote fsck errors to warnings
Hi Junio, On Tue, 23 Dec 2014, Junio C Hamano wrote: Johannes Schindelin johannes.schinde...@gmx.de writes: And *still*, this is *just* a global table with defaults. I would *still* need to copy-on-write when the first customization of the severity level takes place because I cannot allow the global defaults to be modified by one caller (that would defeat the whole purpose of having per-caller settings bundled in the fsck_options struct). If you allocate a per-invocation fsck_options struct, then the initialization the default with code is dead easy---you can just do it immediately after you x[cm]alloc()---no? There is no alloc. Right now, the initialization reads: struct fsck_options options = strict ? FSCK_OPTIONS_STRICT : FSCK_OPTIONS_DEFAULT; Ciao, Dscho -- 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 16/18] fsck: support demoting errors to warnings
Hi Junio, On Tue, 23 Dec 2014, Junio C Hamano wrote: Johannes Schindelin johannes.schinde...@gmx.de writes: The parser I wrote actually accepts both versions, allowing me to skip the tedious step to convert the camelCased config setting into a lower-case-dashed version to pass to `index-pack` or `unpack-objects`, only to be parsed by the same parser as `fsck` would use directly. So I am rather happy with the fact that the parser handles both camelCased and lower-case-dashed versions. That is myopic view of the world that ignores maintainability and teachability, doing disservice to our user base. Okay, so just to clarify: you want me to - split the parser into - a parser that accepts only camelCased variable names when they come from the config (for use in fsck and receive-pack), and - another parser that rejects camelCased variable names and only accepts lower-case-dashed, intended for command-line parsing in fsck, index-pack and unpack-objects, and - consequently have a converter from the camelCased variable names we receive from the config in receive-pack so we can pass lower-case-dashed settings to index-pack and unpack-objects. If you want it this way, I will do it this way. What message does it send to an unsuspecting new user that fsck.random-error is silently accepted (because we will never document it) as an alias for fsck.randomError, while most of the configuration variables will not take such an alias? I will not participate in a discussion about consistency again. There is nothing that can be done about it; what matters is what you will accept and what not. I will make the code stricter (and consequently more complex) if that is what you want. I suspect that it would be much better if the configuration variables were organized the other way around, e.g. $ git config fsck.warn missingTagger,someOtherKindOfError I had something similar in an earlier version of my patch series, but it was shot down rightfully: if you want to allow inheriting defaults from $HOME/.gitconfig, you have to configure the severity levels individually. Hmmm. What's wrong with fsck.warn -missingTagger that overrides the earlier one, or even fsck.info missingTagger after having fsck.warn other,missingTagger,yetanother, with the usual last one wins rule? I will change the code (next year...). But the proposed organization to use one variable per questionable event type (as opposed to one variable per severity level) would lead to a one-shot override of this form, e.g. $ git fsck --missing-tagger=warn --some-other-kind-of-error=warn which I think is insane to require us to support unbound number of dashed options. The intended use case is actually *not* the command-line, but the config file, in particular allowing /etc/gitconfig, $HOME/.gitconfig *and* .git/config to customize the settings. But we do need to worry about one-shot override from the command line. A configuration that sticks without a way to override is a no-no. And of course you can, by specifying the config setting via the -c command-line option. The only inconsistency here is that all other command-line options are lower-case-dashed, while the config settings are camelCased. Or are you saying that we allow git config core.file-mode true from the command line to set core.fileMode configuration? I do not understand this reference. I was puzzled by your command line and wondering if you meant from the command line, aVariable can be spelled a-variable. Well, of course, if you call `git -c aVariable command --option=a-variable` you have a nice accumulation of styles right there ;-) I did not suggest to change `git config`, did I? If I did, I apologize; it was definitely *not* my intention to change long-standing customs. Then fsck.missing-tagger is definitely out. Long standing customs is that a multi-word token at the first and the last level is not dashed-multi-word. But I already changed all of the patches to fsck.missingTagger. The only thing I did not do yet is to split the parser into two, one accepting only camelCased, one accepting only lower-case-dashed options, and a translator to convert from camelCase to lower-case-dashed versions (because it is a lot of work and additional complexity, as well as opportunity for bugs to hide because we'll have three code paths). I asked you above whether you want that, and I will do it if you say that you do. Ciao, Dscho -- 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: Question about installing git from source
Alexander Kuleshov kuleshovm...@gmail.com writes: install -d -m 755 '/usr/bin' Copying scripts to /usr/bin As 'git grep Copying scripts' gives us nothing, the message is obviously not what we are giving. Perhaps you have a strange install in your path that does not understand -d is a way to tell it to make sure a directory exists (by creating one as necessary)? -- 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: Question about installing git from source
I looked at Makefile, and seems that it occurs from here: https://github.com/git/git/blob/master/Makefile#L2205 It tries to copy all files/folders from git root directory but 'install' command prints this error. I tried to execute 'sudo install -d -m 755' in other directory and if there is directory too it prints the same error. For example: ~/scripts $ ls addPPAcleanMailTrash git-autorgit-ranges git-tp install scriptsssh-live term-help tp-git updateMail xray-start buildGit deployWork git-install git-remove-tags git-update-commit-message install.sh ssh-build ssh-wiwob-lab test updateGitDev updateNews test - is directory here ~/scripts $ sudo install -d -m 755 . Copying scripts to /usr/bin cp: omitting directory ‘test’ Done 2014-12-23 23:43 GMT+06:00 Junio C Hamano gits...@pobox.com: Alexander Kuleshov kuleshovm...@gmail.com writes: install -d -m 755 '/usr/bin' Copying scripts to /usr/bin As 'git grep Copying scripts' gives us nothing, the message is obviously not what we are giving. Perhaps you have a strange install in your path that does not understand -d is a way to tell it to make sure a directory exists (by creating one as necessary)? -- _ 0xAX -- 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 2/4] rev-list: add an option to mark fewer edges as uninteresting
This patch causes an error on my mac, test 5500 fetch-pack errors on part 44 - fetch creating new shallow root. It looks for remote: Total 1 in the fetch output and gets 3 instead. On Tue, Dec 23, 2014 at 7:01 AM, brian m. carlson sand...@crustytoothpaste.net wrote: In commit fbd4a70 (list-objects: mark more commits as edges in mark_edges_uninteresting - 2013-08-16), we marked an increasing number of edges uninteresting. This change, and the subsequent change to make this conditional on --objects-edge, are used by --thin to make much smaller packs for shallow clones. Unfortunately, they cause a significant performance regression when pushing non-shallow clones with lots of refs (23.322 seconds vs. 4.785 seconds with 22400 refs). Add an option to git rev-list, --objects-edge-aggressive, that preserves this more aggressive behavior, while leaving --objects-edge to provide more performant behavior. Signed-off-by: brian m. carlson sand...@crustytoothpaste.net --- Documentation/git-rev-list.txt | 3 ++- Documentation/rev-list-options.txt | 4 list-objects.c | 4 ++-- revision.c | 6 ++ revision.h | 1 + 5 files changed, 15 insertions(+), 3 deletions(-) diff --git a/Documentation/git-rev-list.txt b/Documentation/git-rev-list.txt index fd7f8b5..5b11922 100644 --- a/Documentation/git-rev-list.txt +++ b/Documentation/git-rev-list.txt @@ -46,7 +46,8 @@ SYNOPSIS [ \--extended-regexp | -E ] [ \--fixed-strings | -F ] [ \--date=(local|relative|default|iso|iso-strict|rfc|short) ] -[ [\--objects | \--objects-edge] [ \--unpacked ] ] +[ [ \--objects | \--objects-edge | \--objects-edge-aggressive ] + [ \--unpacked ] ] [ \--pretty | \--header ] [ \--bisect ] [ \--bisect-vars ] diff --git a/Documentation/rev-list-options.txt b/Documentation/rev-list-options.txt index 2277fcb..8cb6f92 100644 --- a/Documentation/rev-list-options.txt +++ b/Documentation/rev-list-options.txt @@ -657,6 +657,10 @@ These options are mostly targeted for packing of Git repositories. objects in deltified form based on objects contained in these excluded commits to reduce network traffic. +--objects-edge-aggressive:: + Similar to `--objects-edge`, but it tries harder to find excluded + commits at the cost of increased time. + --unpacked:: Only useful with `--objects`; print the object IDs that are not in packs. diff --git a/list-objects.c b/list-objects.c index 2910bec..2a139b6 100644 --- a/list-objects.c +++ b/list-objects.c @@ -157,7 +157,7 @@ void mark_edges_uninteresting(struct rev_info *revs, show_edge_fn show_edge) if (commit-object.flags UNINTERESTING) { mark_tree_uninteresting(commit-tree); - if (revs-edge_hint !(commit-object.flags SHOWN)) { + if (revs-edge_hint_aggressive !(commit-object.flags SHOWN)) { commit-object.flags |= SHOWN; show_edge(commit); } @@ -165,7 +165,7 @@ void mark_edges_uninteresting(struct rev_info *revs, show_edge_fn show_edge) } mark_edge_parents_uninteresting(commit, revs, show_edge); } - if (revs-edge_hint) { + if (revs-edge_hint_aggressive) { for (i = 0; i revs-cmdline.nr; i++) { struct object *obj = revs-cmdline.rev[i].item; struct commit *commit = (struct commit *)obj; diff --git a/revision.c b/revision.c index 75dda92..753dd2f 100644 --- a/revision.c +++ b/revision.c @@ -1853,6 +1853,12 @@ static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg revs-tree_objects = 1; revs-blob_objects = 1; revs-edge_hint = 1; + } else if (!strcmp(arg, --objects-edge-aggressive)) { + revs-tag_objects = 1; + revs-tree_objects = 1; + revs-blob_objects = 1; + revs-edge_hint = 1; + revs-edge_hint_aggressive = 1; } else if (!strcmp(arg, --verify-objects)) { revs-tag_objects = 1; revs-tree_objects = 1; diff --git a/revision.h b/revision.h index 9cb5adc..033a244 100644 --- a/revision.h +++ b/revision.h @@ -93,6 +93,7 @@ struct rev_info { blob_objects:1, verify_objects:1, edge_hint:1, + edge_hint_aggressive:1, limited:1, unpacked:1, boundary:2, -- 2.2.1.209.g41e5f3a -- To unsubscribe from this list: send the line unsubscribe git in the body of a
Re: [PATCH 16/18] fsck: support demoting errors to warnings
Johannes Schindelin johannes.schinde...@gmx.de writes: Okay, so just to clarify: you want me to - split the parser into - a parser that accepts only camelCased variable names when they come from the config (for use in fsck and receive-pack), and OK. - another parser that rejects camelCased variable names and only accepts lower-case-dashed, intended for command-line parsing in fsck, index-pack and unpack-objects, and - consequently have a converter from the camelCased variable names we receive from the config in receive-pack so we can pass lower-case-dashed settings to index-pack and unpack-objects. I am not sure about the latter two. This needs a design discussion what the command line options should be. I think the command line should be like this: git cmd --warn=missingTags,missingAuthor in the first place, i.e. we may invent tokens to denote new kinds of errors as we improve fsck, not with we may add options for new kinds of errors, i.e. the command line should not look like this: git cmd --missing-tags=warn --missing-author=warn And from that point of view, I see no reason to support the dashed variant anywhere in the code, neither in the config parser or in the command line parser. -- 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 16/18] fsck: support demoting errors to warnings
Hi Junio, On Tue, 23 Dec 2014, Junio C Hamano wrote: Johannes Schindelin johannes.schinde...@gmx.de writes: Okay, so just to clarify: you want me to - split the parser into - a parser that accepts only camelCased variable names when they come from the config (for use in fsck and receive-pack), and OK. - another parser that rejects camelCased variable names and only accepts lower-case-dashed, intended for command-line parsing in fsck, index-pack and unpack-objects, and - consequently have a converter from the camelCased variable names we receive from the config in receive-pack so we can pass lower-case-dashed settings to index-pack and unpack-objects. I am not sure about the latter two. This needs a design discussion what the command line options should be. I think the command line should be like this: git cmd --warn=missingTags,missingAuthor Okay. This contradicts the convention where Git uses lower-case-dashed command-line option values (e.g. on-demand, error-all, etc) and no camelCased options were present so far. But your wish is my command. Ciao, Dscho in the first place, i.e. we may invent tokens to denote new kinds of errors as we improve fsck, not with we may add options for new kinds of errors, i.e. the command line should not look like this: git cmd --missing-tags=warn --missing-author=warn And from that point of view, I see no reason to support the dashed variant anywhere in the code, neither in the config parser or in the command line parser. -- 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 16/18] fsck: support demoting errors to warnings
Junio C Hamano gits...@pobox.com writes: Johannes Schindelin johannes.schinde...@gmx.de writes: Okay, so just to clarify: you want me to - split the parser into - a parser that accepts only camelCased variable names when they come from the config (for use in fsck and receive-pack), and OK. - another parser that rejects camelCased variable names and only accepts lower-case-dashed, intended for command-line parsing in fsck, index-pack and unpack-objects, and - consequently have a converter from the camelCased variable names we receive from the config in receive-pack so we can pass lower-case-dashed settings to index-pack and unpack-objects. I am not sure about the latter two. This needs a design discussion what the command line options should be. I think the command line should be like this: git cmd --warn=missingTags,missingAuthor in the first place, i.e. we may invent tokens to denote new kinds of errors as we improve fsck, not with we may add options for new kinds of errors, i.e. the command line should not look like this: git cmd --missing-tags=warn --missing-author=warn And from that point of view, I see no reason to support the dashed variant anywhere in the code, neither in the config parser or in the command line parser. Having said that, I think missingTags etc. should not be configuration variable names (instead, they should be values). Because of that, I do not think we need consistency between the way these tokens that denote kinds of errors fsck denotes are spelled and the way configuration variable names are spelled. In other words, I do not think there is nothing that comes from how our configuration variable names are spelled that gives preference to one over the other between the two styles: (1) Tokens are camelCased [fsck] warn = missingTagger,missingAuthor error = zeroPadTreeEntry $ git cmd --warn=missingTagger,missingAuthor (2) Tokens are dashed-multi-words [fsck] warn = missing-tagger,missing-author error = zero-pad-tree-entry $ git cmd --warn=missing-tagger,missing-author Do I have a strong preference between these two? Not really. My gut reaction is that (2) may be easier to read, but I can be persuaded either way. Who else has/had opinions on this? Earlier you said that the configuration the other way, i.e. [fsck] warn = missingTag, was shot down---who did shoot it? Perhaps that person may be able to point out where in my thinking above I am going in the wrong direction. Thanks. [Footnote] In either case, I'd recommend that we take [ ,]+ as inter-token separator to ease the use on the command line and config file, to allow these: [fsck] warn = missingTagger missingAuthor warn = missingTagger,missingAuthor $ git cmd --warn missingTagger,missingAuthor $ git cmd --warn missingTagger missingAuthor -- 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 04/18] Offer a function to demote fsck errors to warnings
On Tue, Dec 23, 2014 at 9:28 AM, Johannes Schindelin johannes.schinde...@gmx.de wrote: Johannes Schindelin johannes.schinde...@gmx.de writes: And *still*, this is *just* a global table with defaults. I would *still* need to copy-on-write when the first customization of the severity level takes place because I cannot allow the global defaults to be modified by one caller (that would defeat the whole purpose of having per-caller settings bundled in the fsck_options struct). There is no alloc. Right now, the initialization reads: struct fsck_options options = strict ? FSCK_OPTIONS_STRICT : FSCK_OPTIONS_DEFAULT; Then it is just the matter of having fsck_options_init(options); if (strict) options.some_field = make_it_strict; as the first few statements, no? I am not sure why it is so difficult -- 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 16/18] fsck: support demoting errors to warnings
Hi Junio, On Tue, 23 Dec 2014, Junio C Hamano wrote: Having said that, I think missingTags etc. should not be configuration variable names (instead, they should be values). Because of that, I do not think we need consistency between the way these tokens that denote kinds of errors fsck denotes are spelled and the way configuration variable names are spelled. Okay. That makes more sense. Now I can remove the complexity introduced by teaching the parser to accept camelCased values, and we're golden. In either case, I'd recommend that we take [ ,]+ as inter-token separator to ease the use on the command line and config file And this is indeed the case: +void fsck_strict_mode(struct fsck_options *options, const char *mode) +... + while (*mode) { + int len = strcspn(mode, ,|), equal, msg_id; +... In other words, I even allowed the pipe symbol as separator. Ciao, Dscho -- 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] is_hfs_dotgit: loosen over-eager match of \u{..47}
Torsten Bögershausen tbo...@web.de writes: Don't we have the same possible problem under NTFS? Under Linux + VFAT ? Under all OS + VFAT ? I do not think so, as NTFS codepath does not use the (previouly too wide but now fixed) is_hfs_dotgit() in the first place, and is_ntfs_dotgit() does not read one unicode codepoint at a time if I remember correctly. And what happens if I export NTFS to Mac OS X? (Other combinations possible) Shouldn't fsck under all OS warn for NTFS and hfs possible attacks ? Doesn't it? Have you read the whole series you are commenting on? -- 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] is_hfs_dotgit: loosen over-eager match of \u{..47}
On Tue, Dec 23, 2014 at 04:24:57PM +0100, Torsten Bögershausen wrote: Don't we have the same possible problem under NTFS? Under Linux + VFAT ? Under all OS + VFAT ? I'm not sure what you mean. This code path is _only_ about checking for HFS+-specific problems. We check general case-insensitivity in another code path. And we check NTFS-specific problems in another code path. (Technically we could make a check all function that runs over the data only once, which would be more efficient. But doing it this way makes the code much easier to follow). And would it make sense to turn this return (out 0xff00) ? out : tolower(out); into this: static ucs_char_t unicode_tolower(ucs_char_t ch) { return (ch 0xff00) ? ch : tolower(ch); } Perhaps, but you would need a big warning at the top of that function that is _only_ downcasing ASCII characters. I.e., it is fine for our use here, but you would not want other unicode-aware code to call it. The next_hfs_char already has such a warning at the top. And what happens if I export NTFS to Mac OS X? (Other combinations possible) Shouldn't fsck under all OS warn for NTFS and hfs possible attacks ? Fsck already warns for all system types, no matter what platform you're on. And we do case-insensitive blocking of .git entering the index for all platforms. We don't turn on filesystem-specific blocking of paths entering the index on all platforms. You get HFS+ blocking by default on OS X, and NTFS on Windows. If you are using HFS+ on Linux, you should set core.protectHFS. Possibly we should just turn on all checks everywhere. That would be a safer default, at the cost to Linux folks of: 1. Some slight inefficiency in each read-tree, as we have to do extra processing on each name. 2. Some names would be disallowed that are otherwise OK. For the most part these are not names that would be used in practice, though (e.g., losing the ability to create .git\u200c is not a big loss to anyone). I think Git for Windows has started blocking other stuff like CON that is not specifically related to .git, though, and that is more plausible for somebody to use in real life. -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 04/18] Offer a function to demote fsck errors to warnings
Hi Junio, On Tue, 23 Dec 2014, Junio C Hamano wrote: On Tue, Dec 23, 2014 at 9:28 AM, Johannes Schindelin johannes.schinde...@gmx.de wrote: Johannes Schindelin johannes.schinde...@gmx.de writes: And *still*, this is *just* a global table with defaults. I would *still* need to copy-on-write when the first customization of the severity level takes place because I cannot allow the global defaults to be modified by one caller (that would defeat the whole purpose of having per-caller settings bundled in the fsck_options struct). There is no alloc. Right now, the initialization reads: struct fsck_options options = strict ? FSCK_OPTIONS_STRICT : FSCK_OPTIONS_DEFAULT; Then it is just the matter of having fsck_options_init(options); if (strict) options.some_field = make_it_strict; as the first few statements, no? I am not sure why it is so difficult It is not difficult. But I try to avoid complexity when I can. Since you asked specifically, I will introduce it, though. Hopefully still this year (I'll not be available for a while starting tomorrow). Ciao, Dscho -- 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] Use wc instead of awk to count subtrees in t0090-cache-tree
Jonathan Nieder jrnie...@gmail.com writes: With the updated subject, Reviewed-by: Jonathan Nieder jrnie...@gmail.com Thanks. Here is what I tentatively queued for today's pushout. -- 8 -- From: Ben Walton bdwal...@gmail.com Date: Mon, 22 Dec 2014 15:25:44 -0800 Subject: [PATCH] t0090: tweak awk statement for Solaris /usr/xpg4/bin/awk The awk statements previously used in this test weren't compatible with the native versions of awk on Solaris: echo dir | /bin/awk -v c=0 '$1 {++c} END {print c}' awk: syntax error near line 1 awk: bailing out near line 1 echo dir | /usr/xpg4/bin/awk -v c=0 '$1 {++c} END {print c}' 0 Even though we do not cater to tools in /usr/bin on Solaris that are overridden by corresponding ones in /usr/xpg?/bin, in this case, even the XPG version does not work correctly. With GNU awk for comparison: echo dir | /opt/csw/gnu/awk -v c=0 '$1 {++c} END {print c}' 1 which is what this test expects (and is in line with POSIX; non-empty string is true and an empty string is false). Work this issue around by using $1 != to state more explicitly that we are skipping empty lines. Helped-by: Jonathan Nieder jrnie...@gmail.com Signed-off-by: Ben Walton bdwal...@gmail.com Reviewed-by: Jonathan Nieder jrnie...@gmail.com Signed-off-by: Junio C Hamano gits...@pobox.com --- t/t0090-cache-tree.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/t/t0090-cache-tree.sh b/t/t0090-cache-tree.sh index 067f4c6..601d02d 100755 --- a/t/t0090-cache-tree.sh +++ b/t/t0090-cache-tree.sh @@ -22,7 +22,7 @@ generate_expected_cache_tree_rec () { # ls-files might have foo/bar, foo/bar/baz, and foo/bar/quux # We want to count only foo because it's the only direct child subtrees=$(git ls-files|grep /|cut -d / -f 1|uniq) - subtree_count=$(echo $subtrees|awk -v c=0 '$1 {++c} END {print c}') + subtree_count=$(echo $subtrees|awk -v c=0 '$1 != {++c} END {print c}') entries=$(git ls-files|wc -l) printf SHA $dir (%d entries, %d subtrees)\n $entries $subtree_count for subtree in $subtrees -- 2.2.1-321-gd161b79 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] git remote add: allow re-adding remotes with the same URL
Hi Junio, [re-Cc:ing the list] On Tue, 23 Dec 2014, Junio C Hamano wrote: On Tue, Dec 23, 2014 at 5:25 AM, Johannes Schindelin johannes.schinde...@gmx.de wrote: On Mon, 22 Dec 2014, Junio C Hamano wrote: So, it is an error if we have remote and if (1) URL for the remote is defined already twice or more; or (2) we are adding a nickname (i.e. not a URL) and it is different from what we already have; or (3) we already have fetch_refspec The way I read the log message's rationale was that this is to allow replacing an existing remote's URL; wouldn't checking the existence of fetch_refspec go against that goal? Puzzled. Either the code is wrong or I am mislead by the explanation in the log. I hope v2 addresses your concerns. Unfortunately I am still confused. The way the overlong line is folded in the new version of the patch makes it easier to read, but I didn't find a reason why checking the number of fetch_refspec does not go against that goal there. Since you pointed out rightfully that the main goal is *not* to allow multiple `git remote add` to succeed when they try to add the same remote with the same URL, I changed the commit message to point out that the goal was to handle adding remotes via `git remote add nick url` when url.url.insteadOf = nick already exists, with the same nick and url. Since I have no interest in opening the can of worms to allow re-adding remotes, I did not touch that part at all, I hope you do not mind. Ciao, Dscho -- 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/4] Improve push performance with lots of refs
brian m. carlson sand...@crustytoothpaste.net writes: The only change from v2 is the addition of a fourth patch, which fixes t5500. It's necessary because the test wants packs for fetches to shallow clones to be minimal. I'm not especially thrilled with having to provide a --shallow command line argument, but the alternative is to buffer a potentially large amount of data in order to determine whether the remote side is shallow. You spell --thin-aggressive as two words, --thin --shallow, in this series, essentially, no? I think this is going in the right direction. The shallow propagated on the wire from the fetcher is the right thing to use to make this decision. I wonder if the call to is_repository_shallow() is still necessary (read: I would prefer to see it go away) where we decide between --objects-edge and --objects-edge-aggressive. Here is the relevant part from 4/4: @@ -2711,7 +2714,7 @@ int cmd_pack_objects(int argc, const char **argv, const char *prefix) argv_array_push(rp, pack-objects); if (thin) { use_internal_rev_list = 1; - argv_array_push(rp, is_repository_shallow() + argv_array_push(rp, is_repository_shallow() || shallow ? --objects-edge-aggressive : --objects-edge); } else -- 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 2/4] rev-list: add an option to mark fewer edges as uninteresting
On Tue, Dec 23, 2014 at 12:55:48PM -0500, Michael Blume wrote: This patch causes an error on my mac, test 5500 fetch-pack errors on part 44 - fetch creating new shallow root. It looks for remote: Total 1 in the fetch output and gets 3 instead. It fails for me on Linux, too. Interestingly the tip of the series passes. I didn't investigate 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
[PATCH] git-new-workdir: support submodules
The basic problem with submodules, from git-new-workdir's point of view, is that instead of having a .git directory, they have a .git file with contents `gitdir: some other path`. This is a problem because the submodule's config file has an entry like `worktree = ../../../khan-exercises` which is relative to some other path rather than to submodule_dir/.git. As a result, if we want the new workdir to work properly, it needs to keep the same directory structure as the original repository: it should also contain a .git file with a 'gitdir', and the actual .git contents should be in the place mentioned therein. (There are other ways we could have solved this problem, including modifying the 'config' file, but this seemed most in keeping with the symlink-y philosophy of git-new-branch.) This commit implements this, by detecting if the source .git directory is actually a .git file with 'gitdir: ...' as its contents, and if so reproducing the .git file + gitdir structure in the destination work-tree. Test plan: On a repo (~/khan/webapp) with a submodule, ran git new-workdir ~/khan/webapp /tmp/webapp # the main module git new-workdir ~/khan/webapp/khan-exercises /tmp/webapp/khan-exercises and saw that /tmp/webapp/khan-exercises was populated correctly, /tmp/webapp/.git/modules/khan-exercises existed with symlinks, and /tmp/webapp/khan-exercises/.git was a file with a 'gitdir:' entry pointing to the .git/modules directory. Signed-off-by: Craig Silverstein csilv...@khanacademy.org --- contrib/workdir/git-new-workdir | 55 +++-- 1 file changed, 36 insertions(+), 19 deletions(-) diff --git a/contrib/workdir/git-new-workdir b/contrib/workdir/git-new-workdir index 888c34a..3cc50de 100755 --- a/contrib/workdir/git-new-workdir +++ b/contrib/workdir/git-new-workdir @@ -52,26 +52,45 @@ then a complete repository. fi +# don't modify an existing directory, unless it's empty +if test -d $new_workdir test $(ls -a1 $new_workdir/. | wc -l) -ne 2 +then + die destination directory '$new_workdir' is not empty. +fi + # make sure the links in the workdir have full paths to the original repo git_dir=$(cd $git_dir pwd) || exit 1 -# don't recreate a workdir over an existing directory, unless it's empty -if test -d $new_workdir +new_git_dir=$new_workdir/.git + +# if $orig_git is a .git file with a 'gitdir' entry (as is the case for +# submodules), have the new git dir follow that same pattern. otherwise +# the 'worktree' entry in .git/config, which is a relative path, will +# not resolve properly because we're not in the expected subdirectory. +gitdir_text=$(sed -ne 's/^gitdir: *//p' $orig_git/.git 2/dev/null) +if test -n $gitdir_text; then + ln -s $orig_git/.git $new_workdir/.git || failed + new_git_dir=$new_workdir/$gitdir_text +fi + +# if new_workdir already exists, leave it along in case of error +if ! test -d $new_workdir then - if test $(ls -a1 $new_workdir/. | wc -l) -ne 2 - then - die destination directory '$new_workdir' is not empty. - fi - cleandir=$new_workdir/.git -else - cleandir=$new_workdir + clean_new_workdir=true fi -mkdir -p $new_workdir/.git || failed -cleandir=$(cd $cleandir pwd) || failed +mkdir -p $new_git_dir || failed +cleandir=$(cd $cleandir pwd) || failed cleanup () { - rm -rf $cleandir + if test z$clean_new_workdir = ztrue + then + rm -rf $new_workdir + fi + # this may (or may not) be a noop if new_workdir was already deleted. + rm -rf $new_git_dir + # this is a noop unless .git is a 'gitdir: ...' file. + rm -f $new_workdir/.git } siglist=0 1 2 15 trap cleanup $siglist @@ -84,22 +103,20 @@ do # create a containing directory if needed case $x in */*) - mkdir -p $new_workdir/.git/${x%/*} + mkdir -p $new_git_dir/${x%/*} ;; esac - ln -s $git_dir/$x $new_workdir/.git/$x || failed + ln -s $git_dir/$x $new_git_dir/$x || failed done -# commands below this are run in the context of the new workdir -cd $new_workdir || failed - # copy the HEAD from the original repository as a default branch -cp $git_dir/HEAD .git/HEAD || failed +cp $git_dir/HEAD $new_git_dir/HEAD || failed # the workdir is set up. if the checkout fails, the user can fix it. trap - $siglist # checkout the branch (either the same as HEAD from the original repository, -# or the one that was asked for) +# or the one that was asked for). we must be in the new workdir for this. +cd $new_workdir || failed git checkout -f $branch -- 2.2.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 1/2] git remote add: allow re-adding remotes with the same URL
Johannes Schindelin johannes.schinde...@gmx.de writes: I hope v2 addresses your concerns. Unfortunately I am still confused. The way the overlong line is folded in the new version of the patch makes it easier to read, but I didn't find a reason why checking the number of fetch_refspec does not go against that goal there. Since you pointed out rightfully that the main goal is *not* to allow multiple `git remote add` to succeed when they try to add the same remote with the same URL, I changed the commit message to point out that the goal was to handle adding remotes via `git remote add nick url` when url.url.insteadOf = nick already exists, with the same nick and url. Since I have no interest in opening the can of worms to allow re-adding remotes, I did not touch that part at all, I hope you do not mind. Oh, don't get me wrong. I do not have any interest in allowing re-adding remotes, either, so we are on the same page. I was just saying that it was unclear what ensures that the change in the codepath only affects the remote setting that conflicts with insteadOf without randomly (read: sometimes it allows, sometimes it forbids) allowing re-adding remotes. -- 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 16/18] fsck: support demoting errors to warnings
Johannes Schindelin johannes.schinde...@gmx.de writes: Hi Junio, On Tue, 23 Dec 2014, Junio C Hamano wrote: Having said that, I think missingTags etc. should not be configuration variable names (instead, they should be values). Because of that, I do not think we need consistency between the way these tokens that denote kinds of errors fsck denotes are spelled and the way configuration variable names are spelled. Okay. That makes more sense. I am sorry that I didn't step back and think about it earlier to notice that we shouldn't be talking about configuration variable name syntax. I could have saved us time going back and forth if I did so earlier. -- 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
git: regression with mergetool and answering n (backport fix / add tests)
Hi, this is in reply to the commits from David: commit 0ddedd4d6b9b3e8eb3557d8ed28e1a0b354a25f8 Refs: v2.2.0-60-g0ddedd4 Merge: e886efd 1e86d5b Author: Junio C Hamano gits...@pobox.com AuthorDate: Fri Dec 12 14:31:39 2014 -0800 Commit: Junio C Hamano gits...@pobox.com CommitDate: Fri Dec 12 14:31:39 2014 -0800 Merge branch 'da/difftool-mergetool-simplify-reporting-status' Code simplification. * da/difftool-mergetool-simplify-reporting-status: mergetools: stop setting $status in merge_cmd() mergetool: simplify conditionals difftool--helper: add explicit exit statement mergetool--lib: remove use of $status global mergetool--lib: remove no-op assignment to $status from setup_user_tool I've ran into a problem, where git mergetool (using vimdiff) would add the changes to the index, although you'd answered n after not changing/saving the merged file. This regression has been introduced in: commit 99474b6340dbcbe58f6c256fdee231cbadb060f4 Author: David Aguilar dav...@gmail.com Date: Fri Nov 14 13:33:55 2014 -0800 difftool: honor --trust-exit-code for builtin tools run_merge_tool() was not setting $status, which prevented the exit code for builtin tools from being forwarded to the caller. Capture the exit status and add a test to guarantee the behavior. Reported-by: Adria Farres 14farr...@gmail.com Signed-off-by: David Aguilar dav...@gmail.com Signed-off-by: Junio C Hamano gits...@pobox.com diff --git a/git-mergetool--lib.sh b/git-mergetool--lib.sh index c45a020..cce4f8c 100644 --- a/git-mergetool--lib.sh +++ b/git-mergetool--lib.sh @@ -221,6 +221,7 @@ run_merge_tool () { else run_diff_cmd $1 fi + status=$? return $status } My fix has been the following, but I agree that the changes from David are much better in general. diff --git a/git-mergetool--lib.sh b/git-mergetool--lib.sh index cce4f8c..fa9acb1 100644 --- a/git-mergetool--lib.sh +++ b/git-mergetool--lib.sh @@ -105,6 +105,7 @@ check_unchanged () { esac done fi + return $status } valid_tool () { I haven't verified if it really fixes the regression, but if it does it should get backported into the branches where the regression is present. Also, there should be some tests for this. I've came up with the following, which is not finished and left in a state of debugging why it did not trigger the bug. In t/t7610-mergetool.sh: test_expect_success 'return code with untrusted mergetool' ' # git config merge.tool untrusted # git config mergetool.untrusted.cmd true # git config mergetool.untrusted.trustExitCode false test_config mergetool.vimdiff.cmd cat \\$REMOTE\ git config merge.tool vimdiff git config --get-regexp mergetool. git config --get-regexp merge\.tool. git checkout -b test_untrusted branch1 test_must_fail git merge master /dev/null 21 git status -s test $(git status --short both) = AA both ( echo n | test_must_fail git mergetool both ) git status -s test $(git status --short both) = AA both ( echo y | git mergetool both ) git status -s test $(git status --short both) = M both git reset --hard git config merge.tool mytool Cheers, Daniel. -- http://daniel.hahler.de/ signature.asc Description: OpenPGP digital signature
Re: [PATCH v3 2/4] rev-list: add an option to mark fewer edges as uninteresting
Jeff King p...@peff.net writes: On Tue, Dec 23, 2014 at 12:55:48PM -0500, Michael Blume wrote: This patch causes an error on my mac, test 5500 fetch-pack errors on part 44 - fetch creating new shallow root. It looks for remote: Total 1 in the fetch output and gets 3 instead. It fails for me on Linux, too. Interestingly the tip of the series passes. I didn't investigate further. Not surprised, as the first three are the same from the yesterday's one that failed for everybody. -- 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] git-new-workdir: support submodules
Craig Silverstein csilvers.m...@gmail.com writes: The basic problem with submodules, from git-new-workdir's point of view, is that instead of having a .git directory, they have a .git file with contents `gitdir: some other path`. This is a problem because the submodule's config file has an entry like `worktree = ../../../khan-exercises` which is relative to some other path rather than to submodule_dir/.git. As a result, if we want the new workdir to work properly, it needs to keep the same directory structure as the original repository: it should also contain a .git file with a 'gitdir', and the actual .git contents should be in the place mentioned therein. H, does that mean that the submodule S in the original repository O's working tree and its checkout in the secondary working tree W created from O using git-new-workdir share the same repository location? More specifically: O/.git/ - original repository O/.git/index- worktree state in O O/S - submodule S's checkout in O O/S/.git- a gitfile pointing to O/.git/modules/S O/.git/modules/S- submodule S's repository contents O/.git/modules/S/config - submodule S's config W/.git/ - secondary working tree W/.git/config - symlink to O/.git/config W/.git/index- worktree state in W (independent of O) W/S - submodule S's checkout in W (independent of O) W/S/.git- a gitfile pointing to O/.git/modules/S Doesn't a submodule checkout keep some state tied to the working tree in its repository configuration file? Wouldn't this change introduce problems by sharing O/.git/modules/S/config between the two checkouts? -- 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 16/18] fsck: support demoting errors to warnings
Hi Junio, On Tue, 23 Dec 2014, Junio C Hamano wrote: Johannes Schindelin johannes.schinde...@gmx.de writes: On Tue, 23 Dec 2014, Junio C Hamano wrote: Having said that, I think missingTags etc. should not be configuration variable names (instead, they should be values). Because of that, I do not think we need consistency between the way these tokens that denote kinds of errors fsck denotes are spelled and the way configuration variable names are spelled. Okay. That makes more sense. I am sorry that I didn't step back and think about it earlier to notice that we shouldn't be talking about configuration variable name syntax. I could have saved us time going back and forth if I did so earlier. Do not worry. You were just trying to make this software better, same as I tried. Unfortunately, I will not be able to submit v2 of this patch series this year, but I will do so in the second week of January (including the change to the global array with the default severity levels because I do want to see this feature integrated). Ciao, Dscho -- 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] is_hfs_dotgit: loosen over-eager match of \u{..47}
Jeff King wrote: Signed-off-by: Jeff King p...@peff.net --- t/t1450-fsck.sh | 16 utf8.c | 17 - 2 files changed, 28 insertions(+), 5 deletions(-) Mph. Sorry I missed this before. Reviewed-by: Jonathan Nieder jrnie...@gmail.com [...] +++ b/t/t1450-fsck.sh [...] + git fsck 2err + cat err + ! test -s err Nit: if this said test_line_count = 0 err then the error message would be more obvious when it fails with --verbose. Thanks, Jonathan -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] is_hfs_dotgit: loosen over-eager match of \u{..47}
Am 23.12.2014 um 09:45 schrieb Jeff King: @@ -606,7 +606,7 @@ static ucs_char_t next_hfs_char(const char **in) * but this is enough to catch anything that will convert * to .git */ - return tolower(out); + return out; Did you consider changing the comment that we see in the pre-context here? -- Hannes -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] is_hfs_dotgit: loosen over-eager match of \u{..47}
Jonathan Nieder jrnie...@gmail.com writes: +++ b/t/t1450-fsck.sh [...] +git fsck 2err +cat err +! test -s err Nit: if this said test_line_count = 0 err then the error message would be more obvious when it fails with --verbose. That's a good suggestion, I think. This is meant to apply on top of d08c13b, and we already had test_line_count back then. So far I collected these follow-ups to squash into Peff's patch. t/t1450-fsck.sh | 3 +-- utf8.c | 15 --- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/t/t1450-fsck.sh b/t/t1450-fsck.sh index 5e42385..0279b2b 100755 --- a/t/t1450-fsck.sh +++ b/t/t1450-fsck.sh @@ -284,8 +284,7 @@ test_expect_success 'fsck allows .Ňit' ' printf 100644 blob $blob\t.\\305\\207it tree tree=$(git mktree tree) git fsck 2err - cat err - ! test -s err + test_line_count = 0 err ) ' diff --git a/utf8.c b/utf8.c index 18a8f42..9c9fa3a 100644 --- a/utf8.c +++ b/utf8.c @@ -630,8 +630,8 @@ int mbs_chrlen(const char **text, size_t *remainder_p, const char *encoding) } /* - * Pick the next char from the stream, folding as an HFS+ filename comparison - * would. Note that this is _not_ complete by any means. It's just enough + * Pick the next char from the stream, ignoring codepoints an HFS+ would. + * Note that this is _not_ complete by any means. It's just enough * to make is_hfs_dotgit() work, and should not be used otherwise. */ static ucs_char_t next_hfs_char(const char **in) @@ -668,11 +668,6 @@ static ucs_char_t next_hfs_char(const char **in) continue; } - /* -* there's a great deal of other case-folding that occurs, -* but this is enough to catch anything that will convert -* to .git -*/ return out; } } @@ -685,6 +680,12 @@ int is_hfs_dotgit(const char *path) if (c != '.') return 0; c = next_hfs_char(path); + + /* +* there's a great deal of other case-folding that occurs +* in HFS+, but this is enough to catch anything that will +* convert to .git +*/ if (c != 'g' c != 'G') return 0; c = next_hfs_char(path); -- 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] is_hfs_dotgit: loosen over-eager match of \u{..47}
On Tue, Dec 23, 2014 at 12:14:16PM -0800, Jonathan Nieder wrote: +++ b/t/t1450-fsck.sh [...] + git fsck 2err + cat err + ! test -s err Nit: if this said test_line_count = 0 err then the error message would be more obvious when it fails with --verbose. Thanks, I actually considered adding a test_file_is_empty for that reason, but empty line-count does the same thing. -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] is_hfs_dotgit: loosen over-eager match of \u{..47}
On Tue, Dec 23, 2014 at 09:31:10PM +0100, Johannes Sixt wrote: Am 23.12.2014 um 09:45 schrieb Jeff King: @@ -606,7 +606,7 @@ static ucs_char_t next_hfs_char(const char **in) * but this is enough to catch anything that will convert * to .git */ - return tolower(out); + return out; Did you consider changing the comment that we see in the pre-context here? I did consider it, but the comment that is there was actually written for the _original_ version, before I added tolower in the first place (it also applied equally to the tolower() version, so I left it). So it was clear either way, at least in my brain. :) Of course that does not say anything about people's brains who did not write the patch. The changes Junio suggested elsewhere in the thread do make it more clear. -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] is_hfs_dotgit: loosen over-eager match of \u{..47}
On Tue, Dec 23, 2014 at 01:02:23PM -0800, Junio C Hamano wrote: Jonathan Nieder jrnie...@gmail.com writes: +++ b/t/t1450-fsck.sh [...] + git fsck 2err + cat err + ! test -s err Nit: if this said test_line_count = 0 err then the error message would be more obvious when it fails with --verbose. That's a good suggestion, I think. This is meant to apply on top of d08c13b, and we already had test_line_count back then. So far I collected these follow-ups to squash into Peff's patch. They all look good to me. Thanks. -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 16/18] fsck: support demoting errors to warnings
Johannes Schindelin johannes.schinde...@gmx.de writes: Unfortunately, I will not be able to submit v2 of this patch series this year, but I will do so in the second week of January (including the change to the global array with the default severity levels because I do want to see this feature integrated). Heh, we are not in a hurry. Enjoy your holidays. A happy new year to you in advance ;-) -- 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] git-new-workdir: support submodules
[Ack, I forgot to cc myself on the original patch so now I can't reply to it normally. Hopefully my workaround doesn't mess up the threading too badly.] Junio C Hamano gitster at pobox.com writes: H, does that mean that the submodule S in the original repository O's working tree and its checkout in the secondary working tree W created from O using git-new-workdir share the same repository location? More specifically: O/.git/ - original repository O/.git/index- worktree state in O O/S - submodule S's checkout in O O/S/.git- a gitfile pointing to O/.git/modules/S O/.git/modules/S- submodule S's repository contents O/.git/modules/S/config - submodule S's config W/.git/ - secondary working tree W/.git/config - symlink to O/.git/config W/.git/index- worktree state in W (independent of O) W/S - submodule S's checkout in W (independent of O) W/S/.git- a gitfile pointing to O/.git/modules/S Right until the last line. The .git file holds a relative path (at least, it always does in my experience), so W/S/.git will point to W/.git/modules/S. Also, to complete the story, my changes sets the following: W/.git/modules/S- secondary working tree for S W/.git/modules/S/config - symlink to O/.git/modules/S/config W/.git/modules/S/index- worktree state in W's S (independent of O and O's S) Doesn't a submodule checkout keep some state tied to the working tree in its repository configuration file? Do you mean, in 'config' itself? If so, I don't see it. (Though it's possible there are ways to use submodules that do keep working-tree state in the config file, and we just happen not to use those ways.) Here's what my webapp/.git/modules/khan-exercises/config looks like: --- [core] repositoryformatversion = 0 filemode = true bare = false logallrefupdates = true worktree = ../../../khan-exercises [remote origin] url = http://github.com/Khan/khan-exercises.git fetch = +refs/heads/*:refs/remotes/origin/* [branch master] remote = origin merge = refs/heads/master rebase = true [submodule test/qunit] url = https://github.com/jquery/qunit.git -- The only thing that seems vaguely working-tree related is the 'worktree' field, which is the field that motivated me to set up my patch the way it is. Wouldn't this change introduce problems by sharing O/.git/modules/S/config between the two checkouts? It's true that this change does result in sharing that file, so if that's problematic then you're right. I'm afraid I don't know all the things that can go into a submodule config file. (There are other things I don't know as well, such as: do we see .git files with 'gitdir: ...' contents only for submodules, or are there other ways to create them as well? Are 'gitdir' paths always relative? Are there special files in .git (or rather .git/modules/S) that exist only for submodules and not for 'normal' repos, that we need to worry about symlinking? I apologize for not knowing all these git internals, and hope you guys can help point out any gaps that affect this patch!) craig -- 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
Gitk Branch Ordering
Hi, I have noticed that gitk does not order the branches it draws the same way as 'git log --graph' 'git log' seems to keep your current branch as the leftmost line. In the below screenshot here (https://drive.google.com/open?id=0B9b8DuGRceMSTHgzV3dBZkxiTVUauthuser=0) master is the current branch. It is the leftmost branch and we can see that at faf2797 someone made a branch and later merged it back into master. This is shown as a bump out to the right. However in gitk this is not the case. In the shot here (https://drive.google.com/open?id=0B9b8DuGRceMSNUJjbEhGaDZXeTgauthuser=0) the same commits are shown but the bump is out to the left. Master is in the middle of a number of branches. This is counter intuitive (to me at least). I can't seem to find any setting or argument that will have gitk behave the same as 'git log' with regards to branch ordering. Is this possible? Thanks, Nathan Mascitelli -- 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: mangled file names in git checkout-index --temp output when run in repo subdirectory
On Tue, Dec 23, 2014 at 11:36 AM, Russ Cox r...@golang.org wrote: I am using git checkout-index --temp to obtain copies of files from the index, but it does not always print valid file names unless run from the repository root. git checkout-index --temp prints names of files in the index interpreted relative to the current directory below the repository root. If you have a git repo in /tmp/gitbug, you are in /tmp/gitbug/dir1, and you run git checkout-index --temp /tmp/gitbug/dir1/file1, the file listing prints just file1. So far so good. However, this file name shortening appears to assume that all file names being printed will be within the subtree rooted at the current directory, and it just skips over the first N characters in the name, where N is the length of the current directory name relative to the repo root (in the above example, N = len(dir1/) = 5). Indeed, although the checkout functionality of checkout-index properly handles relative paths above the current directory, the code which prints them doesn't. I'm working up a patch. -- 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] merge: release strbuf after use in suggest_conflicts()
Signed-off-by: Rene Scharfe l@web.de --- builtin/merge.c | 1 + 1 file changed, 1 insertion(+) diff --git a/builtin/merge.c b/builtin/merge.c index 215d485..d722889 100644 --- a/builtin/merge.c +++ b/builtin/merge.c @@ -894,6 +894,7 @@ static int suggest_conflicts(void) append_conflicts_hint(msgbuf); fputs(msgbuf.buf, fp); + strbuf_release(msgbuf); fclose(fp); rerere(allow_rerere_auto); printf(_(Automatic merge failed; -- 2.2.1 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] commit-tree: simplify parsing of option -S using skip_prefix()
Signed-off-by: Rene Scharfe l@web.de --- builtin/commit-tree.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/builtin/commit-tree.c b/builtin/commit-tree.c index 8a66c74..25aa2cd 100644 --- a/builtin/commit-tree.c +++ b/builtin/commit-tree.c @@ -66,10 +66,8 @@ int cmd_commit_tree(int argc, const char **argv, const char *prefix) continue; } - if (!memcmp(arg, -S, 2)) { - sign_commit = arg + 2; + if (skip_prefix(arg, -S, sign_commit)) continue; - } if (!strcmp(arg, --no-gpg-sign)) { sign_commit = NULL; -- 2.2.1 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] transport: simplify duplicating a substring in transport_get() using xmemdupz()
Signed-off-by: Rene Scharfe l@web.de --- transport.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/transport.c b/transport.c index 70d38e4..08bcd3a 100644 --- a/transport.c +++ b/transport.c @@ -971,9 +971,7 @@ struct transport *transport_get(struct remote *remote, const char *url) } else { /* Unknown protocol in URL. Pass to external handler. */ int len = external_specification_len(url); - char *handler = xmalloc(len + 1); - handler[len] = 0; - strncpy(handler, url, len); + char *handler = xmemdupz(url, len); transport_helper_init(ret, handler); } -- 2.2.1 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] refs: release strbuf after use in check_refname_component()
Signed-off-by: Rene Scharfe l@web.de --- refs.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/refs.c b/refs.c index 5fcacc6..ed3b2cb 100644 --- a/refs.c +++ b/refs.c @@ -2334,7 +2334,7 @@ static struct ref_lock *lock_ref_sha1_basic(const char *refname, struct strbuf err = STRBUF_INIT; unable_to_lock_message(ref_file, errno, err); error(%s, err.buf); - strbuf_reset(err); + strbuf_release(err); goto error_return; } } -- 2.2.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 5/7] receive-pack: move execute_commands_non_atomic before execute_commands
I tried all four diff options as listed in the man page of format-diff. I forget which one I used, but there was no large difference w.r.t. reviewability if I remember correctly. On Mon, Dec 22, 2014 at 10:19 AM, Junio C Hamano gits...@pobox.com wrote: Stefan Beller sbel...@google.com writes: Notes: This patch is new with v6 of the series As execute_commands_non_atomic is larger than execute_commands, the diff is not moving around execute_commands_non_atomic, but execute_commands. ;-) Next time perhaps try --patience to decide between with and without which one reads better? -- 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
GIT_PUSH_CERT* env vars and update/post-update hooks...
Hi git core devs, Any chance I could persuade you to set the GIT_PUSH_CERT* environment variables for the update (and post-update) hooks also? Background: gitolite takes over the update hook [1] for authorisation and enforcement, and I want to avoid taking over the pre-receive hook also in order to do this check. The post-update is not so important; gitolite doesn't use it anyway, so if I have to take over one of them, I may as well take over post-receive. I just added that for consistency. thanks sitaram [1]: because it's nice to *selectively* reject refs when more than one ref is pushed at the same time; pre-receive is all or none. -- 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] commit-tree: simplify parsing of option -S using skip_prefix()
René Scharfe wrote: Signed-off-by: Rene Scharfe l@web.de --- builtin/commit-tree.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) Reviewed-by: Jonathan Nieder jrnie...@gmail.com Thanks. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] transport: simplify duplicating a substring in transport_get() using xmemdupz()
René Scharfe wrote: Signed-off-by: Rene Scharfe l@web.de --- transport.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) Since external_specification_len(url) is defined to always be less than strlen(url) when is_url(url), this should be safe. Reviewed-by: Jonathan Nieder jrnie...@gmail.com -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: git update-ref --stdin : too many open files
On 22.12.2014 13:22, Junio C Hamano wrote: Loic Dachary l...@dachary.org writes: Hi, Steps to reproduce: $ git --version git version 1.9.1 $ wc -l /tmp/1 9090 /tmp/1 $ head /tmp/1 delete refs/pull/1/head create refs/heads/pull/1 86b715f346e52920ca7c9dfe65424eb9946ebd61 delete refs/pull/1/merge create refs/merges/1 c0633abdc5311354c9729374e0ba25c97a89f69e ... $ ulimit -n 1024 $ git update-ref --stdin /tmp/1 fatal: Unable to create /home/gitmirror/repositories/Ceph/ceph/refs/heads/pull/1917.lock': Too many open files $ head -20 /tmp/1 | git update-ref --stdin $ echo $? 0 The workaround is to increase ulimit -n git update-ref --stdin should probably close some files. Cheers Sounds like the recent ref update in a transaction issue to me. Stefan, want to take a look? I think we do need to keep the .lock files without renaming while in transaction, but we do not have to keep them open, so I suspect that a fix may be to split the commit function into two (one to close but not rename, the other to finalize by renaming) or something. Also the version of transaction series we have queued seem to lock these refs very late in the process, but as we discussed privately a few weeks ago, we would want to move the lock much earlier, when the first update is attempted. So I decided the first thing to do was to put this case into the test suite. so I typed in good faith: test_expect_success 'but does it scale?' ' for i in $(seq 1 1234567) ; do git branch branch_${i} echo delete refs/heads/branch_${i} large_input done git update-ref --stdin large_input ' And there I have problems with my hard disk having more than a million files in one directory. So once I get rid of that I'll come up with a better way to test and fix this problem. Thanks, Stefan -- 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] merge: release strbuf after use in suggest_conflicts()
René Scharfe wrote: Signed-off-by: Rene Scharfe l@web.de --- builtin/merge.c | 1 + 1 file changed, 1 insertion(+) diff --git a/builtin/merge.c b/builtin/merge.c index 215d485..d722889 100644 --- a/builtin/merge.c +++ b/builtin/merge.c @@ -894,6 +894,7 @@ static int suggest_conflicts(void) append_conflicts_hint(msgbuf); fputs(msgbuf.buf, fp); + strbuf_release(msgbuf); fclose(fp); rerere(allow_rerere_auto); printf(_(Automatic merge failed; The caller is about to exit so this is a small one-time leak, but freeing it doesn't cost much and makes analysis by valgrind a little easier. So this seems like a good change. Reviewed-by: Jonathan Nieder jrnie...@gmail.com -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] git-log: added --grep-begin .. --grep-end syntax
This is useful to specify more complicated pattern as with '--grep'. Signed-off-by: Christoph Junghans ott...@gentoo.org --- builtin/grep.c | 73 +- grep.c | 62 + grep.h | 10 revision.c | 56 4 files changed, 134 insertions(+), 67 deletions(-) diff --git a/builtin/grep.c b/builtin/grep.c index 4063882..0127fa0 100644 --- a/builtin/grep.c +++ b/builtin/grep.c @@ -551,67 +551,6 @@ static int context_callback(const struct option *opt, const char *arg, return 0; } -static int file_callback(const struct option *opt, const char *arg, int unset) -{ - struct grep_opt *grep_opt = opt-value; - int from_stdin = !strcmp(arg, -); - FILE *patterns; - int lno = 0; - struct strbuf sb = STRBUF_INIT; - - patterns = from_stdin ? stdin : fopen(arg, r); - if (!patterns) - die_errno(_(cannot open '%s'), arg); - while (strbuf_getline(sb, patterns, '\n') == 0) { - /* ignore empty line like grep does */ - if (sb.len == 0) - continue; - - append_grep_pat(grep_opt, sb.buf, sb.len, arg, ++lno, - GREP_PATTERN); - } - if (!from_stdin) - fclose(patterns); - strbuf_release(sb); - return 0; -} - -static int not_callback(const struct option *opt, const char *arg, int unset) -{ - struct grep_opt *grep_opt = opt-value; - append_grep_pattern(grep_opt, --not, command line, 0, GREP_NOT); - return 0; -} - -static int and_callback(const struct option *opt, const char *arg, int unset) -{ - struct grep_opt *grep_opt = opt-value; - append_grep_pattern(grep_opt, --and, command line, 0, GREP_AND); - return 0; -} - -static int open_callback(const struct option *opt, const char *arg, int unset) -{ - struct grep_opt *grep_opt = opt-value; - append_grep_pattern(grep_opt, (, command line, 0, GREP_OPEN_PAREN); - return 0; -} - -static int close_callback(const struct option *opt, const char *arg, int unset) -{ - struct grep_opt *grep_opt = opt-value; - append_grep_pattern(grep_opt, ), command line, 0, GREP_CLOSE_PAREN); - return 0; -} - -static int pattern_callback(const struct option *opt, const char *arg, - int unset) -{ - struct grep_opt *grep_opt = opt-value; - append_grep_pattern(grep_opt, arg, -e option, 0, GREP_PATTERN); - return 0; -} - static int help_callback(const struct option *opt, const char *arg, int unset) { return -1; @@ -710,21 +649,21 @@ int cmd_grep(int argc, const char **argv, const char *prefix) N_(show the surrounding function)), OPT_GROUP(), OPT_CALLBACK('f', NULL, opt, N_(file), - N_(read patterns from file), file_callback), + N_(read patterns from file), grep_file_callback), { OPTION_CALLBACK, 'e', NULL, opt, N_(pattern), - N_(match pattern), PARSE_OPT_NONEG, pattern_callback }, + N_(match pattern), PARSE_OPT_NONEG, grep_pattern_callback }, { OPTION_CALLBACK, 0, and, opt, NULL, N_(combine patterns specified with -e), - PARSE_OPT_NOARG | PARSE_OPT_NONEG, and_callback }, + PARSE_OPT_NOARG | PARSE_OPT_NONEG, grep_and_callback }, OPT_BOOL(0, or, dummy, ), { OPTION_CALLBACK, 0, not, opt, NULL, , - PARSE_OPT_NOARG | PARSE_OPT_NONEG, not_callback }, + PARSE_OPT_NOARG | PARSE_OPT_NONEG, grep_not_callback }, { OPTION_CALLBACK, '(', NULL, opt, NULL, , PARSE_OPT_NOARG | PARSE_OPT_NONEG | PARSE_OPT_NODASH, - open_callback }, + grep_open_callback }, { OPTION_CALLBACK, ')', NULL, opt, NULL, , PARSE_OPT_NOARG | PARSE_OPT_NONEG | PARSE_OPT_NODASH, - close_callback }, + grep_close_callback }, OPT__QUIET(opt.status_only, N_(indicate hit with exit status without output)), OPT_BOOL(0, all-match, opt.all_match, diff --git a/grep.c b/grep.c index 6e085f8..0c9a977 100644 --- a/grep.c +++ b/grep.c @@ -1796,3 +1796,65 @@ static int grep_source_is_binary(struct grep_source *gs) return 0; } + +int grep_file_callback(const struct option *opt, const char *arg, int unset) +{ + struct grep_opt *grep_opt = opt-value; + int from_stdin = !strcmp(arg, -); + FILE *patterns; + int lno = 0; + struct strbuf sb = STRBUF_INIT; + + patterns = from_stdin ? stdin : fopen(arg, r); + if (!patterns) + die_errno(_(cannot open '%s'),
Re: [PATCH] git-log: added --invert-grep option
Ok, I drafted a first version of the suggest --grep-begin ... --grep-end syntax. However, I could not find a good ways to invert the match on a commit basis instead of the normal line-wise version. Any suggestions? -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/7] receive-pack.c: add protocol support to negotiate atomic-push
On 12/19/2014 08:38 PM, Stefan Beller wrote: From: Ronnie Sahlberg sahlb...@google.com This adds support to the protocol between send-pack and receive-pack to * allow receive-pack to inform the client that it has atomic push capability * allow send-pack to request atomic push back. There is currently no setting in send-pack to actually request that atomic pushes are to be used yet. This only adds protocol capability not ability for the user to activate it. Sorry to jump in so late... If I understand correctly, after this patch the server advertises the atomic capability even though it doesn't actually have that ability until a later patch. It seems to me that the order of the patches should be reversed: don't advertise the capability before it is actually implemented. Why? Bisection. Between the two patches the server is buggy. Michael -- Michael Haggerty mhag...@alum.mit.edu -- 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: Question about installing git from source
Yeah, was wrong install script. thank you. 2014-12-23 23:53 GMT+06:00 Alexander Kuleshov kuleshovm...@gmail.com: I looked at Makefile, and seems that it occurs from here: https://github.com/git/git/blob/master/Makefile#L2205 It tries to copy all files/folders from git root directory but 'install' command prints this error. I tried to execute 'sudo install -d -m 755' in other directory and if there is directory too it prints the same error. For example: ~/scripts $ ls addPPAcleanMailTrash git-autorgit-ranges git-tp install scriptsssh-live term-help tp-git updateMail xray-start buildGit deployWork git-install git-remove-tags git-update-commit-message install.sh ssh-build ssh-wiwob-lab test updateGitDev updateNews test - is directory here ~/scripts $ sudo install -d -m 755 . Copying scripts to /usr/bin cp: omitting directory ‘test’ Done 2014-12-23 23:43 GMT+06:00 Junio C Hamano gits...@pobox.com: Alexander Kuleshov kuleshovm...@gmail.com writes: install -d -m 755 '/usr/bin' Copying scripts to /usr/bin As 'git grep Copying scripts' gives us nothing, the message is obviously not what we are giving. Perhaps you have a strange install in your path that does not understand -d is a way to tell it to make sure a directory exists (by creating one as necessary)? -- _ 0xAX -- _ 0xAX -- 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