Re: [PATCH 1/2] git-credential-store: support XDG config dir
Hi all, Thanks for the review. I apologize for rushing the patch out as I wanted to get feedback on the new behavior before committing to any more code changes. I also wanted to quickly clear any doubts that people may have about my coding ability. (Or maybe it created more ;) ) On Wed, Mar 4, 2015 at 6:00 AM, Matthieu Moy wrote: > Paul Tan writes: >> * get: call lookup_credential() on the XDG file first if it exists. If >> the credential can't be found, call lookup_credential() on the HOME >> file. >> * erase: Call remove_credential() on both the XDG file if it exists and >> the HOME file if it exists. >> * store: If the XDG file exists, call store_credential() on the XDG file >> and remove_credential() on the HOME file to prevent duplicates. >> * If "--file" is provided, use the file for all operations instead. > > When writting a commit message, always insist on _why_ you did what you > did, not _what_ you did (the patch already says it). For example, your > proposal for erase makes sense because if you're using "erase", you > probably don't want to leave cleartext passwords in another file. But > you didn't give the argument. > > In other words: I hate GNU-style changelogs ;-). > > Also, we usually put blank lines between items (read the output of "git > log --no-merges" in git.git to get an idea of the conventions). Generally, I would like git to have full support for the XDG base dir spec because it would allow it to become a good citizen in an ecosystem of software which already support the XDG base dir spec by default. Personally, I am annoyed when a piece of software does not store its configuration into its own separate directory because I version control the configuration of every piece of software (with git ;) ) in its own directory, and store them as submodules in my home directory git repo. Keeping consistent with the behavior of git-config, the presence of the credentials file in the XDG git directory signals that the user wants credentials to be stored in the XDG directory rather than the HOME directory. However, we keep reading from the home credentials in case the user is using old versions of git on the same home directory. In fact, thinking about it again, I think the behavior implemented in the patch may not be suitable. Comments below. >> Likewise, >> lookup_credential() returns 1 if it could find the credential, and 0 if >> it could not. > > Err, you're changing the calling convention, and you're not the only > caller (git grep lookup_credential). > > If you need to change this existing function, best is to start your > series with a preparatory patch that does the calling convention change, > adapts the other caller, and then write your change on top, as [PATCH 2]. Eh? I thought lookup_credential has static linkage. The only other use of lookup_credential is in credential-cache--daemon.c, and that has its own function definition with static linkage. >> - if (!strcmp(op, "get")) >> - lookup_credential(file, &c); >> - else if (!strcmp(op, "erase")) >> - remove_credential(file, &c); >> - else if (!strcmp(op, "store")) >> - store_credential(file, &c); >> - else >> + if (!strcmp(op, "get")) { >> + if (file) { >> + lookup_credential(file, &c); >> + } else { >> + if (xdg_file && access_or_warn(xdg_file, R_OK, 0) == 0) >> + ret = lookup_credential(xdg_file, &c); >> + if (!ret && home_file && access_or_warn(home_file, >> R_OK, 0) == 0) >> + lookup_credential(home_file, &c); >> + } >> + } else if (!strcmp(op, "erase")) { >> + if (file) { >> + remove_credential(file, &c); >> + } else { >> + if (xdg_file && access(xdg_file, F_OK) == 0) >> + remove_credential(xdg_file, &c); >> + if (home_file && access(home_file, F_OK) == 0) >> + remove_credential(home_file, &c); > > Why is it somethimes access_or_warn and sometimes just access? (genuine > question) For "get" even though the xdg file cannot be read I believe it should not be a fatal error because the credential may be found in the home file. We should still warn the user though because it may not be what the user wants. However, I see now that I mistakenly broke compatibility with the old behavior, which errors out if the home credential file could not be read. Thinking about it again, the behavior could go both ways. The user may actually not want the xdg or home credential file to be unreadable, and so we should error out if the files are unreadable. This is something that the community should decide, I think, so opinions please :) Personally, I think we should go with this behavior. If that is so, access(file, F_OK) would be used instead of access_or_warn. For "erase", as st
Re: [PATCH v2 2/3] sha1_file: implement changes for "cat-file --literally -t"
karthik nayak writes: >> Looks suspiciously familiar... > Yes, you suggested it. > It has a similar structure to unpack_sha1_header(). > Anything wrong with it? I don't know if there is something wrong with the code, or not, but it wasn't mentioned in the log message at all that it is not your code, and I was mostly reacting to that. It is fairly common around here to take somebody else's code and run with it, but people say things like "This is based on suggestion from X", "This function was stolen from Y", etc. and then conclude with "but I adjusted it to match the caller I wrote, so bugs are mine." when they do so. -- 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: [RFH] GSoC 2015 application
Stefan Beller writes: > I cannot remember the usual option off hand to describe the revision > instead of a date. Because "log --since= master" and "log v1.0..master" mean two completely different things, we need some way to specify which one of these two the user wants to "git clone" so that the user can say at what point the user wants the shallow history to begin. I think it is a good idea to use --since for date-based cut-off, in addition to the existing --depth that is depth-based cut-off, and we probably would want another one that gives the topology-based cut-off, so that we can express the range in a similar way to "log v1.0..master". But when we talk about the topology-based cut-off locally, we always use the set syntax A..B, ^A B, etc. and never a command line option with an argument to specify "the bottom of the history". It is not surprising that you don't remember any usual option for that, because there isn't one. The closest thing I can think of that looks somewhat like a command line option is "--not", as in git log A B --not C D E that is equivalent to "A B ^C ^D ^E", but that is not an option that takes an argument. I do not know if it is particularly a good idea to say: git clone --not v2.0 $URL to specify topology-based cut-off. But we would need some way to say a set-based cut-off; I do not think using --since for that purpose is a good idea, though, because that is already taken for date-based cut-off, and mixing them together will introduce confusion. -- 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: feature request: excluding files/paths from "git grep"
Jeff King writes: > On Wed, Mar 04, 2015 at 12:56:10PM -0800, Junio C Hamano wrote: > >> > As in, while working on a specific project, I sometimes just want to >> > exclude, for the time being, a bunch of stuff from 'git grep'. >> >> The key word here is "for the time being", though. What would you >> do once you are done with the "for the time being" activity? "git >> config --unset"? > > IMHO this is being too paternalistic. You can already shoot yourself > in the foot by configuring an alias to grep, running your alias, and > wondering why it does not produce the results you wanted. Yeah, as I said, it is a deliberately paternalistic stance. But at least when I say "git mygrep" using the alias mechanism and get a result that is different from what I expect from "git grep", I would know I am doing something different with "mygrep" from "grep", no? And a great thing about that "use alias" approach is that we can sidestep the entire "then what should I do when I have to override the configured thing for one-shot invocation?" question, as there is an obvious simple answer "don't use that alias but use the underlying command". -- 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 describe --contains doesn't work properly for a commit
Jeff King writes: > On Wed, Mar 04, 2015 at 12:41:57PM -0800, Junio C Hamano wrote: > > >> This one, and $gmane/264101, are a few instances of this known issue >> raised here recently. > > If $gmane/264101 is caused by clock skew, I'd find that disturbing. > Those algorithms are supposed to be "correct, but slower" in the face of > skew, not ever incorrect. My understanding is that the commit painting done by merge-base is designed to be always correct, but the A..B revision traversal depends on SLOP being big enough. When the traversal queue is filled with all UNINTERESTING commits, some of which needs to be dug to reveal newer commits that are not yet painted as UNINTERESTING in order to get the complete picture, the still_interesting() logic will end up stopping prematurely, leaving commits that are actually UNINTERESTING in the topological sense unpainted in the resulting newlist that is assigned to revs->commits in limit_list(), yielding an incorrect result. >> > Calculating them is simple. Caching and storage is the bigger question. >> >> Yes, also having to handle the ones whose generation numbers haven't >> been computed yet adds to the complexity. > > I'm not sure it's that bad. If you cache generation numbers for all > known commits when you repack, then worst case you have to traverse all > commits not in the pack. > ... > IMHO, if you are going to go to the trouble to detect and store skew, > you should just go to the trouble to calculate and store reliable > generation numbers. I would actually prefer a solution with generation numbers, of course, because that would give us provably correct result. If it can be done without too much hassle, I am all for it. Scrap the half-baked "I've been wondering" compromise ;-) 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: feature request: excluding files/paths from "git grep"
On Wed, Mar 04, 2015 at 12:56:10PM -0800, Junio C Hamano wrote: > > As in, while working on a specific project, I sometimes just want to > > exclude, for the time being, a bunch of stuff from 'git grep'. > > The key word here is "for the time being", though. What would you > do once you are done with the "for the time being" activity? "git > config --unset"? IMHO this is being too paternalistic. You can already shoot yourself in the foot by configuring an alias to grep, running your alias, and wondering why it does not produce the results you wanted. But I'd also oppose a `grep.pathspecs` config option for a similar reason: you can already accomplish the same thing (and more) with an alias. -Peff PS One annoying thing about aliases is that you cannot re-alias an existing command, and git has already taken all of the good, easy names that we have trained our fingers for. :) Of course re-aliasing git-grep is another recipe for head-scratching (and broken scripts), but I am not sure there aren't a host of other ways to do a similar thing (basically any configuration option has the capacity to produce unexpected results if you forget that it is set). I dunno. Probably I am a bad person for dredging up that ancient argument again. -- 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 describe --contains doesn't work properly for a commit
On Wed, Mar 04, 2015 at 12:41:57PM -0800, Junio C Hamano wrote: > > Calculating them is simple. Caching and storage is the bigger question. > > Yes, also having to handle the ones whose generation numbers haven't > been computed yet adds to the complexity. I'm not sure it's that bad. If you cache generation numbers for all known commits when you repack, then worst case you have to traverse all commits not in the pack. > This one, and $gmane/264101, are a few instances of this known issue > raised here recently. If $gmane/264101 is caused by clock skew, I'd find that disturbing. Those algorithms are supposed to be "correct, but slower" in the face of skew, not ever incorrect. > I have been wondering if we can do something > along the following (these are not alternatives) as a cheaper > workaround: > > (1) Introduce '--skewed-timestamps[=(allow|warn|reject)' to all > commands that create new commit objects. If the committer > timestamp being used is older than any of the parent commits, > "warn" or "reject" depending on the setting. I think this idea has come up before. If it's _your_ timestamp that is screwed up, this detects it, which is good. But if it's somebody else's timestamp that is screwed up, there's often not much you can do. It's already baked into the history. I don't mind it as an extra layer of protection, I guess. But my recollection of the great skew survey[1] is that most of these problems don't come from actual clock skew, but from software bugs or bogus data in imported commits. True skew is generally less than a day, and can be handled with a fixed slop time. [1] http://article.gmane.org/gmane.comp.version-control.git/159065 > (2) Compute a bitmap whose timestamps are suspect when we pack to > mark commits. When revision.c:limit_list() tries to see if > there still are interesting commits, an UNINTERESTING commit > marked as such shouldn't be counted as "not interesting because > it is old enough". Use the same hint in the walker used in > "describe --contains". If you see mismatched timestamps between a parent and child commit, it's often not clear which one is suspicious. Is the parent skewed to the future, or is the child skewed to the past? Which one do you mark as suspect? IMHO, if you are going to go to the trouble to detect and store skew, you should just go to the trouble to calculate and store reliable generation numbers. -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
From: William Broady
Hi http://newmajorityopportunity.org/interest.php?action=6azpd6cuw9fb2 wb...@aol.com Sent from my iPhone -- 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] archive-zip: add --text parameter
René Scharfe writes: > No sign-off, yet, because I'm not sure we really need another option. > E.g. --text=all doesn't seem to be actually useful, but it was easy to > implement. Info-ZIP's zip always creates archives like --text=auto > does, so perhaps we should make that our default behavior as well? My knee-jerk reaction is "yeah, why not? what are the downsides, other than the result will not be bit-for-bit identical to the output from older Git". I am sure I am missing something as I do not regularly use this format. > @@ -256,6 +264,8 @@ static int write_zip_entry(struct archiver_args *args, > return error("cannot read %s", >sha1_to_hex(sha1)); > crc = crc32(crc, buffer, size); > + if (is_binary < 0) > + is_binary = buffer_is_binary(buffer, size); In this codepath, do you have the path of the thing the buffer contents came from? I am wondering if consulting the attributes system is a better idea. Anything that is explicitly marked as "binary" or "-diff" is definitely binary, and anything that is not marked as "binary" is text to us for all practical purposes, no? -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [BUG] Segfault with rev-list --bisect
On Wed, Mar 4, 2015 at 6:44 PM, Junio C Hamano wrote: > Troy Moure writes: > >> git rev-list --bisect --first-parent --parents HEAD --not HEAD~1 > > Hmm, as "rev-list --bisect" is not end-user facing command (it is > purely an implementation detail for "git bisect") and we never call > it with --first-parent, I am not sure if it is worth labelling it as > a BUG. Surely, the command can refuse to operate when it sees both > options given, but that would be a fairly low priority. Hrm, ok. I didn't realize "--bisect" is only intended to be used by git-bisect (although I suppose the fact that it treats ref/bisect/* specially should have been a hint). If uses of "--bisect" other than by git-bisect are considered unsupported, IMO it would be good to say that in the documentation - right now it looks like just another rev-list parameter. (I realize rev-list itself is "plumbing", but that's not the same as "not user facing", is it?) If you're curious, I ran into this because I am working on a script that can be run repeatedly to process commits, and uses git notes to mark commits that have been processed. Parents are always processed before their children, so if a commit has a note, it means all its ancestors also have notes. I want to quickly find the set of commits that have not yet been processed. I am thinking of finding the "boundary" commits (commits that have a note and at least one child that does not) by using a binary search to find the boundary commit on the first-parent chain, and then recursively doing the same thing starting from each non-first parent of each merge commit between the boundary commit and the starting point. Upon further thought, it's probably better to just read the whole first-parent chain and do the binary search in the script, since "git rev-list --bisect" would have generate the chain each time it's called. But I'd already run into the segfault, so I thought I'd report it. Of course, I'd appreciate any thoughts or comments on the problem I'm trying to solve as well. Thanks, Troy -- 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: Easy Non-Fast-Forward Pushes
Lasse Kliemann writes: > 1. Try pushing to origin/master. If it works, fine. If not, goto 2. > > 2. Push to the appropriate personal branch. I wonder what happens to this user _after_ that change gets integrated on the project side. Presumably somebody picks up the change from the "personal branch", does necessary merge and updates the master, so the next time "git pull" is done, it will fast-forward? I have a feeling that running trivial merges on the server-side when a push is made, and immedately pulling that result back might help such userbase who does not care too much about the history better, instead of using the bare-metal 'git pull' and 'git push'. You'd be scripting on the client side to do the above two steps for your end users anyway, so it would not be too much of a stretch to make that script a bit smarter still? -- 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] versionsort: support reorder prerelease suffixes
Junio C Hamano writes: >> +versionsort.prereleaseSuffix:: >> +When version sort is used in linkgit:git-tag[1], prerelease >> +tags (e.g. "1.0-rc1") may appear after the main release >> +"1.0". By specifying the suffix "-rc" in this variable, >> +"1.0-rc1" will appear before "1.0". One variable assignment >> +per suffix. > > I think the last half-sentence want to mean that > > [versionsort] > prereleaseSuffix = -pre > prereleaseSuffix = -rc > > is the supported way to write, and not > > [versionsort] > prereleaseSuffix = -pre -rc > > but it probably is unclear unless the reader already knows what it > is trying to say. The reader also needs to learn somewhere how the > order of the entries affects the result. This is already in 'next', so could you fix that half-sentence in the documentation via an incremental update? 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 v2 2/3] sha1_file: implement changes for "cat-file --literally -t"
On 03/05/2015 02:28 AM, Junio C Hamano wrote: Karthik Nayak writes: > add "sha1_object_info_literally()" which is to be used when > the "literally" option is given to get the type of object > and print it, using "sha1_object_info_extended()". > > add "unpack_sha1_header_literally()" to unpack sha headers > which may be greater than 32 bytes, which is the threshold > for a regular object header. > > modify "sha1_loose_object_info()" to include a flag argument > and also include changes to call "unpack_sha1_header_literally()" > when the literally flag is passed. Also copies the obtained > type to the typename field of object_info. > > modify "sha1_object_info_extended()" to call the function > "sha1_loose_object_info()" with flags. > > Signed-off-by: Karthik Nayak > --- > sha1_file.c | 84 +++-- > 1 file changed, 77 insertions(+), 7 deletions(-) > > diff --git a/sha1_file.c b/sha1_file.c > index 69a60ec..1068ca0 100644 > --- a/sha1_file.c > +++ b/sha1_file.c > @@ -1564,6 +1564,36 @@ int unpack_sha1_header(git_zstream *stream, unsigned char *map, unsigned long ma > return git_inflate(stream, 0); > } > > +static int unpack_sha1_header_literally(git_zstream *stream, unsigned char *map, > +unsigned long mapsize, > +struct strbuf *header) > +{ > +... > +} Looks suspiciously familiar... Yes, you suggested it. It has a similar structure to unpack_sha1_header(). Anything wrong with it? > +int sha1_object_info_literally(const unsigned char *sha1) > +{ > +enum object_type type; > +struct strbuf sb = STRBUF_INIT; > +struct object_info oi = {NULL}; > + > +oi.typename = &sb; > +oi.typep = &type; > +if (sha1_object_info_extended(sha1, &oi, LOOKUP_LITERALLY) < 0) > +return -1; > +if (*oi.typep > 0) > +printf("%s\n", typename(*oi.typep)); > +else > +printf("%s\n", oi.typename->buf); > +strbuf_release(oi.typename); > +return 0; > +} > + NAK. Please don't add end-user facing final output to sha1_file.c; instead have the caller use a helper function like this one to extract necessary information and perform the end-user interaction there. Ok, will do that. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFH] GSoC 2015 application
Stefan Beller writes: > On Wed, Mar 4, 2015 at 4:22 PM, Junio C Hamano wrote: >> On Wed, Mar 4, 2015 at 4:17 PM, Philip Oakley wrote: git clone --since 2.weeks.ago git clone --since v2.10 >>> >>> The use of --since instead of --date would be an equally valid way of >>> spelling the option (coders choice;-) >> >> I think it is a demonstration of poor taste. Everywhere else, >> --since is a way to specify the date, not a revision. Why should >> this one alone should be different? > > I wanted to point out the broader use case than being stylish correct, OK. I actually think it is OK to envision that "--since" will be made appliable to non-dates in the longer term. The proposal, when dug into minor detail, may include extending approxidate() to take a revision name, i.e. git log --since=v2.0.0 master would behave as if git log --since=@1401300269 master because that is the timestamp the v2.0.0 tag carries. Note that I do not think it is a good idea to rewrite the above to git log v2.0.0..master at all (hint: think how you would rewrite --until). As to the shallow-boundary capability implementation, I think what has been discussed in the thread is basically sound. Just send the end-user supplied string ("2.weeks.ago", "v2.10", etc.) over the wire without interpreting locally, and have the server end apply (an updated) approxidate() on it, internally compute the exact cut-point just like "git bundle create" does when it computes the prerequisites, and use the resulting commits as the shallow boundary. -- 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] Adds configuration options for some commonly used command-line options (GSOC micro-project)
Amate Yolande writes: > This is a patch for my work on one of the micro projects, as I intend > to apply for the Google Summer of Code 2015 under the Git community. > This patch gives the user the oppotunity to specify configuration > options for some commonly used command-line options for exampel: > git config defopt.am 'am -3' > --- Check Documentaiton/SubmittingPatches again? It would be beneficial to use the output of "git log --no-merges" for recent history to see the recommended style of log messages while studying it. > Makefile |1 + > defopt.c | 24 > git.c| 56 Docs and tests? > +static int handle_defopt(int *argcp, const char ***argv) > +{ > + int envchanged = 0, ret = 0, saved_errno = errno; What is the point of having a local envchanged here, receiving the info from handle_options() only to discard? > + subdir = setup_git_directory_gently(&unused_nongit); > + ... > + if (subdir && chdir(subdir)) > + die_errno("Cannot change to '%s'", subdir); Why do you need to do this chdir() here? Wouldn't the caller of the codepath after the callsite you added the call to this function go to the top-level as necessary already? > + errno = saved_errno; > + > +} > + > + > static int handle_alias(int *argcp, const char ***argv) > { > int envchanged = 0, ret = 0, saved_errno = errno; > @@ -517,6 +570,9 @@ static void handle_builtin(int argc, const char **argv) > argv[1] = argv[0]; > argv[0] = cmd = "help"; > } > + > + if(is_builtin(cmd) && argc == 1) > + handle_defopt(&argc, &argv); You used "am -3" as an example, but is "am" a built-in? Even if it were, being able to say "git am" (no arguments) and getting that rewritten to "git am -3", only when there is no other arguments, is not all that useful, as a typical workflow with "am" is to save a series of patches in a mailbox file (e.g. I would save the message I am responding to in ./+ay-defopt.mbox) and then feed that as an argument (e.g. "git am ./+ay-defopt.mbox"). A lazier version of me (but not me who is typing this message) might appreciate it if "git am ./+ay-defopt.mbox" gets rewritten to "git am -3 ./+ay-defopt.mbox" by setting "git config am.threeway yes" once, while having an option to countermand that per invocation, by saying "git am --no-3way ./+ay-defopt.mbox". I think what I am saying is that an ultra-generic solution like the patch I am commenting on implements is way too simple to be useful. With today's code, our users can do this once: git config alias.am3 "am -3" and then "git am3 ./+ay-defopt.mbox" would behave as if they typed "git am -3 ./+ay-defopt.mbox", which would already be one step more useful than this "only when there is no argument" design. I think the problem with this patch ultimately came from a poor phrasing of the Micro suggestion, which said something like "find some common command options and add configuration". It was meant to allow many different people to do many different things (i.e. one person does am.threeway and am.threeway only), not an invitation to design something that is generic that covers all these command options in one go. So, perhaps limit the scope of Micro to a single command with a few commonly desired configured options and try again? 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: [RFC/PATCH 0/3] protocol v2
On Wed, Mar 4, 2015 at 11:10 AM, Shawn Pearce wrote: > On Wed, Mar 4, 2015 at 4:05 AM, Duy Nguyen wrote: >> On Wed, Mar 4, 2015 at 11:27 AM, Shawn Pearce wrote: >>> Let me go on a different tangent a bit from the current protocol. >>> >>> http://www.grpc.io/ was recently released and is built on the HTTP/2 >>> standard. It uses protobuf as a proven extensibility mechanism. >>> Including a full C based grpc stack just to speak the Git wire >>> protocol is quite likely overkill, but I think the embedding of a >>> proven extensible format inside of a bi-directional framed streaming >>> system like HTTP/2 offers some good guidance. >> >> I'll take this as "learn from grpc, not just reuse grpc" > > Correct, that was what I was trying to say and I just wrote it poorly. > > HTTP 1.x, HTTP/2 and protobuf have proven themselves to be fairly open > to extension and working well in the wild for transports. There is > useful guidance there that we should draw from to try and leave doors > open for the future. > > HTTP/2, protobuf and grpc are fairly complex. I consider any one of > them too complicated for Git specific use. However HTTP/2 is probably > the future of HTTP stacks so we may see it show up in libcurl or > something as popular as libcurl in another 10 years. Hg had some > reasonably sane ideas about building the wire protocol to work well on > HTTP 1.x upfront rather than Git tacking it on much later. > >>> Network protocol parsing is hard. Especially in languages like C where >>> buffer overflows are possible. Or where a client could trivially DoS a >>> server by sending a packet of size uint_max and the server naively >>> trying to malloc() that buffer. Defining the network protocol in an >>> IDL like protobuf 3 and being machine generated from stable well >>> maintained code has its advantages. >> >> I'm still studying the spec, so I can't comment if using IDL/protobuf3 >> is a good idea yet. >> >> But I think at least we can avoid DoS by changing the pkt-line (again) >> a bit: the length 0x means that actual length is 0xfffe and the >> next pkt-line is part of this pkt-line. Higher level (upload-pack or >> fetch-pack, for example) must set an upper limit for packet_read() so >> it won't try to concatenate pkt-lines forever. > > pkt-line is a reasonably simple and efficient framing system. A 64 KiB > pkt-line frame only costs ~0.0061% overhead; ~0.0076% overhead if you > are a pack stream in a side-band-64k channel. That is probably more > efficient than HTTP/2 or SSL framing. > > I see no reason to attempt to try and reduce that overhead further. 64 > KiB frame size is enough for anyone to move data efficiently with > these headers. In practice you are going to wrap that up into SSH or > SSL/TLS and those overheads are so much higher it doesn't matter we > have a tiny loss here. > > I think a mistake in the wire protocol was making the pkt-line length > human readable hex, but the sideband channel binary. _If_ we redo the > framing the only change I would make is making the side band readable. > Thus far we have only used 0, 1, 2 for sideband channels. These could > easily be moved into human readable channel ids: > > 'd': currently sideband 0; this is the application data, aka the pack data > 'p': currently sideband 1; this is the progress stream for stderr > 'e': currently sideband 2; there was an error, data in this packet > is the message text, and the connection will shutdown after the > packet. > > And then leave all other sideband values undefined and reserved for > future use, just like they are all open today. > > I am not convinced framing changes are necessary. I would fine with > leaving the sideband streams as 0,1,2... but if we want a text based > protocol for ease of debugging we should be text based across the > board and try very hard to avoid these binary values in the framing, > or ever needing to use a magical NUL byte in the middle of a packet to > find a gap in older parsers for newer data. > > > If you want to build a larger stream like ref advertisement inside a > pkt-line framing without using a pkt-line per ref record, you should > follow the approach used by pack data streams where it uses the 5 byte > side-band pkt-line framing and a side-band channel is allocated for > that data. Application code can then run a side-band demux to yank out > the inner stream and parse it. > > It may be simpler to restrict ref names to be smaller than 64k in > length so you have room for framing and hash value to be transferred > inside of a single pkt-line, then use the pkt-line framing to do the > transfer. > > Today's upload-pack ref advertisment has ~25% overhead. Most of that > is in the duplicated tag name for the peeled refs/tags/v1.0^{} lines. > If you drop those names (but keep the pkt-line and SHA-1), its only > about 8% overhead above the packed-refs file. > > I think optimization efforts for ref advertisement need to focus on > reducing the number of refs sent back
Re: [PATCH] reset: allow "-" short hand for previous commit
Sudhanshu Shekhar writes: >> What should worry us even more is what the user would get when @{-1} >> does not resolve to something the command can use. It would be bad >> if we give an error message with @{-1} in it that the user never >> typed (and may not even understand what it means). > > I apologize for having overlooked this use case. Thanking is fine, but there is no need to apologize in response to review comments. We are imperfect humans and review exchanges are designed to allow us cover points each of us missed in our collective effort to make Git better. > Another thing, can someone guide me regarding the right place to add a > test case? Should it be t7102-reset.sh or some other file? At the end of that file sounds like a reasonable choice. You would want to test various cases, such as (1) what happens when there is no @{-1} at all (you would need a newly initialied test repository, just like the last test in that file creates mixed_worktree repository for its own use), (2) what happens when there is, split into two , i.e. (1-a) @{-1} does not exist but there is a file whose name is "-"; (1-b) @{-1} does not exist and there is no file whose name is "-"; (2-a) @{-1} exists but there is a file whose name is "-"; and (2-b) @{-1} exists and there is no file whose name is "-". Do not just test (2-b) and declare victory---making sure that a feature does not trigger when it should not is also important. 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: [RFH] GSoC 2015 application
On Wed, Mar 4, 2015 at 4:22 PM, Junio C Hamano wrote: > On Wed, Mar 4, 2015 at 4:17 PM, Philip Oakley wrote: >>> >>>git clone --since 2.weeks.ago >>>git clone --since v2.10 >> >> The use of --since instead of --date would be an equally valid way of >> spelling the option (coders choice;-) > > I think it is a demonstration of poor taste. Everywhere else, --since > is a way to > specify the date, not a revision. Why should this one alone should be > different? I wanted to point out the broader use case than being stylish correct, though from an English grammars point of view `--since` should also be able to describe a point in time ("since 2 weeks ago" is as valid as "since Feb 17th") I cannot remember the usual option off hand to describe the revision instead of a date. Maybe we want to have one option long term to allow any kind of input (revision and date), as this may be easier to remember, especially if it aligns well with the English language. -- 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: [RFH] GSoC 2015 application
On Thu, Mar 5, 2015 at 6:55 AM, Stefan Beller wrote: > On Wed, Mar 4, 2015 at 2:05 PM, Philip Oakley wrote: > >> A possible idea is to add a date based variant of shallow clone : >> >> 'git clone --date ...' >> >> in the same vein as the existing depth (shallow) clone. > > As food for thought: > Maybe broaden this further up to the git-ish way of describing refs, so > > git clone --since 2.weeks.ago > git clone --since v2.10 > git clone --since c0ffee^^ > > would all equally work? Yes, and we could generalize it further: add shallow-rev-list capability and send "shallow-rev-list " line to define shallow boundary. The protocol change would be simple enough. The hard part is select a safe subset of rev-list options that can be used in this mode and reject the rest. > I am not sure if that is feasible though, but it would come in handy. (E.g. > you are an end user and want to bisect down a bug which you notice in the > new version X but not in old version Y, so you start on getting the sources, > compiling, bisecting) -- Duy -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFH] GSoC 2015 application
On Wed, Mar 4, 2015 at 4:17 PM, Philip Oakley wrote: >> >>git clone --since 2.weeks.ago >>git clone --since v2.10 > > The use of --since instead of --date would be an equally valid way of > spelling the option (coders choice;-) I think it is a demonstration of poor taste. Everywhere else, --since is a way to specify the date, not a revision. Why should this one alone should be different? -- 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
Interest in contributing to the Git for GSOC 2015
Hello My name is Amate Yolande a first year computer science student from Cameroon. I would like to participate in the google summer fo code 2015 with the Git community. I have done one of the micro project and I hope it gets reviewed soon so I could update it. I would like to work on the Unifying git branch -l, git tag -l, and git for-each-ref project for GSOC 2015 and I hope I can get more directives on how to move on with this project. Thanks Yolande -- 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/7] Fix some problems with reflog expiration
On Tue, Mar 3, 2015 at 3:43 AM, Michael Haggerty wrote: > This is v3 of the patch series. Thanks to Junio for his comments > about v2 [1]; I think this version addresses all of his points. > > Changes since v2: > > * Introduce a temporary in "struct ref_lock: delete the force_write > member" to make a complicated boolean expression easier to > understand. > > * Split the documentation of options for "reflog expire" and "reflog > delete", and make another couple tweaks to the "reflog" > documentation. > > * Add a NEEDSWORK comment suggesting that the "reflog" command should > use parse_options() to process its command line. > > [1] http://thread.gmane.org/gmane.comp.version-control.git/264586 The whole series is Reviewed-by: Stefan Beller Thanks for adjusting the commit from me to fit in better into the series. Thanks, Stefan > > Michael Haggerty (6): > write_ref_sha1(): remove check for lock == NULL > write_ref_sha1(): Move write elision test to callers > lock_ref_sha1_basic(): do not set force_write for missing references > reflog: improve and update documentation > reflog_expire(): ignore --updateref for symbolic references > reflog_expire(): never update a reference to null_sha1 > > Stefan Beller (1): > struct ref_lock: delete the force_write member > > Documentation/git-reflog.txt | 144 > ++- > builtin/reflog.c | 9 +-- > refs.c | 65 ++- > 3 files changed, 126 insertions(+), 92 deletions(-) > > -- > 2.1.4 > -- 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: [RFH] GSoC 2015 application
From: "Stefan Beller" On Wed, Mar 4, 2015 at 2:05 PM, Philip Oakley wrote: A possible idea is to add a date based variant of shallow clone : 'git clone --date ...' in the same vein as the existing depth (shallow) clone. As food for thought: Maybe broaden this further up to the git-ish way of describing refs, so git clone --since 2.weeks.ago git clone --since v2.10 git clone --since c0ffee^^ would all equally work? The use of --since instead of --date would be an equally valid way of spelling the option (coders choice;-) At the clone stage, the local Git can't determine (for the 2nd & 3d option) where such a revison is located, so would have to send the revision string to the server for processing, which could complicate the protocol. Hence my choice of a simple unix time value at the protocol level. An alternate/addition is to use a nominated sha1 (from ls-remote) as a stand in for a date, allowing your option 2 (--since ) to be implemented as an alias or script. It all depends on how complicated we want it to become, but starting simple (though extensible) is important. I am not sure if that is feasible though, but it would come in handy. (E.g. you are an end user and want to bisect down a bug which you notice in the new version X but not in old version Y, so you start on getting the sources, compiling, bisecting) -- Philip -- 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] Adds configuration options for some commonly used command-line options (GSOC micro-project)
This is a patch for my work on one of the micro projects, as I intend to apply for the Google Summer of Code 2015 under the Git community. This patch gives the user the oppotunity to specify configuration options for some commonly used command-line options for exampel: git config defopt.am 'am -3' --- Makefile |1 + defopt.c | 24 git.c| 56 3 files changed, 81 insertions(+) create mode 100644 defopt.c diff --git a/Makefile b/Makefile index 2320de5..e713e23 100644 --- a/Makefile +++ b/Makefile @@ -789,6 +789,7 @@ LIB_OBJS += csum-file.o LIB_OBJS += ctype.o LIB_OBJS += date.o LIB_OBJS += decorate.o +LIB_OBJS += defopt.o LIB_OBJS += diffcore-break.o LIB_OBJS += diffcore-delta.o LIB_OBJS += diffcore-order.o diff --git a/defopt.c b/defopt.c new file mode 100644 index 000..b4fa9e2 --- /dev/null +++ b/defopt.c @@ -0,0 +1,24 @@ +#include "cache.h" + +static const char *defopt_key; +static char *defopt_val; + +static int defopt_lookup_cb(const char *k, const char *v, void *cb) +{ + const char *name; + if (skip_prefix(k, "defopt.", &name) && !strcmp(name, defopt_key)) { + if (!v) + return config_error_nonbool(k); + defopt_val = xstrdup(v); + return 0; + } + return 0; +} + +char *defopt_lookup(const char *defopt) +{ + defopt_key = defopt; + defopt_val = NULL; + git_config(defopt_lookup_cb, NULL); + return defopt_val; +} diff --git a/git.c b/git.c index 9c49519..4b556e1 100644 --- a/git.c +++ b/git.c @@ -223,6 +223,59 @@ static int handle_options(const char ***argv, int *argc, int *envchanged) return (*argv) - orig_argv; } +static int handle_defopt(int *argcp, const char ***argv) +{ + int envchanged = 0, ret = 0, saved_errno = errno; + const char *subdir; + int count, option_count; + const char **new_argv; + const char *defopt_command; + char *defopt_string; + int unused_nongit; + + subdir = setup_git_directory_gently(&unused_nongit); + + defopt_command = (*argv)[0]; + defopt_string = defopt_lookup(defopt_command); + if (defopt_string) { + + count = split_cmdline(defopt_string, &new_argv); + if (count < 0) + return; + option_count = handle_options(&new_argv, &count, &envchanged); + + memmove(new_argv - option_count, new_argv, + count * sizeof(char *)); + new_argv -= option_count; + + if (count < 1) + return; + + if (strcmp(defopt_command, new_argv[0])) + return; + + trace_argv_printf(new_argv, + "trace: defopt: %s =>", + defopt_command); + + new_argv = xrealloc(new_argv, sizeof(char *) * + (count + *argcp)); + /* insert after command name */ + memcpy(new_argv + count, *argv + 1, sizeof(char *) * *argcp); + + *argv = new_argv; + *argcp += count - 1; + + } + + if (subdir && chdir(subdir)) + die_errno("Cannot change to '%s'", subdir); + + errno = saved_errno; + +} + + static int handle_alias(int *argcp, const char ***argv) { int envchanged = 0, ret = 0, saved_errno = errno; @@ -517,6 +570,9 @@ static void handle_builtin(int argc, const char **argv) argv[1] = argv[0]; argv[0] = cmd = "help"; } + + if(is_builtin(cmd) && argc == 1) + handle_defopt(&argc, &argv); for (i = 0; i < ARRAY_SIZE(commands); i++) { struct cmd_struct *p = commands+i; -- Signed-off-by: Yolande Amate 1.7.10.4 -- 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: [RFH] GSoC 2015 application
On Wed, Mar 4, 2015 at 2:05 PM, Philip Oakley wrote: > A possible idea is to add a date based variant of shallow clone : > > 'git clone --date ...' > > in the same vein as the existing depth (shallow) clone. As food for thought: Maybe broaden this further up to the git-ish way of describing refs, so git clone --since 2.weeks.ago git clone --since v2.10 git clone --since c0ffee^^ would all equally work? I am not sure if that is feasible though, but it would come in handy. (E.g. you are an end user and want to bisect down a bug which you notice in the new version X but not in old version Y, so you start on getting the sources, compiling, bisecting) -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [BUG] Segfault with rev-list --bisect
Troy Moure writes: > git rev-list --bisect --first-parent --parents HEAD --not HEAD~1 Hmm, as "rev-list --bisect" is not end-user facing command (it is purely an implementation detail for "git bisect") and we never call it with --first-parent, I am not sure if it is worth labelling it as a BUG. Surely, the command can refuse to operate when it sees both options given, but that would be a fairly low priority. Of course, if you are planning to do "git bisect --first-parent", it is one of the things that needs to be addressed, together with counting the rounds and bisecting the linear set of commits on the first-parent chain correctly. -- 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 unsigned char to squash compiler warnings
>On 4 Mar 2015, Junio C Hamano Wrote: > Sent: March 4, 2015 5:11 PM > To: Ben Walton > Cc: git@vger.kernel.org > Subject: Re: [PATCH] Use unsigned char to squash compiler warnings > > Ben Walton writes: > > > On Mon, Mar 2, 2015 at 8:30 PM Junio C Hamano > wrote: > > > >> The conversion looked good from a cursory view; I didn't check it > >> very carefully though. > >> > > Yes, because of the Solaris ABI, the Studio compiler defaults char to > > signed char. > > Doesn't our beloved GCC also uses signed char when you write char? > You keep saying that "defaults to signed char is the problem", but that does not > explain why those in the rest of the world outside the Solaris land do not > encounter this problem. > > $ cat >x.c <<\EOF > #include > int main (void) { > SIGNED char ch = 0xff; > printf("%d\n", ch); > return 0; > } > EOF > $ gcc -Wall -DSIGNED= x.c && ./a.out > -1 > $ gcc -Wall -DSIGNED=signed x.c && ./a.out > -1 > > I think th problem is not Solaris uses signed char for char like everybody else > does ;-) but it gives a fairly useless warning to annoy people. > > In any case, here is what I queued, FYI, on bw/kwset-use-unsigned topic. Even the NonStop c99 compiler does not report a warning - and it is usually very noisy. The default is unsigned char for c99 on this platform, and the value interpretation is significant. #include int main (void) { char ch0 = 0xff; signed char ch1 = 0xff; unsigned char ch = 0xff; printf("%d, %d, %d, %d, %d\n", ch0, ch, ch1, ch==ch0, ch==ch1); return 0; } 255, 255, -1, 1, 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] Use unsigned char to squash compiler warnings
Ben Walton writes: > On Mon, Mar 2, 2015 at 8:30 PM Junio C Hamano wrote: > >> The conversion looked good from a cursory view; I didn't check it >> very carefully though. >> > Yes, because of the Solaris ABI, the Studio compiler defaults char to > signed char. Doesn't our beloved GCC also uses signed char when you write char? You keep saying that "defaults to signed char is the problem", but that does not explain why those in the rest of the world outside the Solaris land do not encounter this problem. $ cat >x.c <<\EOF #include int main (void) { SIGNED char ch = 0xff; printf("%d\n", ch); return 0; } EOF $ gcc -Wall -DSIGNED= x.c && ./a.out -1 $ gcc -Wall -DSIGNED=signed x.c && ./a.out -1 I think th problem is not Solaris uses signed char for char like everybody else does ;-) but it gives a fairly useless warning to annoy people. In any case, here is what I queued, FYI, on bw/kwset-use-unsigned topic. Thanks. commit 189c860c9ec5deb95845c056ca5c15b58970158e Author: Ben Walton Date: Mon Mar 2 19:22:31 2015 + kwset: use unsigned char to store values with high-bit set Sun Studio on Solaris issues warnings about improper initialization values being used when defining tolower_trans_tbl[] in ctype.c. The array wants to store values with high-bit set and treat them as values between 128 to 255. Unlike the rest of the Git codebase where we explicitly specify 'unsigned char' for such variables and arrays, however, kwset code we borrowed from elsewhere uses 'char' for this and other variables. Fix the declarations to explicitly use 'unsigned char' where necessary to bring it in line with the rest of the Git. Signed-off-by: Ben Walton Signed-off-by: Junio C Hamano -- 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 describe --contains doesn't work properly for a commit
On Wed, Mar 04, 2015 at 12:41:57PM -0800, Junio C Hamano wrote: > Jeff King writes: > > > On Wed, Mar 04, 2015 at 04:06:17PM +0100, Michael J Gruber wrote: > > > >> Complexity: Was that due to replace refs? Other than that, it seemed to > >> be simple: max(parent generation numbers)+1. > > > > Calculating them is simple. Caching and storage is the bigger question. > > Yes, also having to handle the ones whose generation numbers haven't > been computed yet adds to the complexity. > > This one, and $gmane/264101, are a few instances of this known issue > raised here recently. I have been wondering if we can do something > along the following (these are not alternatives) as a cheaper > workaround: > > (1) Introduce '--skewed-timestamps[=(allow|warn|reject)' to all > commands that create new commit objects. If the committer > timestamp being used is older than any of the parent commits, > "warn" or "reject" depending on the setting. > > Make 'reject' the default for commands that are purely local > (i.e. recording your own progress by cherry-picking, > committing, rebasing, reverting, etc.) and 'warn' the default > for commands that merge other peoples' history that you may > lack the power to rewind and correct (e.g. 'pull' and 'merge' > from remote tracking refs). Please note that a common cause (for me, at least) of skewed timestamps is importing from a foreign VCS. Mike -- 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: [RFH] GSoC 2015 application
From: "Matthieu Moy" Sent: Friday, February 20, 2015 9:39 AM Jeff King writes: - Matthieu, who also cited time constraints Just to clarify: last year we were co-mentoring with Ram. I ended up having a lot of time and did most of the work (not blaming Ram, and I enjoyed the experience). I'm still motivated to co-mentor, but this time the co-mentoring has to be more balanced (or unballanced to the other mentor ;-) ). - Junio, who contributed some project ideas, but who in the past has declined to mentor in order to remain impartial as the maintainer who evaluates student results (which I think is quite reasonable) Yes, as a mentor I did appreciate having Junio as impartial maintainer/reviewer. And he did for sure contribute even without being a mentor! From your list, it seems we can target 1 or 2 slots. I'd say it's still worth applying, but if we don't find more mentors then perhaps it would make sense to say so explicitely in http://git.github.io/SoC-2015-Ideas.html so that students looking for organization know that we'll have very few slots. -- Hi, Given the mention of the GSoC ideas list, I thought it worth writing out one of my little ideas.. A possible idea is to add a date based variant of shallow clone : 'git clone --date ...' in the same vein as the existing depth (shallow) clone. On the wire advertise a 'shallow-date' capability, passing a signed big integer as the unix time for the shallow cut-off point (i.e. future extensible to cover a very wide date range), with optional(?) date+depth hysteresis (clock skew) parameters. Command line interface to use existing date/time formats, (and possibly revision dates?). Extend 'git fetch' to include the --date option. Ensure that 'git push' continues to work with and between shallow/shallow-date clones. Update the documentation in line with the capability. Document any migration plan (if required) Why === This capability would eliminate the existing confusion over the --depth parameter as different branches may require different depths to reach a "common" start point. Extra points for an easy method of '--unshallow-date ' to remove 'old' commits that the user may no longer need locally. (unshallow may not be the right term...) -- Philip -- 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 6/7] Documentation/config.txt: simplify boolean description in the syntax section
The 'true' short-hand doesn't deserve a separate sentence; even our own git config --bool foo.bar yes would not produce it. Signed-off-by: Junio C Hamano --- Documentation/config.txt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Documentation/config.txt b/Documentation/config.txt index c40bf4a..f1cf571 100644 --- a/Documentation/config.txt +++ b/Documentation/config.txt @@ -54,8 +54,8 @@ restrictions as section names. All the other lines (and the remainder of the line after the section header) are recognized as setting variables, in the form -'name = value'. If there is no equal sign on the line, the entire line -is taken as 'name' and the variable is recognized as boolean "true". +'name = value' (or just 'name', which is a short-hand to say that +the variable is the boolean "true"). The variable names are case-insensitive, allow only alphanumeric characters and `-`, and must start with an alphabetic character. -- 2.3.1-316-g7c93423 -- 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 5/7] Documentation/config.txt: describe 'color' value type in the "Values" section
Instead of describing it for color.branch. and have everybody else refer to it, explain how colors are spelled in "Values" section upfront. Signed-off-by: Junio C Hamano --- Documentation/config.txt | 29 + 1 file changed, 13 insertions(+), 16 deletions(-) diff --git a/Documentation/config.txt b/Documentation/config.txt index 7be608b..c40bf4a 100644 --- a/Documentation/config.txt +++ b/Documentation/config.txt @@ -146,6 +146,16 @@ integer:: be suffixed with `k`, `M`,... to mean "scale the number by 1024", "by 1024x1024", etc. +color:: + The value for a variables that takes a color is a list of + colors (at most two) and attributes (at most one), separated + by spaces. The colors accepted are `normal`, `black`, + `red`, `green`, `yellow`, `blue`, `magenta`, `cyan` and + `white`; the attributes are `bold`, `dim`, `ul`, `blink` and + `reverse`. The first color given is the foreground; the + second is the background. The position of the attribute, if + any, doesn't matter. + Variables ~ @@ -838,14 +848,6 @@ color.branch.:: `remote` (a remote-tracking branch in refs/remotes/), `upstream` (upstream tracking branch), `plain` (other refs). -+ -The value for these configuration variables is a list of colors (at most -two) and attributes (at most one), separated by spaces. The colors -accepted are `normal`, `black`, `red`, `green`, `yellow`, `blue`, -`magenta`, `cyan` and `white`; the attributes are `bold`, `dim`, `ul`, -`blink` and `reverse`. The first color given is the foreground; the -second is the background. The position of the attribute, if any, -doesn't matter. color.diff:: Whether to use ANSI escape sequences to add color to patches. @@ -865,8 +867,7 @@ color.diff.:: of `plain` (context text), `meta` (metainformation), `frag` (hunk header), 'func' (function in hunk header), `old` (removed lines), `new` (added lines), `commit` (commit headers), or `whitespace` - (highlighting whitespace errors). The values of these variables may be - specified as in color.branch.. + (highlighting whitespace errors). color.decorate.:: Use customized color for 'git log --decorate' output. `` is one @@ -899,8 +900,6 @@ color.grep.:: separators between fields on a line (`:`, `-`, and `=`) and between hunks (`--`) -- -+ -The values of these variables may be specified as in color.branch.. color.interactive:: When set to `always`, always use colors for interactive prompts @@ -913,8 +912,7 @@ color.interactive.:: Use customized color for 'git add --interactive' and 'git clean --interactive' output. `` may be `prompt`, `header`, `help` or `error`, for four distinct types of normal output from - interactive commands. The values of these variables may be - specified as in color.branch.. + interactive commands. color.pager:: A boolean to enable/disable colored output when the pager is in @@ -940,8 +938,7 @@ color.status.:: `untracked` (files which are not tracked by Git), `branch` (the current branch), or `nobranch` (the color the 'no branch' warning is shown in, defaulting - to red). The values of these variables may be specified as in - color.branch.. + to red). color.ui:: This variable determines the default value for variables such -- 2.3.1-316-g7c93423 -- 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 7/7] log --decorate: do not leak "commit" color into the next item
In "git log --decorate", you would see the commit header like this: commit ... (HEAD, jc/decorate-leaky-separator-color) where "commit ... (" is painted in color.diff.commit, "HEAD" in color.decorate.head, ", " in color.diff.commit, the branch name in color.decorate.branch and then closing ")" in color.diff.commit. If you wanted to paint the HEAD and local branch name in the same color as the body text (perhaps because cyan and green are too faint on a black-on-white terminal to be readable), you would not want to have to say [color "decorate"] head = black branch = black because that you would not be able to reuse same configuration on a white-on-black terminal. You would naively expect [color "decorate"] head = normal branch = normal to work, but unfortunately it does not. It paints the string "HEAD" and the branch name in the same color as the opening parenthesis or comma between the decoration elements. This is because the code forgets to reset the color after printing the "prefix" in its own color. It theoretically is possible that some people were expecting and relying on that the attribute set as the "diff.commit" color, which is used to draw these opening parenthesis and inter-item comma, is inherited by the drawing of branch names, but it is not how the coloring works everywhere else. Signed-off-by: Junio C Hamano --- Documentation/config.txt | 7 +++ log-tree.c | 1 + t/t4207-log-decoration-colors.sh | 16 3 files changed, 16 insertions(+), 8 deletions(-) diff --git a/Documentation/config.txt b/Documentation/config.txt index f1cf571..6d65033 100644 --- a/Documentation/config.txt +++ b/Documentation/config.txt @@ -155,6 +155,13 @@ color:: `reverse`. The first color given is the foreground; the second is the background. The position of the attribute, if any, doesn't matter. ++ +The attributes are meant to be reset at the beginning of each item +in the colored output, so setting color.decorate.branch to `black` +will paint that branch name in a plain `black`, even if the previous +thing on the same output line (e.g. opening parenthesis before the +list of branch names in `log --decorate` output) is set to be +painted with `bold` or some other attribute. Variables diff --git a/log-tree.c b/log-tree.c index 1982631..11676d5 100644 --- a/log-tree.c +++ b/log-tree.c @@ -200,6 +200,7 @@ void format_decorations(struct strbuf *sb, while (decoration) { strbuf_addstr(sb, color_commit); strbuf_addstr(sb, prefix); + strbuf_addstr(sb, color_reset); strbuf_addstr(sb, decorate_get_color(use_color, decoration->type)); if (decoration->type == DECORATION_REF_TAG) strbuf_addstr(sb, "tag: "); diff --git a/t/t4207-log-decoration-colors.sh b/t/t4207-log-decoration-colors.sh index 925f577..6b8ad4f 100755 --- a/t/t4207-log-decoration-colors.sh +++ b/t/t4207-log-decoration-colors.sh @@ -44,15 +44,15 @@ test_expect_success setup ' ' cat >expected
[PATCH v2 4/7] Documentation/config.txt: have a separate "Values" section
The various types of values set to the configuration variables deserve more than a brief footnote mention in the syntax section, and it will be more so after the later steps of this clean up effort. Move the mention of booleans from the syntax section to this new section, and describe how human-readble integers can be spelled with scaling there. Signed-off-by: Junio C Hamano --- Documentation/config.txt | 39 +++ 1 file changed, 31 insertions(+), 8 deletions(-) diff --git a/Documentation/config.txt b/Documentation/config.txt index 1444614..7be608b 100644 --- a/Documentation/config.txt +++ b/Documentation/config.txt @@ -75,14 +75,6 @@ The following escape sequences (beside `\"` and `\\`) are recognized: and `\b` for backspace (BS). No other char escape sequence, nor octal char sequences are valid. -The values following the equals sign in variable assign are all either -a string, an integer, or a boolean. Boolean values may be given as yes/no, -1/0, true/false or on/off. Case is not significant in boolean values, when -converting value to the canonical form using '--bool' type specifier; -'git config' will ensure that the output is "true" or "false". - -Some variables may require a special value format. - Includes @@ -124,6 +116,37 @@ Example path = foo ; expand "foo" relative to the current file path = ~/foo ; expand "foo" in your $HOME directory + +Values +~~ + +Values of many variables are treated as a simple string, but there +are variables that take values of specific types and there are rules +as to how to spell them. + +boolean:: + + When a variable is said to take a boolean value, many + synonyms are accepted for 'true' and 'false'; these are all + case-insensitive. + + true;; Boolean true can be spelled as `yes`, `on`, `true`, + or `1`. Also, a variable defined without `= ` + is taken as true. + + false;; Boolean false can be spelled as `no`, `off`, + `false`, or `0`. ++ +When converting value to the canonical form using '--bool' type +specifier; 'git config' will ensure that the output is "true" or +"false" (spelled in lowercase). + +integer:: + The value for many variables that specify various sizes can + be suffixed with `k`, `M`,... to mean "scale the number by + 1024", "by 1024x1024", etc. + + Variables ~ -- 2.3.1-316-g7c93423 -- 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] archive-zip: add --text parameter
Entries in a ZIP file can be marked as text files. Extractors can use that flag to apply end-of-line conversions. An example is unzip -a. git archive currently marks all ZIP file entries as binary files. This patch adds the new option --text that can be used to mark non-binary files or all files as text files, thus enabling the use of unzip -a. No sign-off, yet, because I'm not sure we really need another option. E.g. --text=all doesn't seem to be actually useful, but it was easy to implement. Info-ZIP's zip always creates archives like --text=auto does, so perhaps we should make that our default behavior as well? Changing the default behavior would cause newer versions of git archive to create different ZIP files than older ones, of course. This can break caching and signature checking. The last time we did that was in 2012 when we added an extended mtime field (227bf5980), I think. I don't remember any fallout from that change, but there was a recent discussion about the stability of generated tar files, so I'm a bit cautious: http://thread.gmane.org/gmane.comp.version-control.git/258516 --- Documentation/git-archive.txt | 5 archive-zip.c | 23 ++ archive.c | 18 ++ archive.h | 7 ++ t/t5003-archive-zip.sh| 56 +++ 5 files changed, 105 insertions(+), 4 deletions(-) diff --git a/Documentation/git-archive.txt b/Documentation/git-archive.txt index cfa1e4e..684ca36 100644 --- a/Documentation/git-archive.txt +++ b/Documentation/git-archive.txt @@ -93,6 +93,11 @@ zip Highest and slowest compression level. You can specify any number from 1 to 9 to adjust compression speed and ratio. +--text=:: + Mark the specfied entries as text files so that `unzip -a` + converts end-of-line characters while extracting. The value + must be either 'all', 'auto', or 'none' (the default). + CONFIGURATION - diff --git a/archive-zip.c b/archive-zip.c index 4bde019..3767940 100644 --- a/archive-zip.c +++ b/archive-zip.c @@ -5,6 +5,7 @@ #include "archive.h" #include "streaming.h" #include "utf8.h" +#include "xdiff-interface.h" static int zip_date; static int zip_time; @@ -210,6 +211,7 @@ static int write_zip_entry(struct archiver_args *args, struct git_istream *stream = NULL; unsigned long flags = 0; unsigned long size; + int is_binary = -1; crc = crc32(0, NULL, 0); @@ -238,8 +240,14 @@ static int write_zip_entry(struct archiver_args *args, method = 0; attr2 = S_ISLNK(mode) ? ((mode | 0777) << 16) : (mode & 0111) ? ((mode) << 16) : 0; - if (S_ISREG(mode) && args->compression_level != 0 && size > 0) - method = 8; + if (S_ISREG(mode)) { + if (args->compression_level != 0 && size > 0) + method = 8; + if (args->text == ARCHIVE_TEXT_ALL) + is_binary = 0; + else if (args->text == ARCHIVE_TEXT_NONE) + is_binary = 1; + } if (S_ISREG(mode) && type == OBJ_BLOB && !args->convert && size > big_file_threshold) { @@ -256,6 +264,8 @@ static int write_zip_entry(struct archiver_args *args, return error("cannot read %s", sha1_to_hex(sha1)); crc = crc32(crc, buffer, size); + if (is_binary < 0) + is_binary = buffer_is_binary(buffer, size); out = buffer; } compressed_size = (method == 0) ? size : 0; @@ -300,7 +310,6 @@ static int write_zip_entry(struct archiver_args *args, copy_le16(dirent.extra_length, ZIP_EXTRA_MTIME_SIZE); copy_le16(dirent.comment_length, 0); copy_le16(dirent.disk, 0); - copy_le16(dirent.attr1, 0); copy_le32(dirent.attr2, attr2); copy_le32(dirent.offset, zip_offset); @@ -328,6 +337,8 @@ static int write_zip_entry(struct archiver_args *args, if (readlen <= 0) break; crc = crc32(crc, buf, readlen); + if (is_binary < 0) + is_binary = buffer_is_binary(buffer, size); write_or_die(1, buf, readlen); } close_istream(stream); @@ -361,6 +372,8 @@ static int write_zip_entry(struct archiver_args *args, if (readlen <= 0) break; crc = crc32(crc, buf, readlen); + if (is_binary < 0) + is_binary = buffer_is_binary(buff
[PATCH v2 2/7] Documentation/config.txt: explain multi-valued variables once
The syntax section repeats what the preamble explained already. That a variable can have multiple values is more about what a variable is than the syntax of the file. Signed-off-by: Junio C Hamano --- Documentation/config.txt | 7 +++ 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/Documentation/config.txt b/Documentation/config.txt index dbe7035..405bf25 100644 --- a/Documentation/config.txt +++ b/Documentation/config.txt @@ -14,7 +14,8 @@ the fully qualified variable name of the variable itself is the last dot-separated segment and the section name is everything before the last dot. The variable names are case-insensitive, allow only alphanumeric characters and `-`, and must start with an alphabetic character. Some -variables may appear multiple times. +variables may appear multiple times; we say then that the variable is +multivalued. Syntax ~~ @@ -56,9 +57,7 @@ header) are recognized as setting variables, in the form 'name = value'. If there is no equal sign on the line, the entire line is taken as 'name' and the variable is recognized as boolean "true". The variable names are case-insensitive, allow only alphanumeric characters -and `-`, and must start with an alphabetic character. There can be more -than one value for a given variable; we say then that the variable is -multivalued. +and `-`, and must start with an alphabetic character. Leading and trailing whitespace in a variable value is discarded. Internal whitespace within a variable value is retained verbatim. -- 2.3.1-316-g7c93423 -- 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/7] Fix leak of color/attributes in "git log --decorate"
In "git log --decorate", you would see the commit header like this: commit ... (HEAD, jc/decorate-leaky-separator-color) where "commit ... (" is painted in color.diff.commit, "HEAD" in color.decorate.head, ", " in color.diff.commit, the branch name in color.decorate.branch and then closing ")" in color.diff.commit. However, setting color.decorate.head to normal does not paint "HEAD" in the "normal" color you have for your terminal. It just uses the same color it used to paint the "(", i.e. color.diff.commit. Fixing this was a simple one-liner; the code forgot to reset the terminal attributes before moving on to the next item. It however turns out that the existing documentation was rather messy and I ended up doing some preparatory clean-up on the documentation around how configuration variables are explained before updating the documentation to clarify that 'normal' ought to work (in other words, the colors and attributes are not cumulative). I am reasonably happy with the result, and I can go with or without [6/7]. The previous round starts at $gmane/264065 [*1*] Junio C Hamano (7): Documentation/config.txt: avoid unnecessary negation Documentation/config.txt: explain multi-valued variables once Documentation/config.txt: describe the structure first and then meaning Documentation/config.txt: have a separate "Values" section Documentation/config.txt: describe 'color' value type in the "Values" section Documentation/config.txt: simplify boolean description in the syntax section log --decorate: do not leak "commit" color into the next item Documentation/config.txt | 111 --- log-tree.c | 1 + t/t4207-log-decoration-colors.sh | 16 +++--- 3 files changed, 77 insertions(+), 51 deletions(-) [Footnote] *1* http://thread.gmane.org/gmane.comp.version-control.git/264063/focus=264065 http://mid.gmane.org/xmqqpp9628tl@gitster.dls.corp.google.com -- 2.3.1-316-g7c93423 -- 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 3/7] Documentation/config.txt: describe the structure first and then meaning
A line can be continued via a backquote-LF and can be chomped at a comment character. But that is not specific to string-typed values. It is common to all, just like unquoted leading and trailing whitespaces are stripped and inter-word spacing are retained. Move the description around and desribe these structural rules first, then introduce the double-quote facility as a way to override them, and finally mention various types of values. Note that these structural rules only apply to the value part of the configuration file. E.g. [aSection] \ name \ = value does not work, because the rules kick in only after seeing "name =". Both the original and the updated text are phrased in an awkward way by singling out the "value" part of the line because of this. Signed-off-by: Junio C Hamano --- Documentation/config.txt | 31 +++ 1 file changed, 15 insertions(+), 16 deletions(-) diff --git a/Documentation/config.txt b/Documentation/config.txt index 405bf25..1444614 100644 --- a/Documentation/config.txt +++ b/Documentation/config.txt @@ -59,32 +59,31 @@ is taken as 'name' and the variable is recognized as boolean "true". The variable names are case-insensitive, allow only alphanumeric characters and `-`, and must start with an alphabetic character. -Leading and trailing whitespace in a variable value is discarded. -Internal whitespace within a variable value is retained verbatim. +A line that defines a value can be continued to the next line by +ending it with a `\`; the backquote and the end-of-line are +stripped. Leading whitespaces after 'name =', the remainder of the +line after the first comment character '#' or ';', and trailing +whitespaces of the line are discarded unless they are enclosed in +double quotes. Internal whitespaces within the value are retained +verbatim. -The values following the equals sign in variable assign are all either -a string, an integer, or a boolean. Boolean values may be given as yes/no, -1/0, true/false or on/off. Case is not significant in boolean values, when -converting value to the canonical form using '--bool' type specifier; -'git config' will ensure that the output is "true" or "false". - -String values may be entirely or partially enclosed in double quotes. -You need to enclose variable values in double quotes if you want to -preserve leading or trailing whitespace, or if the variable value contains -comment characters (i.e. it contains '#' or ';'). -Double quote `"` and backslash `\` characters in variable values must -be escaped: use `\"` for `"` and `\\` for `\`. +Inside double quotes, double quote `"` and backslash `\` characters +must be escaped: use `\"` for `"` and `\\` for `\`. The following escape sequences (beside `\"` and `\\`) are recognized: `\n` for newline character (NL), `\t` for horizontal tabulation (HT, TAB) and `\b` for backspace (BS). No other char escape sequence, nor octal char sequences are valid. -Variable values ending in a `\` are continued on the next line in the -customary UNIX fashion. +The values following the equals sign in variable assign are all either +a string, an integer, or a boolean. Boolean values may be given as yes/no, +1/0, true/false or on/off. Case is not significant in boolean values, when +converting value to the canonical form using '--bool' type specifier; +'git config' will ensure that the output is "true" or "false". Some variables may require a special value format. + Includes -- 2.3.1-316-g7c93423 -- 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/7] Documentation/config.txt: avoid unnecessary negation
Section names and variable names are both case-insensitive, but one is described as "not case sensitive". Use "case-insensitive" for both. Instead of saying "... have to be escaped" without telling what that escaping achieves, state it in a more positive way, i.e. "... can be included by escaping". Signed-off-by: Junio C Hamano --- Documentation/config.txt | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/Documentation/config.txt b/Documentation/config.txt index 097fdd4..dbe7035 100644 --- a/Documentation/config.txt +++ b/Documentation/config.txt @@ -25,7 +25,7 @@ blank lines are ignored. The file consists of sections and variables. A section begins with the name of the section in square brackets and continues until the next -section begins. Section names are not case sensitive. Only alphanumeric +section begins. Section names are case-insensitive. Only alphanumeric characters, `-` and `.` are allowed in section names. Each variable must belong to some section, which means that there must be a section header before the first setting of a variable. @@ -40,8 +40,8 @@ in the section header, like in the example below: Subsection names are case sensitive and can contain any characters except -newline (doublequote `"` and backslash have to be escaped as `\"` and `\\`, -respectively). Section headers cannot span multiple +newline (doublequote `"` and backslash can be included by escaping them +as `\"` and `\\`, respectively). Section headers cannot span multiple lines. Variables may belong directly to a section or to a given subsection. You can have `[section]` if you have `[section "subsection"]`, but you don't need to. -- 2.3.1-316-g7c93423 -- 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: [PATCHv2 1/2] t7508: test git status -v
Michael J Gruber writes: >> Ahh, OK. The way the existing tests prepare 'expect' is "by hand". >> >> So I think what is wrong with this new test is not that relies on >> the current contents of 'expect', but that it modifies it (imagine >> being a merge/patch monkey who has to accept this change while a >> change from somebody else that wants to add another test that relies >> on the original 'expect' intact and then have to scratch his or her >> head when the two topics are merged, wondering why the latter test >> starts failing). >> >> Perhaps >> >> ( cat expect && git diff --cached ) >expect-with-v && >> git status -v >actual && >> test_cmp expect-with-v actual >> >> or something? > > That's what I had first, but the new file shows up as untracked file in > the status output... If we step back and wonder why it is not a problem for the test to create 'expect' and 'output' that are not untracked, what would we find? It seems that there are two ways to do this: - Spell these out in 'expect' as untracked; or - Throw them in .gitignore to be ignored by 'status'. As some other tests want to see how untracked files appear in the output, I wonder if throwing expect and output that are already used in the test, together with the new "expect-with-v" and friends, to a .gitignore file might not be a better direction to go. Perhaps such a clean-up effort might begin with something like this patch? -- >8 -- t7508: .gitignore 'expect' and 'output' files These files are used to observe the behaviour of the 'status' command and if there weren't any such observer, the expected output from 'status' wouldn't even mention them. Place them in .gitignore to unclutter the output expected by the tests. An added benefit is that future tests can add such files that are purely for use by the observer, i.e. the tests themselves, by naming them as expect-foo and/or output-bar. --- t/t7508-status.sh | 62 +-- 1 file changed, 5 insertions(+), 57 deletions(-) diff --git a/t/t7508-status.sh b/t/t7508-status.sh index 8ed5788..9d944a3 100755 --- a/t/t7508-status.sh +++ b/t/t7508-status.sh @@ -320,7 +320,11 @@ EOF test_i18ncmp expect output ' -rm -f .gitignore +cat >.gitignore <<\EOF +.gitignore +expect* +output* +EOF cat >expect <<\EOF ## master @@ -329,8 +333,6 @@ A dir2/added ?? dir1/untracked ?? dir2/modified ?? dir2/untracked -?? expect -?? output ?? untracked EOF @@ -434,8 +436,6 @@ Untracked files: dir2/modified dir2/untracked dir3/ - expect - output untracked EOF @@ -456,8 +456,6 @@ A dir2/added ?? dir2/modified ?? dir2/untracked ?? dir3/ -?? expect -?? output ?? untracked EOF test_expect_success 'status -s -unormal' ' @@ -493,8 +491,6 @@ Untracked files: dir2/untracked dir3/untracked1 dir3/untracked2 - expect - output untracked EOF @@ -518,8 +514,6 @@ A dir2/added ?? dir1/untracked ?? dir2/modified ?? dir2/untracked -?? expect -?? output ?? untracked EOF test_expect_success 'status -s -uall' ' @@ -554,8 +548,6 @@ Untracked files: untracked ../dir2/modified ../dir2/untracked - ../expect - ../output ../untracked EOF @@ -569,8 +561,6 @@ A ../dir2/added ?? untracked ?? ../dir2/modified ?? ../dir2/untracked -?? ../expect -?? ../output ?? ../untracked EOF test_expect_success 'status -s with relative paths' ' @@ -586,8 +576,6 @@ A dir2/added ?? dir1/untracked ?? dir2/modified ?? dir2/untracked -?? expect -?? output ?? untracked EOF @@ -625,8 +613,6 @@ Untracked files: dir1/untracked dir2/modified dir2/untracked - expect - output untracked EOF @@ -647,8 +633,6 @@ cat >expect <<\EOF ?? dir1/untracked ?? dir2/modified ?? dir2/untracked -?? expect -?? output ?? untracked EOF @@ -676,8 +660,6 @@ cat >expect <<\EOF ?? dir1/untracked ?? dir2/modified ?? dir2/untracked -?? expect -?? output ?? untracked EOF @@ -694,8 +676,6 @@ A dir2/added ?? dir1/untracked ?? dir2/modified ?? dir2/untracked -?? expect -?? output ?? untracked EOF @@ -755,8 +735,6 @@ Untracked files: dir1/untracked dir2/modified dir2/untracked - expect - output untracked EOF @@ -772,8 +750,6 @@ A dir2/added ?? dir1/untracked ?? dir2/modified ?? dir2/untracked -?? expect -?? output ?? untracked EOF @@ -798,8 +774,6 @@ Untracked files: dir1/untracked dir2/ - expect - output untracked EOF @@ -848,8 +822,6 @@ Untracked files: dir1/untracked dir2/modified dir2/untracked - expect - output untracked EOF @@ -870,8 +842,6 @@ A sm ?? dir1/untracked ?? dir2/modified ?? dir2/untracked -?? expect -?? output ?? untracked EOF test_expect_success 'status -s submodule summary is disabled by defaul
Re: [PATCHv2 2/2] commit/status: show the index-worktree diff with -v -v
Michael J Gruber writes: > Junio C Hamano venit, vidit, dixit 03.03.2015 22:26: >> Michael J Gruber writes: >> >>> +diff --git INDEX=staged-for-commit/dir1/modified >>> WORKTREE=not-staged-for-commit/dir1/modified >>> +index e69de29..d00491f 100644 >>> +--- INDEX=staged-for-commit/dir1/modified >>> WORKTREE=not-staged-for-commit/dir1/modified >> >> This might be OK for a project like Git itself, but I suspect people >> with long pathnames (like, eh, those in Java land) would not >> appreciate it. >> >> Wouldn't mnemonic prefix, which the users are already familiar with, >> be the most suitable tool for this disambiguation? After all that >> was what it was invented for 8 years ago. > > Well...: > >> or it may want to even be like this: >> >> diff --git a/A b/A >> ... >> diff --git to-be-committed/A left-out-of-the-commit/A >> ... >> diff --git a/B b/B >> ... >> >> by using a custom, unusual and easy-to-notice prefixes. > > Your idea was to use these verbous prefixes so that one recognizes the > different types of diffs, and so that we don't need to sort them by file. Yeah, but I can become wiser over time and change my opinion, no ;-)? As to pairing the diffs by paths so that c/i and i/w diffs for the same path come together, which I mentioned in the older message you quoted, I think what you said in response made sense, i.e. "the intention was to show the diff for the two categories of changes which "git status" lists without diff already". So I'd prefer showing c/i diff and then optionall i/w diff like you did, without mixing them together. > I'm happy with c/,i/ and i/,w/ and without sorting. Maybe we would need > headings between the two diffs then? Yup. The i/w diff is a new thing and a heading before it to explain what it is would be very helpful for the users to understand what they are looking at. A new heading before c/i diff might help but it may be OK without. E.g. something along the following lines Changes to be committed: modified: foo Changes left in the working tree: modified: bar -- Changes to be committed diff --git c/foo i/foo ... -- Changes left in the working tree diff --git i/bar w/bsar ... -- 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: zip files created with git archive flags text files as binaries
Am 23.02.2015 um 20:30 schrieb René Scharfe: Am 23.02.2015 um 14:58 schrieb Ulrike Fischer: The zip contained four text files and a pdf. The CTAN maintainers informed me that all files in the zip are flagged as binaries and this makes it difficult for them to process them further (they want to correct line feeds of the text files: http://mirror.ctan.org/tex-archive/help/ctan/CTAN-upload-addendum.html#crlf) By the way, a workaround for CTAN could be to extract the files using unzip and zip them again using Info-ZIP zip (one of the "good zip programs" mentioned on that website). The result will be a ZIP file with text files flagged appropriately. Just saying. Would it be possible to correct the zip-backend so that it flags text files correctly? Or alternativly could one configure git archive to use another zip programm? We would have to detect the line ending format (DOS, Unix, Macintosh, etc.) of each file, then set the attribute "t" (text) and the host system. The detection would slow down archive creation a bit and the resulting files would be different, of course, so this feature should only be enabled by a new command line option. I'll take a look. Actually its easier than that. unzip -a (with end-of-line conversion) doesn't care for the actual format, it simply converts all occurrences of CR, CRLF and LF to the appropriate newline chars for the platform it is running on if the text flag is set. Files with mixed line endings are normalized, i.e. they have a consistent end-of-line style afterward. I'll send a first patch shortly. René -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 2/3] sha1_file: implement changes for "cat-file --literally -t"
Karthik Nayak writes: > add "sha1_object_info_literally()" which is to be used when > the "literally" option is given to get the type of object > and print it, using "sha1_object_info_extended()". > > add "unpack_sha1_header_literally()" to unpack sha headers > which may be greater than 32 bytes, which is the threshold > for a regular object header. > > modify "sha1_loose_object_info()" to include a flag argument > and also include changes to call "unpack_sha1_header_literally()" > when the literally flag is passed. Also copies the obtained > type to the typename field of object_info. > > modify "sha1_object_info_extended()" to call the function > "sha1_loose_object_info()" with flags. > > Signed-off-by: Karthik Nayak > --- > sha1_file.c | 84 > +++-- > 1 file changed, 77 insertions(+), 7 deletions(-) > > diff --git a/sha1_file.c b/sha1_file.c > index 69a60ec..1068ca0 100644 > --- a/sha1_file.c > +++ b/sha1_file.c > @@ -1564,6 +1564,36 @@ int unpack_sha1_header(git_zstream *stream, unsigned > char *map, unsigned long ma > return git_inflate(stream, 0); > } > > +static int unpack_sha1_header_literally(git_zstream *stream, unsigned char > *map, > + unsigned long mapsize, > + struct strbuf *header) > +{ > +... > +} Looks suspiciously familiar... > +int sha1_object_info_literally(const unsigned char *sha1) > +{ > + enum object_type type; > + struct strbuf sb = STRBUF_INIT; > + struct object_info oi = {NULL}; > + > + oi.typename = &sb; > + oi.typep = &type; > + if (sha1_object_info_extended(sha1, &oi, LOOKUP_LITERALLY) < 0) > + return -1; > + if (*oi.typep > 0) > + printf("%s\n", typename(*oi.typep)); > + else > + printf("%s\n", oi.typename->buf); > + strbuf_release(oi.typename); > + return 0; > +} > + NAK. Please don't add end-user facing final output to sha1_file.c; instead have the caller use a helper function like this one to extract necessary information and perform the end-user interaction there. -- 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: feature request: excluding files/paths from "git grep"
Noel Grandin writes: > On 2015-03-02 02:50 PM, Trevor Saunders wrote: >> I think they solve somewhat different problems, but maybe my problem >> is so specialized I should just have a wrapper around grep that >> changes defaults. Trev > > I'm with Trevor on this one. While I see the appeal of the generality > of a macro solution, this is really just about convenience for me on a > per-project basis. > > As in, while working on a specific project, I sometimes just want to > exclude, for the time being, a bunch of stuff from 'git grep'. The key word here is "for the time being", though. What would you do once you are done with the "for the time being" activity? "git config --unset"? If you forget to do so when the "for the time being" activity ends, and then you try to run 'git grep' and see that you did not get expected hits from hierarchies that you set to exclude earlier, you either (1) get misled to a wrong decison based on that false non-hit, or (2) start scratching your head, wasting time trying to figure out why 'git grep' is not hitting, no? I expect the answer might be "No, I won't forget; I am very well organized and you do not have to worry for me". But a feature is an invitation for people other than yourself, so... -- 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 describe --contains doesn't work properly for a commit
Jeff King writes: > On Wed, Mar 04, 2015 at 04:06:17PM +0100, Michael J Gruber wrote: > >> Complexity: Was that due to replace refs? Other than that, it seemed to >> be simple: max(parent generation numbers)+1. > > Calculating them is simple. Caching and storage is the bigger question. Yes, also having to handle the ones whose generation numbers haven't been computed yet adds to the complexity. This one, and $gmane/264101, are a few instances of this known issue raised here recently. I have been wondering if we can do something along the following (these are not alternatives) as a cheaper workaround: (1) Introduce '--skewed-timestamps[=(allow|warn|reject)' to all commands that create new commit objects. If the committer timestamp being used is older than any of the parent commits, "warn" or "reject" depending on the setting. Make 'reject' the default for commands that are purely local (i.e. recording your own progress by cherry-picking, committing, rebasing, reverting, etc.) and 'warn' the default for commands that merge other peoples' history that you may lack the power to rewind and correct (e.g. 'pull' and 'merge' from remote tracking refs). (2) Compute a bitmap whose timestamps are suspect when we pack to mark commits. When revision.c:limit_list() tries to see if there still are interesting commits, an UNINTERESTING commit marked as such shouldn't be counted as "not interesting because it is old enough". Use the same hint in the walker used in "describe --contains". -- 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] t5516-fetch-push: Correct misspelled pushInsteadOf
Michael J Gruber writes: >> I am not sure how the intention of the commit 1c2eafb8 (Add >> url..pushInsteadOf: URL rewriting for push only, 2009-09-07), >> which introduced the behaviour verified by this test, interacts with >> the desire to redefine what URL and pushURL mean in our recent past, >> what e.g. e6196ae1 (remote: add --fetch and --both options to set-url, >> 2014-11-25) wanted to do, though. Thoughts? >> > > Wow. That looks strange to me on first read. Sorry I didn't catch it > back then. "--fetch" sets both url and pushurl?? Yeah, sounds crazy, no? Taken in isolation without consideration around the InsteadOf rewriting, the change sort-of makes sense, though. But I do not know if the combined whole makes much sense. > So, for definiteness sake, I'll use "url" and "pushurl" for the config > names and struct members (which the config values end up in), and I'll > use "URL for fetch" and "URL for push" for the URLs that git will use > for fetch resp. push. > > If there is no "pushurl": > "url" will be used as the "URL for fetch" and as the "URL for push". > if we are pushing and there is "pushinsteadof" for (part of) "url" > subsitute that within "url" > else if there is "insteadof" for (part of) "url" > substitute that within "url" > > If there is a "pushurl": > "url" will be used as the "URL for fetch" and "pushurl" as the "URL > for push". > if we are pushing and there is "insteadof" for (part of) "pushurl" > substitute that within "pushurl" > else if we are fetching and there is "insteadof" for (part of) "url" > substiute that within "url" These make sense to me. > I don't know what "remote set-url" does, I think the whole remote > command as it is does not fit in well with our UI, but that applies to > other commands with non-option subcommands as well (or bad). I would > have hoped it is set-url and set-pushurl to set those two config > options. I would have hoped so, too, but I don't use "git remote" myself (as you said, it is an odd man out and there is nothing it and only it can do that is essential for my everyday workflow), so... ;-) > I rarely use pushurl, but could use it to override url, or to do the > above magic on a case by case level, such as if I need to specify > different usernames for the same host. > > So, that's my understanding of how these things are supposed to work and > why it is useful. Yes, I am not questioning the usefulness of insteadof magic. I am wondering if that "--fetch sets both?" breaks the expectation of those who rely on it. -- 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-describe considers WC dirty incorrectly when using --git-dir
On Wed, Mar 4, 2015 at 11:54 AM, Junio C Hamano wrote: > Chris Pimlott writes: > >> folio:text chris$ git --git-dir=../.git describe --always --dirty >> c0edd63-dirty > > I have a feeling that this is not limited to describe at all. With > the --git-dir option, you are telling Git that your GIT_DIR is over > there and (by not using --work-tree together with that option) you > are telling Git that you do not want Git to guess where the working > tree is (instead, you are telling Git that you are at the top of the > working tree), no? Ah, my apologies, you are correct. I was not aware of --work-tree and didn't realize that specifying --git-dir would turn off the normal working tree discovery process. 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] xmerge.c: fix xdl_merge to conform with the manual
Anton Trunov writes: > For the code version before applying this patch the following scenario > will take place if "git merge -Xignore-all-space remote" gets executed. > > base file: > 1st line > 2nd line > > master file: > 1st line > 2nd line with substantial change > > remote file: > 1st line > 2nd line > > merge result file: > 1st line > 2nd line with substantial change > > So essentially it does what "git merge -s ours remote" does in case if > all their changes are trivial. > This seems like reasonable solution to me: we _are_ trying to ignore > whitespace changes and we are explicit about it. Now, the above makes readers wonder what happens when you merged master into the remote. I.e. in the opposite direction. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] Disclaimer about the number of slots.
Matthieu Moy writes: > Jeff King writes: > >> Maybe something along the lines of "If you are really excited about >> working on git, we would love to see your application; if you are just >> looking for a random project, there may be a lot of competition for a >> small number of slots". Except I am not quite sure how to phrase "just >> looking for a random project" that does not sound quite so demeaning. > > Yes, that's the idea, but I didn't find a way to say "if you're one of > the great students we're looking for, please come in, otherwise go > away", so I went for a neutral wording that would let the reader infer > it by themselves ;-). What is this? Matt and Jeff's comedy hour? You two made me laugh hard ;-) -- 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-describe considers WC dirty incorrectly when using --git-dir
Chris Pimlott writes: > It seems that git-describe always thinks that working copy is dirty if > you are not in the WC root and you explicitly specify the .git > directory location using --git-dir: > > # set up test repo > folio:~ chris$ mkdir repo && cd repo > folio:repo chris$ mkdir text && echo hi > text/hi.txt > folio:repo chris$ git init . && git add . && git commit -m "text/hi.txt" > Initialized empty Git repository in /home/chris/repo/.git/ > [master (root-commit) c0edd63] text/hi.txt > 1 file changed, 1 insertion(+) > create mode 100644 text/hi.txt > > # git-describe from non-root directory > folio:repo chris$ cd text > folio:text chris$ git describe --always --dirty > c0edd63 > folio:text chris$ git --git-dir=../.git describe --always --dirty > c0edd63-dirty > folio:text chris$ git --git-dir=$(git rev-parse > --show-toplevel)/.git describe --always --dirty > c0edd63-dirty I have a feeling that this is not limited to describe at all. With the --git-dir option, you are telling Git that your GIT_DIR is over there and (by not using --work-tree together with that option) you are telling Git that you do not want Git to guess where the working tree is (instead, you are telling Git that you are at the top of the working tree), no? -- 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-describe considers WC dirty incorrectly when using --git-dir
It seems that git-describe always thinks that working copy is dirty if you are not in the WC root and you explicitly specify the .git directory location using --git-dir: # set up test repo folio:~ chris$ mkdir repo && cd repo folio:repo chris$ mkdir text && echo hi > text/hi.txt folio:repo chris$ git init . && git add . && git commit -m "text/hi.txt" Initialized empty Git repository in /home/chris/repo/.git/ [master (root-commit) c0edd63] text/hi.txt 1 file changed, 1 insertion(+) create mode 100644 text/hi.txt # git-describe from non-root directory folio:repo chris$ cd text folio:text chris$ git describe --always --dirty c0edd63 folio:text chris$ git --git-dir=../.git describe --always --dirty c0edd63-dirty folio:text chris$ git --git-dir=$(git rev-parse --show-toplevel)/.git describe --always --dirty c0edd63-dirty # git-describe from root directory folio:repo chris$ cd .. folio:repo chris$ git describe --always --dirty c0edd63 folio:repo chris$ git --git-dir=.git describe --always --dirty c0edd63 folio:repo chris$ git --git-dir=$(git rev-parse --show-toplevel)/.git describe --always --dirty c0edd63 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH 0/3] protocol v2
On Wed, Mar 4, 2015 at 4:05 AM, Duy Nguyen wrote: > On Wed, Mar 4, 2015 at 11:27 AM, Shawn Pearce wrote: >> Let me go on a different tangent a bit from the current protocol. >> >> http://www.grpc.io/ was recently released and is built on the HTTP/2 >> standard. It uses protobuf as a proven extensibility mechanism. >> Including a full C based grpc stack just to speak the Git wire >> protocol is quite likely overkill, but I think the embedding of a >> proven extensible format inside of a bi-directional framed streaming >> system like HTTP/2 offers some good guidance. > > I'll take this as "learn from grpc, not just reuse grpc" Correct, that was what I was trying to say and I just wrote it poorly. HTTP 1.x, HTTP/2 and protobuf have proven themselves to be fairly open to extension and working well in the wild for transports. There is useful guidance there that we should draw from to try and leave doors open for the future. HTTP/2, protobuf and grpc are fairly complex. I consider any one of them too complicated for Git specific use. However HTTP/2 is probably the future of HTTP stacks so we may see it show up in libcurl or something as popular as libcurl in another 10 years. Hg had some reasonably sane ideas about building the wire protocol to work well on HTTP 1.x upfront rather than Git tacking it on much later. >> Network protocol parsing is hard. Especially in languages like C where >> buffer overflows are possible. Or where a client could trivially DoS a >> server by sending a packet of size uint_max and the server naively >> trying to malloc() that buffer. Defining the network protocol in an >> IDL like protobuf 3 and being machine generated from stable well >> maintained code has its advantages. > > I'm still studying the spec, so I can't comment if using IDL/protobuf3 > is a good idea yet. > > But I think at least we can avoid DoS by changing the pkt-line (again) > a bit: the length 0x means that actual length is 0xfffe and the > next pkt-line is part of this pkt-line. Higher level (upload-pack or > fetch-pack, for example) must set an upper limit for packet_read() so > it won't try to concatenate pkt-lines forever. pkt-line is a reasonably simple and efficient framing system. A 64 KiB pkt-line frame only costs ~0.0061% overhead; ~0.0076% overhead if you are a pack stream in a side-band-64k channel. That is probably more efficient than HTTP/2 or SSL framing. I see no reason to attempt to try and reduce that overhead further. 64 KiB frame size is enough for anyone to move data efficiently with these headers. In practice you are going to wrap that up into SSH or SSL/TLS and those overheads are so much higher it doesn't matter we have a tiny loss here. I think a mistake in the wire protocol was making the pkt-line length human readable hex, but the sideband channel binary. _If_ we redo the framing the only change I would make is making the side band readable. Thus far we have only used 0, 1, 2 for sideband channels. These could easily be moved into human readable channel ids: 'd': currently sideband 0; this is the application data, aka the pack data 'p': currently sideband 1; this is the progress stream for stderr 'e': currently sideband 2; there was an error, data in this packet is the message text, and the connection will shutdown after the packet. And then leave all other sideband values undefined and reserved for future use, just like they are all open today. I am not convinced framing changes are necessary. I would fine with leaving the sideband streams as 0,1,2... but if we want a text based protocol for ease of debugging we should be text based across the board and try very hard to avoid these binary values in the framing, or ever needing to use a magical NUL byte in the middle of a packet to find a gap in older parsers for newer data. If you want to build a larger stream like ref advertisement inside a pkt-line framing without using a pkt-line per ref record, you should follow the approach used by pack data streams where it uses the 5 byte side-band pkt-line framing and a side-band channel is allocated for that data. Application code can then run a side-band demux to yank out the inner stream and parse it. It may be simpler to restrict ref names to be smaller than 64k in length so you have room for framing and hash value to be transferred inside of a single pkt-line, then use the pkt-line framing to do the transfer. Today's upload-pack ref advertisment has ~25% overhead. Most of that is in the duplicated tag name for the peeled refs/tags/v1.0^{} lines. If you drop those names (but keep the pkt-line and SHA-1), its only about 8% overhead above the packed-refs file. I think optimization efforts for ref advertisement need to focus on reducing the number of refs sent back and forth, not shrinking the individual records down smaller. Earlier in this thread Junio raised a point that the flush-pkt is confusing because it has way too many purposes. I agree. IIRC we have 0001
Re: git describe --contains doesn't work properly for a commit
On Wed, Mar 04, 2015 at 04:06:17PM +0100, Michael J Gruber wrote: > > 3. Introduce a more trust-worthy mechanism for ordering commits. The > > timestamp here is really just a proxy for the oft-discussed > > "generation number" of the commit within the graph. We've avoided > > adding generation numbers because of the storage/complexity issues. > > Hmmh. > > Storage: one int (or maybe less) per commit doesn't sound too bad. We > can probably do without on bare repos by default. > > Complexity: Was that due to replace refs? Other than that, it seemed to > be simple: max(parent generation numbers)+1. Calculating them is simple. Caching and storage is the bigger question. When we do we generate them? Where do we store them? What do we do with replace-refs and grafts? I think the answers are "at repack time", "in an auxiliary file alongside the pack idx", and "we turn it off completely when these features are in use". See: http://thread.gmane.org/gmane.comp.version-control.git/214916 for a sample implementation. > ... or can reachability bitmaps help??? Sometimes. If you are asking about --contains traversals, then bitmaps can let you stop the traversal early. We have some patches that do this that are running in production at GitHub, but they are kind of gnarly. One of my goals is to clean them up and get them upstream. It's also part of why I didn't pursue the series above further. Making "--contains" faster is one goal, but making "rev-list --objects --all" faster was more important (since we do it for every fetch). And making commits faster is only half the equation there. -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 2/2] docs: explain behaviour of remote add without tag option
On Wed, Mar 04, 2015 at 04:46:47PM +0100, Michael J Gruber wrote: > > So, how does this relate to: > > http://permalink.gmane.org/gmane.comp.version-control.git/264592 > > I don't mind you taking this over at all, it's just that we should keep > things together where the discussion begun. > > Michael Sorry, I totally missed that. Wasn't my goal to take it over. I have a filter to seperate out patches from other e-mail, so the thread got separated. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] docs: explain behaviour of remote add without tag option
Kevin Daudt venit, vidit, dixit 04.03.2015 00:12: > Only behaviour with these options are currently explained. Add > explanation what the default behaviour is. > > Signed-off-by: Kevin Daudt > --- > Documentation/git-remote.txt | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/Documentation/git-remote.txt b/Documentation/git-remote.txt > index a77607b..5bde0e1 100644 > --- a/Documentation/git-remote.txt > +++ b/Documentation/git-remote.txt > @@ -58,6 +58,9 @@ remote repository. > With `--no-tags` option, `git fetch ` does not import tags from > the remote repository. > + > +With neither `--tags` or `--no-tags` set, imports only tags that are > +reachable from downloaded history. > ++ > With `-t ` option, instead of the default glob > refspec for the remote to track all branches under > the `refs/remotes//` namespace, a refspec to track only `` > So, how does this relate to: http://permalink.gmane.org/gmane.comp.version-control.git/264592 I don't mind you taking this over at all, it's just that we should keep things together where the discussion begun. Michael -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: An interesting opinion on DVCS/git
David Lang venit, vidit, dixit 04.03.2015 01:53: > On Tue, 3 Mar 2015, Shawn Pearce wrote: > >> On Sun, Mar 1, 2015 at 7:29 PM, Stefan Beller wrote: >>> bitquabit.com/post/unorthodocs-abandon-your-dvcs-and-return-to-sanity >> >> Indeed, a DVCS like Git or Hg does not fit everyone. And neither do >> centralized systems like Subversion. Choice is good. >> >> However... I found some passages troubling for Git, e.g.: >> >> ---snip--- >> Git is so amazingly simple to use that APress, a single publisher, >> needs to have three different books on how to use it. It’s so simple >> that Atlassian and GitHub both felt a need to write their own online >> tutorials to try to clarify the main Git tutorial on the actual Git >> website. It’s so transparent that developers routinely tell me that >> the easiest way to learn Git is to start with its file formats and >> work up to the commands. >> ---snap--- >> >> We have heard this sort of feedback for years. But we have been unable >> to adequately write our own documentation or clean up our man pages to >> be useful to the average person who doesn't know why the --no-frobbing >> option doesn't disable the --frobinator option to the >> --frobbing-subcommand of git frob. :( >> >> http://git-man-page-generator.lokaltog.net/ shouldn't exist and >> shouldn't be funny. Yet it does. :( > > As for the different online tutorials, I'll point out that every university > that > supports it's students using Thunderbird has it's own version of a tutorial > on > how to use and configure Thunderbird. The question is if they are coverying > their one use case of how to use git with their service, or if they are > trying > to duplicate the git documentation. > > > There are two reasons for having multiple books out for a piece of software > > 1. the software is horribly complicated to use, even for beginners > > 2. the software is extremely powerful, to to understand all the different > advanced options, and when to use them, takes a lot of explination > > In the case of git, there's a bit of both. > > Part of the problem is that there are so many different ways to use it (all > in > common use) that there isn't one simple set of insructions that will be right > in > all the different use cases (thus the value of services that force users to > operate in one specific model providing a tutorial in how to use it with > their > service) > > At this point, Internet Lore says "git is hard to use", and if you approach > any > software with that attitude, you will find lots of things to point at to > justify > your opinion. > > I'm not saying that there isn't room for improvement, I'm just saying that > the > evidence provided is not as one-sided as they make it sound. > > David Lang > Yes, that article has a few really weak lines of arguments, such as the tutorial count. Also, that advice to "learn git from the file formats", which I hope means something like "learn the structure, not recipes" is good advice for learning any powerful toolset - rediculing it is not quite a proof of intelligence. And I don't think there's anything we have to regret about the basic architecture of (the DAG/object structure of) git. (OK, the various signature formats ;) That being said, we all know how often we want to change the UI and backwards compatibility keeps us from doing so. The UI could really benefit from a fresh start, where, based on what we know now, we first think about: - How do we structure/denote subcommands within commands? E.g. to dash or not to dash, default subcommand etc. - How do we structure "read" commands vs. "write" commands? E.g. the pairs "branch [-l]"/"branch ", "log"/"commit" etc. - How do we name options consistently? E.g. -n=--dry-run everywhere - How do we make working with the special git concepts more natural? E.g. index, detached heads. Michael -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: git describe --contains doesn't work properly for a commit
Jeff King venit, vidit, dixit 04.03.2015 11:54: > On Thu, Feb 26, 2015 at 03:23:14PM +0100, Michal Hocko wrote: > >> The commit in the middle was ammended to have committer date in the >> past. >> $ git describe --contains d63972e4e4e7eda0444e56739ad09bfbc476b9bd >> tag~1 >> >> but >> $ git describe --contains 108a0d5972fd2e5f25b2f38cfd2fee73031ff9d3 >> fatal: cannot describe '108a0d5972fd2e5f25b2f38cfd2fee73031ff9d3' >> >> I guess this is the same issue reported previously here: >> http://git.661346.n2.nabble.com/git-describe-contains-fails-on-given-tree-td5448286.html > > Yes, the "describe --contains" algorithm uses timestamps to cut off the > traversal, so it can do the wrong thing if there's clock skew. It has a > "slop" margin of one day, but skew larger than that can fool it. > >> Can this be fixed somehow or it would lead to other kind of issues? > > The options are basically: > > 1. Stop cutting off the traversal based on timestamps. This will make > the common case of valid timestamps much slower, though, as it will > have to walk all the way to the roots. > > 2. Use a different slop mechanism. For example, keep walking up to 5 > commits past a commit suspected to be past the cutoff. This is > relatively easy to do (we do it for "--since" checks), and would > catch your case above. But of course it does not catch all cases of > skew. > > 3. Introduce a more trust-worthy mechanism for ordering commits. The > timestamp here is really just a proxy for the oft-discussed > "generation number" of the commit within the graph. We've avoided > adding generation numbers because of the storage/complexity issues. Hmmh. Storage: one int (or maybe less) per commit doesn't sound too bad. We can probably do without on bare repos by default. Complexity: Was that due to replace refs? Other than that, it seemed to be simple: max(parent generation numbers)+1. ... or can reachability bitmaps help??? Michael -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Salvaging borked project history
Mason wrote: I was planning to write 'git diff -q commit^ commit' to test for empty commits. Looks like I'll be needing 'git diff commit^ commit | wc -l' instead? I wrote a script to generate the list of empty commits. git log --format="%h" --reverse 413cb08.. | while read H do if git diff --quiet $H^ $H; then echo $H; fi done >empty_commits But it turns out all this is unnecessary, as git-rebase will automatically filter empty commits! :-) Unless given the --keep-empty option, I presume. Thanks to everyone involved in making this great tool. Regards. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/2] completion: add a test for __git_remotes() helper function
The test checks that both remotes under '$GIT_DIR/remotes' and remotes in the config file are listed. Signed-off-by: SZEDER Gábor --- t/t9902-completion.sh | 19 +++ 1 file changed, 19 insertions(+) diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh index f10a752..7a883d1 100755 --- a/t/t9902-completion.sh +++ b/t/t9902-completion.sh @@ -351,6 +351,25 @@ test_expect_success '__gitcomp_nl - doesnt fail because of invalid variable name __gitcomp_nl "$invalid_variable_name" ' +test_expect_success '__git_remotes - list remotes from $GIT_DIR/remotes and from config file' ' + cat >expect <<-EOF && + remote_from_file_1 + remote_from_file_2 + remote_in_config_1 + remote_in_config_2 + EOF + test_when_finished "rm -rf .git/remotes" && + mkdir -p .git/remotes && + >.git/remotes/remote_from_file_1 && + >.git/remotes/remote_from_file_2 && + test_when_finished "git remote remove remote_in_config_1" && + git remote add remote_in_config_1 git://remote_1 && + test_when_finished "git remote remove remote_in_config_2" && + git remote add remote_in_config_2 git://remote_2 && + __git_remotes >actual && + test_cmp expect actual +' + test_expect_success 'basic' ' run_completion "git " && # built-in -- 2.1.2.1369.g8751039 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/2] completion: simplify __git_remotes()
The __git_remotes() helper function lists the remotes from the config file by processing the output of a 'git config' query. A simple 'git remote' produces the exact same output, so run that instead. Remotes under '$GIT_DIR/remotes' are still listed by running 'ls -1', because 'git remote' unfortunately ignores them. Signed-off-by: SZEDER Gábor --- contrib/completion/git-completion.bash | 7 ++- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash index c21190d..f5ae5e3 100644 --- a/contrib/completion/git-completion.bash +++ b/contrib/completion/git-completion.bash @@ -411,12 +411,9 @@ __git_refs_remotes () __git_remotes () { - local i IFS=$'\n' d="$(__gitdir)" + local d="$(__gitdir)" test -d "$d/remotes" && ls -1 "$d/remotes" - for i in $(git --git-dir="$d" config --get-regexp 'remote\..*\.url' 2>/dev/null); do - i="${i#remote.}" - echo "${i/.url*/}" - done + git --git-dir="$d" remote } __git_list_merge_strategies () -- 2.1.2.1369.g8751039 -- 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] contrib/completion: suppress stderror in bash
Hi, Quoting SZEDER Gábor : Hi, Quoting Junio C Hamano : SZEDER Gábor writes: @@ -412,7 +412,7 @@ __git_refs_remotes () __git_remotes () { local i IFS=$'\n' d="$(__gitdir)" - test -d "$d/remotes" && ls -1 "$d/remotes" + test -d "$d/remotes" && ls -1 "$d/remotes" 2>/dev/null for i in $(git --git-dir="$d" config --get-regexp 'remote\..*\.url' 2>/dev/null); do i="${i#remote.}" echo "${i/.url*/}" Do I smell some bitrotting here? This function just lists all the defined remotes, first by listing the directories under refs/remotes to get the "legacy" remotes and then loops over 'git config's output to get the "modern" ones. This predates the arrival of the 'git remote' command in January 2007, so it was really a long time ago. We should just run 'git remote' instead, shouldn't we? Perhaps. Is it sufficient to just make __git_remotes() a thin wrapper around, i.e. __git_remotes () { git remotes } or do we need to munge its output further (I didn't look)? Well, just like in other cases where we run git from the completion script, we need a '--git-dir="$(__gitdir)"' as well, because the user can specify the path to a different repo via $GIT_DIR or on the command line. Other than that it seems we are OK. Docs say "With no arguments, shows a list of existing remotes." and as far as I can tell, on MSysGit, it does so without any funny formatting. Oh, look what forgotten treasure did I stumble upon in the vaults: https://github.com/szeder/git/commit/e4e3760c15b485b9ff4768e13050f4b19b5968b8 A two and a half year old commit in my old git repo doing the same... completely forgotten :) Unfortunately, however, it's not quite that simple, because 'git remote' doesn't list remotes under '$GIT_DIR/remotes'. Or at least I would have expected the following test to work, but it does not: diff --git a/t/t5505-remote.sh b/t/t5505-remote.sh index 17c6330..6a4c139 100755 --- a/t/t5505-remote.sh +++ b/t/t5505-remote.sh @@ -734,6 +734,15 @@ Pull: refs/heads/master:refs/heads/origin Pull: refs/heads/next:refs/heads/origin2 EOF +test_expect_success 'list remote in $GIT_DIR/remotes' ' + mkdir .git/remotes && + test_when_finished "rm -rf .git/remotes" && + cat remotes_origin >.git/remotes/remote_from_file && + git remote >actual && + echo remote_from_file >expect && + test_cmp expect actual +' + test_expect_success 'migrate a remote from named file in $GIT_DIR/remotes' ' git clone one five && origin_url=$(pwd)/one && because listing remotes is implemented by for_each_remote(), which only reads remotes from the config file. Now, considering how old 'git remote' is, there were plenty of time for someone to miss this functionality and complain about it, but since it's still not implemented is probably a good sign that noone has actually missed it. And I don't think it's worth implementing it now just to shave off two more lines from the completion script. Anyway, 'git remote' could still replace that 'git config' query. I have the patches ready and it seems I got send-email working, so they'll follow in a minute or two. Best, Gábor -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH 0/3] protocol v2
On Wed, Mar 4, 2015 at 11:27 AM, Shawn Pearce wrote: > Let me go on a different tangent a bit from the current protocol. > > http://www.grpc.io/ was recently released and is built on the HTTP/2 > standard. It uses protobuf as a proven extensibility mechanism. > Including a full C based grpc stack just to speak the Git wire > protocol is quite likely overkill, but I think the embedding of a > proven extensible format inside of a bi-directional framed streaming > system like HTTP/2 offers some good guidance. I'll take this as "learn from grpc, not just reuse grpc" > Network protocol parsing is hard. Especially in languages like C where > buffer overflows are possible. Or where a client could trivially DoS a > server by sending a packet of size uint_max and the server naively > trying to malloc() that buffer. Defining the network protocol in an > IDL like protobuf 3 and being machine generated from stable well > maintained code has its advantages. I'm still studying the spec, so I can't comment if using IDL/protobuf3 is a good idea yet. But I think at least we can avoid DoS by changing the pkt-line (again) a bit: the length 0x means that actual length is 0xfffe and the next pkt-line is part of this pkt-line. Higher level (upload-pack or fetch-pack, for example) must set an upper limit for packet_read() so it won't try to concatenate pkt-lines forever. -- Duy -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] grep: correct help string for --exclude-standard
On Wed, Mar 4, 2015 at 6:26 PM, Jeff King wrote: >> > It might be nice if parseopt had a PARSE_OPT_NEGHELP option or something >> > to show the "--no-" form. >> >> Regardless, yes it would be nice to have something like this. I think >> there are places that can make use of this. > > Grepping around, it looks like the best form would be an OPT_NEGBOOL > that acts like a boolean but negates the truth value, and advertises the > negative form. We have a lot of: > > OPT_BOOL('n', "no-checkout", &option_no_checkout, >N_("don't create a checkout")) > > where countermanding an earlier "--no-checkout" has to be spelled as > "--no-no-checkout", rather than "--checkout". If we could write: > > OPT_NEGBOOL('n', "checkout", ...) > > that would be nicer. But the short option is a bit weird. We'd want: > > -n: option_no_checkout=true > --checkout: option_no_checkout=false > --no-checkout: option_no_checkout=true > > That is, we flip the sense of the long option, but the short option > still yields "true". I think that would be useful, but it sure is weird > to explain. Yeah it looks confusing.. How about leaving that first arg as the short option "checkout" and move 'n' elsewhere? Something like this OPT_NEGBOOL(0, "checkout", 'n', ) -- Duy -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] grep: correct help string for --exclude-standard
On Wed, Mar 04, 2015 at 06:13:32PM +0700, Duy Nguyen wrote: > On Wed, Mar 4, 2015 at 5:16 PM, Jeff King wrote: > > Hmm. If the default is "--exclude-standard", then what expect people to > > use is "--no-exclude-standard". Would it make more sense to list that in > > the "-h" output? > > I thought about it and actually edited git-grep man page to clarify > the default, only to find out this. When --untracked is used, > --exclude-standard is the default. When --no-index is used, > --no-exclude-standard is the default. Can't have it both ways. This is > already mentioned with the subtle phrase "only useful with...". Yuck. :) I agree your patch is the right thing to do, then. > > It might be nice if parseopt had a PARSE_OPT_NEGHELP option or something > > to show the "--no-" form. > > Regardless, yes it would be nice to have something like this. I think > there are places that can make use of this. Grepping around, it looks like the best form would be an OPT_NEGBOOL that acts like a boolean but negates the truth value, and advertises the negative form. We have a lot of: OPT_BOOL('n', "no-checkout", &option_no_checkout, N_("don't create a checkout")) where countermanding an earlier "--no-checkout" has to be spelled as "--no-no-checkout", rather than "--checkout". If we could write: OPT_NEGBOOL('n', "checkout", ...) that would be nicer. But the short option is a bit weird. We'd want: -n: option_no_checkout=true --checkout: option_no_checkout=false --no-checkout: option_no_checkout=true That is, we flip the sense of the long option, but the short option still yields "true". I think that would be useful, but it sure is weird to explain. -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: feature request: excluding files/paths from "git grep"
On 2015-03-02 02:50 PM, Trevor Saunders wrote: I think they solve somewhat different problems, but maybe my problem is so specialized I should just have a wrapper around grep that changes defaults. Trev I'm with Trevor on this one. While I see the appeal of the generality of a macro solution, this is really just about convenience for me on a per-project basis. As in, while working on a specific project, I sometimes just want to exclude, for the time being, a bunch of stuff from 'git grep'. Mind you, I use 'git grep' a hang of a lot during development, since it is so powerful, so maybe that's just me. Thanks, Noel Grandin Disclaimer: http://www.peralex.com/disclaimer.html -- 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: [RFH] GSoC 2015 application
On Wed, Mar 4, 2015 at 5:31 PM, Jeff King wrote: > On Thu, Feb 26, 2015 at 08:10:38PM +0700, Duy Nguyen wrote: > >> On Thu, Feb 19, 2015 at 2:14 AM, Jeff King wrote: >> > Where I really need help now is in the "ideas" page: >> > >> > http://git.github.io/SoC-2015-Ideas.html >> >> Is this too ambitious for a summer? I suspect the answer is yes, but anyway.. >> >> Due to http limitations and stateless decision, a lot of data is sent >> back and forth during have/want negotiation for smart-http. I wonder >> if we could implement the "long polling" scheme in a CGI program. The >> program terminates HTTP requests and recreates a full duplex >> connection for upload-pack to talk to the client. upload-pack falls >> back to the normal mode, used by git:// and ssh://. > > So basically Git-over-TCP-over-HTTP? :) Yes. The hidden agenda was, if it works well, we might be able to deprecate smart-http one day. That day, if happens, would be in far future though. By that time hopefully we could just use http2 insteadof tcp-over-http1. > I'm not sure it is too ambitious in terms of actual implementation time, > but I think the design work may exceed what most students are capable > of. Not to mention that I think it can be man-in-the-middle attacked if we're not careful (e.g. send to session token in unencrypted). Which makes it less appealing to me. -- Duy -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] grep: correct help string for --exclude-standard
On Wed, Mar 4, 2015 at 5:16 PM, Jeff King wrote: > Hmm. If the default is "--exclude-standard", then what expect people to > use is "--no-exclude-standard". Would it make more sense to list that in > the "-h" output? I thought about it and actually edited git-grep man page to clarify the default, only to find out this. When --untracked is used, --exclude-standard is the default. When --no-index is used, --no-exclude-standard is the default. Can't have it both ways. This is already mentioned with the subtle phrase "only useful with...". > It might be nice if parseopt had a PARSE_OPT_NEGHELP option or something > to show the "--no-" form. Regardless, yes it would be nice to have something like this. I think there are places that can make use of this. -- Duy -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv2 2/2] commit/status: show the index-worktree diff with -v -v
Junio C Hamano venit, vidit, dixit 03.03.2015 22:26: > Michael J Gruber writes: > >> +diff --git INDEX=staged-for-commit/dir1/modified >> WORKTREE=not-staged-for-commit/dir1/modified >> +index e69de29..d00491f 100644 >> +--- INDEX=staged-for-commit/dir1/modified >> WORKTREE=not-staged-for-commit/dir1/modified > > This might be OK for a project like Git itself, but I suspect people > with long pathnames (like, eh, those in Java land) would not > appreciate it. > > Wouldn't mnemonic prefix, which the users are already familiar with, > be the most suitable tool for this disambiguation? After all that > was what it was invented for 8 years ago. Well...: > or it may want to even be like this: > > diff --git a/A b/A > ... > diff --git to-be-committed/A left-out-of-the-commit/A > ... > diff --git a/B b/B > ... > > by using a custom, unusual and easy-to-notice prefixes. Your idea was to use these verbous prefixes so that one recognizes the different types of diffs, and so that we don't need to sort them by file. I'm happy with c/,i/ and i/,w/ and without sorting. Maybe we would need headings between the two diffs then? HEAD/,INDEX/ resp. INDEX/,WORKTREE/ would be a shorter alternativ that is inline with the short acronyms execept for c/, because COMMIT/ (withiut "base") would be misleading during commit -v, I'm afraid. Michael -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv2 1/2] t7508: test git status -v
Junio C Hamano venit, vidit, dixit 03.03.2015 23:26: > Junio C Hamano writes: > >> Michael J Gruber writes: >> >>> Signed-off-by: Michael J Gruber >>> --- >>> t/t7508-status.sh | 6 ++ >>> 1 file changed, 6 insertions(+) >>> >>> diff --git a/t/t7508-status.sh b/t/t7508-status.sh >>> index 8ed5788..4989e98 100755 >>> --- a/t/t7508-status.sh >>> +++ b/t/t7508-status.sh >>> @@ -133,6 +133,12 @@ test_expect_success 'status with >>> status.displayCommentPrefix=false' ' >>> test_i18ncmp expect output >>> ' >>> >>> +test_expect_success 'status -v' ' >>> + git diff --cached >>expect && >> >> This makes the test rely on the previous one succeeding. Do we >> care, or is reproducing what ought to be in 'expect' at this step >> too expensive? > > Ahh, OK. The way the existing tests prepare 'expect' is "by hand". > > So I think what is wrong with this new test is not that relies on > the current contents of 'expect', but that it modifies it (imagine > being a merge/patch monkey who has to accept this change while a > change from somebody else that wants to add another test that relies > on the original 'expect' intact and then have to scratch his or her > head when the two topics are merged, wondering why the latter test > starts failing). > > Perhaps > > ( cat expect && git diff --cached ) >expect-with-v && > git status -v >actual && > test_cmp expect-with-v actual > > or something? That's what I had first, but the new file shows up as untracked file in the status output... I don't mind setting this one up by hand also, if you prefer. Michael -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: git describe --contains doesn't work properly for a commit
On Thu, Feb 26, 2015 at 03:23:14PM +0100, Michal Hocko wrote: > The commit in the middle was ammended to have committer date in the > past. > $ git describe --contains d63972e4e4e7eda0444e56739ad09bfbc476b9bd > tag~1 > > but > $ git describe --contains 108a0d5972fd2e5f25b2f38cfd2fee73031ff9d3 > fatal: cannot describe '108a0d5972fd2e5f25b2f38cfd2fee73031ff9d3' > > I guess this is the same issue reported previously here: > http://git.661346.n2.nabble.com/git-describe-contains-fails-on-given-tree-td5448286.html Yes, the "describe --contains" algorithm uses timestamps to cut off the traversal, so it can do the wrong thing if there's clock skew. It has a "slop" margin of one day, but skew larger than that can fool it. > Can this be fixed somehow or it would lead to other kind of issues? The options are basically: 1. Stop cutting off the traversal based on timestamps. This will make the common case of valid timestamps much slower, though, as it will have to walk all the way to the roots. 2. Use a different slop mechanism. For example, keep walking up to 5 commits past a commit suspected to be past the cutoff. This is relatively easy to do (we do it for "--since" checks), and would catch your case above. But of course it does not catch all cases of skew. 3. Introduce a more trust-worthy mechanism for ordering commits. The timestamp here is really just a proxy for the oft-discussed "generation number" of the commit within the graph. We've avoided adding generation numbers because of the storage/complexity issues. -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: [RFH] GSoC 2015 application
On Thu, Feb 26, 2015 at 08:10:38PM +0700, Duy Nguyen wrote: > On Thu, Feb 19, 2015 at 2:14 AM, Jeff King wrote: > > Where I really need help now is in the "ideas" page: > > > > http://git.github.io/SoC-2015-Ideas.html > > Is this too ambitious for a summer? I suspect the answer is yes, but anyway.. > > Due to http limitations and stateless decision, a lot of data is sent > back and forth during have/want negotiation for smart-http. I wonder > if we could implement the "long polling" scheme in a CGI program. The > program terminates HTTP requests and recreates a full duplex > connection for upload-pack to talk to the client. upload-pack falls > back to the normal mode, used by git:// and ssh://. So basically Git-over-TCP-over-HTTP? :) That would be a nice thing to have, though looking over the BOSH link (which this is my first exposure to), it does look rather complicated. It's not clear to me how easily one could plug in an existing tunneling solution, and just stick git programs at the endpoints (in other words, let that solution manage all of the connection state and just present a socketpair() to git). I'm not sure it is too ambitious in terms of actual implementation time, but I think the design work may exceed what most students are capable of. -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] grep: correct help string for --exclude-standard
On Fri, Feb 27, 2015 at 09:01:58PM +0700, Nguyễn Thái Ngọc Duy wrote: > The current help string is about --no-exclude-standard. But "git grep -h" > would show --exclude-standard instead. Flip the string. See 0a93fb8 > (grep: teach --untracked and --exclude-standard options - 2011-09-27) > for more info about these options. > > Signed-off-by: Nguyễn Thái Ngọc Duy > --- > builtin/grep.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/builtin/grep.c b/builtin/grep.c > index 4063882..e77f7cf 100644 > --- a/builtin/grep.c > +++ b/builtin/grep.c > @@ -641,7 +641,7 @@ int cmd_grep(int argc, const char **argv, const char > *prefix) > OPT_BOOL(0, "untracked", &untracked, > N_("search in both tracked and untracked files")), > OPT_SET_INT(0, "exclude-standard", &opt_exclude, > - N_("search also in ignored files"), 1), > + N_("ignore files specified via '.gitignore'"), 1), Hmm. If the default is "--exclude-standard", then what expect people to use is "--no-exclude-standard". Would it make more sense to list that in the "-h" output? Sadly I think to do that you have to manually specify "--no-exclude-standard" with OPT_NONEG, and then manually specify "--exclude-standard" in addition with OPT_HIDDEN. It might be nice if parseopt had a PARSE_OPT_NEGHELP option or something to show the "--no-" form. Something like: diff --git a/builtin/grep.c b/builtin/grep.c index 9262b73..c03c3e7 100644 --- a/builtin/grep.c +++ b/builtin/grep.c @@ -640,8 +640,10 @@ int cmd_grep(int argc, const char **argv, const char *prefix) N_("find in contents not managed by git"), 1), OPT_BOOL(0, "untracked", &untracked, N_("search in both tracked and untracked files")), - OPT_SET_INT(0, "exclude-standard", &opt_exclude, - N_("search also in ignored files"), 1), + { OPTION_SET_INT, 0, "exclude-standard", &opt_exclude, + NULL, N_("search also in ignored files"), + PARSE_OPT_NOARG | PARSE_OPT_NEGHELP, + NULL, 1 }, OPT_GROUP(""), OPT_BOOL('v', "invert-match", &opt.invert, N_("show non-matching lines")), diff --git a/parse-options.c b/parse-options.c index 80106c0..0ba7dc4 100644 --- a/parse-options.c +++ b/parse-options.c @@ -599,8 +599,12 @@ static int usage_with_options_internal(struct parse_opt_ctx_t *ctx, } if (opts->long_name && opts->short_name) pos += fprintf(outfile, ", "); - if (opts->long_name) - pos += fprintf(outfile, "--%s", opts->long_name); + if (opts->long_name) { + int neg = opts->flags & PARSE_OPT_NEGHELP; + pos += fprintf(outfile, "--%s%s", + neg ? "no-" : "", + opts->long_name); + } if (opts->type == OPTION_NUMBER) pos += utf8_fprintf(outfile, _("-NUM")); diff --git a/parse-options.h b/parse-options.h index 7940bc7..e688c32 100644 --- a/parse-options.h +++ b/parse-options.h @@ -37,6 +37,7 @@ enum parse_opt_option_flags { PARSE_OPT_LASTARG_DEFAULT = 16, PARSE_OPT_NODASH = 32, PARSE_OPT_LITERAL_ARGHELP = 64, + PARSE_OPT_NEGHELP = 128, PARSE_OPT_SHELL_EVAL = 256 }; Though it is annoying that we have to give up the nice OPT_SET_INT macro to specify an extra flag. -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] t5516-fetch-push: Correct misspelled pushInsteadOf
Anders Kaseorg venit, vidit, dixit 04.03.2015 10:43: > On Wed, 4 Mar 2015, Michael J Gruber wrote: >> If there is no "pushurl": >> "url" will be used as the "URL for fetch" and as the "URL for push". >> if we are pushing and there is "pushinsteadof" for (part of) "url" >> subsitute that within "url" >> else if there is "insteadof" for (part of) "url" >> substitute that within "url" >> >> If there is a "pushurl": >> "url" will be used as the "URL for fetch" and "pushurl" as the "URL for >> push". >> if we are pushing and there is "insteadof" for (part of) "pushurl" >> substitute that within "pushurl" >> else if we are fetching and there is "insteadof" for (part of) "url" >> substiute that within "url" > > Speaking of that, I recently had to reimplement most of this logic for the > openstack git-review tool (https://review.openstack.org/160152), which is > why I was staring too closely at t5516 in the first place. It would be > nice to have a ‘git ls-remote --get-push-url’ analogous to the existing > ‘git ls-remote --get-url’. > > Anders > And vice-versa, it would be nice if "git remote" explained where the resulting URLs come from. I vaguely remember looking into this, but I don't think it was simple. ls-remote and remote don't share code, which is a pitty. GSOC... Michael -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: git add to ignore whitespaces, some day?
On Fri, Feb 27, 2015 at 01:09:30AM +0100, Marc-André Lureau wrote: > It would be nice if git-add could be told to ignore whitespace > changes, wouldn't it? > > According to SO, I am not the one to think so: > http://stackoverflow.com/questions/3515597/git-add-only-non-whitespace-changes > > A change to add--interactive would be as simple as adding the diff -b > or -w option like: > my @diff = run_cmd_pipe("git", @diff_cmd, "-w", "--", $path); What would it mean to stage such a hunk? For example, consider this situation: git init echo 'foo();' >file git add file { echo 'if (something) {' echo 'foo();' echo '}' } >file A regular diff shows: diff --git a/file b/file index a280f9a..ce0eeda 100644 --- a/file +++ b/file @@ -1 +1,3 @@ -foo(); +if (something) { +foo(); +} but "diff -w" would show: diff --git a/file b/file index a280f9a..ce0eeda 100644 --- a/file +++ b/file @@ -1 +1,3 @@ +if (something) { foo(); +} If we try to apply that hunk to what is in the index, it will not work. The context line does not exist in the index file. Even if you could convince git-apply to massage it into place, it still does not update the whitespace in the 'foo();' line. IOW, we did not stage the full hunk at all; running "git add -p" again would show that we still have the whitespace change to stage. So if you were to pursue this, it would have to have two copies of each hunk: the one to apply, and the "display" copy that we show the user. We do this already for colorization. However, I think we rely there on the fact that the two versions of the diff match up, line for line. Whereas here, you would not even necessarily have the same number of hunks between the regular and "-b" versions. -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] xmerge.c: fix xdl_merge to conform with the manual
On Wed, Mar 4, 2015 at 4:55 AM, Anton Trunov wrote: > On 04/03/15 10:07, Eric Sunshine wrote: + echo " \t\ttwo words " >text.txt && >> >> Use of echo "\t" is not portable. Either embed literal tab characters >> or use printf "\t". > OK. > Shouldn't it be printf "\t\n" for exact substitute for echo "\t"? Yes, that was implied; it didn't seem necessary to describe the conversion to printf in full detail. Sorry if there was any confusion. -- 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] xmerge.c: fix xdl_merge to conform with the manual
On 03/03/15 23:17, Torsten Bögershausen wrote: > On 2015-03-03 18.37, Anton Trunov wrote: > [] >> Signed-off-by: Anton Trunov gmail.com> > Should we use the "real email" here (with the '@') ? Didn't realize the parser for the web version mangles emails. Will use the "real email". >> --- >> t/t3032-merge-recursive-options.sh | 43 >> ++ >> xdiff/xmerge.c | 10 - >> 2 files changed, 48 insertions(+), 5 deletions(-) >> >> diff --git a/t/t3032-merge-recursive-options.sh >> b/t/t3032-merge-recursive-options.sh >> index 4029c9c..4cbedb4 100755 >> --- a/t/t3032-merge-recursive-options.sh >> +++ b/t/t3032-merge-recursive-options.sh >> @@ -204,4 +204,47 @@ test_expect_success '--ignore-space-at-eol' ' >> test_cmp expected actual >> ' >> >> +# Setup for automerging with whitespace-only changes >> +# on both sides and in *all* lines >> + >> +test_expect_success 'setup: w/s only changes in all lines on both sides' ' >> +git rm -rf . && >> +git clean -fdqx && >> +rm -rf .git && >> +git init > missing && Nice catch! Thank you. >> + >> +echo " two words" >text.txt && >> +git add text.txt && >> +test_tick && >> +git commit -m "Initial revision" && >> + >> +git checkout -b remote && >> +echo " \t\ttwo words " >text.txt && >> +git commit -a -m "remote: insert whitespace only" && >> + >> +git checkout master && >> +echo "two words" >text.txt && >> +git commit -a -m "master: insert whitespace only" >> +' >> + >> +test_expect_success 'w/s only in all lines: --ignore-space-change preserves >> ours' ' > [] > -- 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] xmerge.c: fix xdl_merge to conform with the manual
On 04/03/15 10:07, Eric Sunshine wrote: > On Tue, Mar 3, 2015 at 3:17 PM, Torsten Bögershausen wrote: >> On 2015-03-03 18.37, Anton Trunov wrote: >> [] >>> Signed-off-by: Anton Trunov gmail.com> >> Should we use the "real email" here (with the '@') ? >>> --- >>> diff --git a/t/t3032-merge-recursive-options.sh >>> b/t/t3032-merge-recursive-options.sh >>> index 4029c9c..4cbedb4 100755 >>> --- a/t/t3032-merge-recursive-options.sh >>> +++ b/t/t3032-merge-recursive-options.sh >>> @@ -204,4 +204,47 @@ test_expect_success '--ignore-space-at-eol' ' >>> test_cmp expected actual >>> ' >>> >>> +# Setup for automerging with whitespace-only changes >>> +# on both sides and in *all* lines >>> + >>> +test_expect_success 'setup: w/s only changes in all lines on both sides' ' >>> + git rm -rf . && >>> + git clean -fdqx && >>> + rm -rf .git && >>> + git init >> missing && >>> + >>> + echo " two words" >text.txt && >>> + git add text.txt && >>> + test_tick && >>> + git commit -m "Initial revision" && >>> + >>> + git checkout -b remote && >>> + echo " \t\ttwo words " >text.txt && > > Use of echo "\t" is not portable. Either embed literal tab characters > or use printf "\t". OK. Shouldn't it be printf "\t\n" for exact substitute for echo "\t"? >>> + git commit -a -m "remote: insert whitespace only" && >>> + >>> + git checkout master && >>> + echo "two words" >text.txt && >>> + git commit -a -m "master: insert whitespace only" >>> +' >>> + >>> +test_expect_success 'w/s only in all lines: --ignore-space-change >>> preserves ours' ' -- 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] t5516-fetch-push: Correct misspelled pushInsteadOf
On Wed, 4 Mar 2015, Michael J Gruber wrote: > If there is no "pushurl": > "url" will be used as the "URL for fetch" and as the "URL for push". > if we are pushing and there is "pushinsteadof" for (part of) "url" > subsitute that within "url" > else if there is "insteadof" for (part of) "url" > substitute that within "url" > > If there is a "pushurl": > "url" will be used as the "URL for fetch" and "pushurl" as the "URL for > push". > if we are pushing and there is "insteadof" for (part of) "pushurl" > substitute that within "pushurl" > else if we are fetching and there is "insteadof" for (part of) "url" > substiute that within "url" Speaking of that, I recently had to reimplement most of this logic for the openstack git-review tool (https://review.openstack.org/160152), which is why I was staring too closely at t5516 in the first place. It would be nice to have a ‘git ls-remote --get-push-url’ analogous to the existing ‘git ls-remote --get-url’. Anders -- 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-credential-store: support XDG config dir
On Wed, Mar 04, 2015 at 04:24:58AM +0800, Paul Tan wrote: > @@ -111,8 +114,7 @@ static void remove_credential(const char *fn, struct > credential *c) > > static int lookup_credential(const char *fn, struct credential *c) > { > - parse_credential_file(fn, c, print_entry, NULL); > - return c->username && c->password; > + return parse_credential_file(fn, c, print_entry, NULL); > } I wondered if we were losing something here, as the return value from parse_credential_file is not the same as "did we find both a username and a password". But then I realized that the existing "return" line is nonsensical. The "c" variable here is our template of what to look for, not the return. I think this is leftover from an earlier iteration, where our callback filled in the values, rather than directly printing them. Nobody noticed because we didn't actually look at the return value of lookup_credential at all. So I think regardless of the end goal, it is nice to see this oddity cleaned up. > + if (!strcmp(op, "get")) { > + if (file) { > + lookup_credential(file, &c); > + } else { > + if (xdg_file && access_or_warn(xdg_file, R_OK, 0) == 0) > + ret = lookup_credential(xdg_file, &c); > + if (!ret && home_file && access_or_warn(home_file, > R_OK, 0) == 0) > + lookup_credential(home_file, &c); > + } > + } else if (!strcmp(op, "erase")) { > + if (file) { > + remove_credential(file, &c); > + } else { > + if (xdg_file && access(xdg_file, F_OK) == 0) > + remove_credential(xdg_file, &c); > + if (home_file && access(home_file, F_OK) == 0) > + remove_credential(home_file, &c); > + } The lookup rules here all look sane. Thanks for paying such attention to the details. Like Matthieu, I was unclear on the inconsistent use of access_or_warn. If we can use the same access variant everywhere, I wonder if it would be more readable to hoist it into a function like: int has_config_file(const char *file) { return file && access_or_warn(file, F_OK) == 0; } It's a tiny function, but then your repetitious "if" statements drop some of the noise: if (has_config_file(xdg_file)) ret = lookup_credential(xdg_file, &c); if (!ret && has_config_file(home_file)) lookup_credential(home_file, &c); > + } else if (!strcmp(op, "store")) { > + if (file) { > + store_credential(file, &c); > + } else if (xdg_file && access(xdg_file, F_OK) == 0) { > + store_credential(xdg_file, &c); > + if (home_file && access(home_file, F_OK) == 0 && > + c.protocol && (c.host || c.path) && c.username > + && c.password) > + remove_credential(home_file, &c); I like that you take care not to lose information during the migration, but I really don't like that we have to replicate the "is this a fully-formed credential" logic. I think I'd rather just store the credential in the xdg_file, and leave it be in home_file. The lookup will prefer the xdg version, and if we ever issue an "erase" (e.g., because the credential changes), it should remove both of them. -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] xmerge.c: fix xdl_merge to conform with the manual
On 03/03/15 23:32, Junio C Hamano wrote: > Anton Trunov writes: > >> The git-merge manual says that the ignore-space-change, >> ignore-all-space, ignore-space-at-eol options preserve our version >> if their version only introduces whitespace changes to a line. >> >> So far if there is whitespace-only changes to both sides >> in *all* lines their version will be used. > > I am having hard time understanding the last sentence, especially > the "So far" part. Do you mean "With the current code, if ours and > theirs change whitespaces on all lines, we take theirs"? By "so far" I mean "until now, but not including it", i.e. the code before applying the patch. > I find the description in the documentation is a bit hard to read. > > * If 'their' version only introduces whitespace changes to a line, > 'our' version is used; > > * If 'our' version introduces whitespace changes but 'their' > version includes a substantial change, 'their' version is used; > > * Otherwise, the merge proceeds in the usual way. > > And it is unclear if your reading is correct to me. In your "So > far" scenario, 'our' version does introduce whitespace changes and > 'their' version does quite a bit of damage to the file (after all, > they both change *all* lines, right?). It does not seem too wrong > to invoke the second clause above and take 'theirs', at least to me. Let me elaborate on this a bit. It doesn't matter if all lines are changed or not. The point is if all the changes in all the *changed* lines are trivial (non-whitespace), i.e. there is no one line with substantial change on both sides, then we just through away their version and keep our whitespace changes. We are talking here about non-so-probable corner-case of trivial changes in our and their versions, perhaps an uncoordinated tabs-vs-space clean-up. So I think I should add "changed lines" to the commit message. For the code version before applying this patch the following scenario will take place if "git merge -Xignore-all-space remote" gets executed. base file: 1st line 2nd line master file: 1st line 2nd line with substantial change remote file: 1st line 2nd line merge result file: 1st line 2nd line with substantial change So essentially it does what "git merge -s ours remote" does in case if all their changes are trivial. This seems like reasonable solution to me: we _are_ trying to ignore whitespace changes and we are explicit about it. But, in the scenario with trivial changes everywhere we get a completely different result: base file: 1st line 2nd line master file: 1st line 2nd line remote file: 1st line 2nd line merge result file: 1st line 2nd line In my opinion if we respect the principle of least astonishment this behavior should be fixed to: merge result file: 1st line 2nd line Exactly so does this patch. > It is an entirely different matter if the behaviour the document > describes is sane, and I didn't ask "git log" what the reasoning > behind that second point is, but my guess is that a change made by > them being "substantial" is a sign that it is a whitespace cleanup > change and we should take the cleanup in such a case, perhaps? If we want to take in their clean-up why would we use the -Xignore-space-change option in the first place? It looks like we're explicitly saying "we don't want any changes that are whitespace-only", right? And if we introduced some cleanup too what should we do when the cleanups conflict? (exactly our case) As far as I am concerned one should either manually resolve that kind of conflicts without using the -Xignore-... options or just git merge -X theirs remote. -- 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] t5516-fetch-push: Correct misspelled pushInsteadOf
Junio C Hamano venit, vidit, dixit 03.03.2015 21:06: > Michael J Gruber writes: > >> Junio C Hamano venit, vidit, dixit 02.03.2015 20:43: >>> Anders Kaseorg writes: >>> Signed-off-by: Anders Kaseorg --- t/t5516-fetch-push.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh index 85c7fec..594d7a6 100755 --- a/t/t5516-fetch-push.sh +++ b/t/t5516-fetch-push.sh @@ -238,7 +238,7 @@ test_expect_success 'push with pushInsteadOf' ' test_expect_success 'push with pushInsteadOf and explicit pushurl (pushInsteadOf should not rewrite)' ' mk_empty testrepo && test_config "url.trash2/.pushInsteadOf" testrepo/ && - test_config "url.trash3/.pusnInsteadOf" trash/wrong && + test_config "url.trash3/.pushInsteadOf" trash/wrong && test_config remote.r.url trash/wrong && test_config remote.r.pushurl "testrepo/" && git push r refs/heads/master:refs/remotes/origin/master && >>> >>> Interesting. >>> >>> Now an obvious and natural question after seeing this change is how >>> the original test passed with misspelled configuration. Is a test >>> that pushes into "trash/wrong" checking the right outcome? If the >>> reason why the existing tests passed without this patch is because >>> they do not test the right thing, then shouldn't they be corrected >>> together with the above fix? >>> >> >> Ha, I was look there, too, just today and was wondering the same. >> >> I guess the test wanted to make sure (among other things) that >> url.trash3/.pushInsteadOf does not affect the push to remote r (which >> has an explicit pushurl)... > > OK, so it makes sure the push goes to testrepo/ but it does not make > sure trash2 or trash3 are not touched. Makes sort of sense. > > Thanks. > > I am not sure how the intention of the commit 1c2eafb8 (Add > url..pushInsteadOf: URL rewriting for push only, 2009-09-07), > which introduced the behaviour verified by this test, interacts with > the desire to redefine what URL and pushURL mean in our recent past, > what e.g. e6196ae1 (remote: add --fetch and --both options to set-url, > 2014-11-25) wanted to do, though. Thoughts? > Wow. That looks strange to me on first read. Sorry I didn't catch it back then. "--fetch" sets both url and pushurl?? Also, talking about these things (in the commit msg etc.) can be confusing very quickly because there are the config names url and pushurl, the struct members url und pushurl, and then suddenly the notion of "fetch URL" appears. So, for definiteness sake, I'll use "url" and "pushurl" for the config names and struct members (which the config values end up in), and I'll use "URL for fetch" and "URL for push" for the URLs that git will use for fetch resp. push. If there is no "pushurl": "url" will be used as the "URL for fetch" and as the "URL for push". if we are pushing and there is "pushinsteadof" for (part of) "url" subsitute that within "url" else if there is "insteadof" for (part of) "url" substitute that within "url" If there is a "pushurl": "url" will be used as the "URL for fetch" and "pushurl" as the "URL for push". if we are pushing and there is "insteadof" for (part of) "pushurl" substitute that within "pushurl" else if we are fetching and there is "insteadof" for (part of) "url" substiute that within "url" I don't know what "remote set-url" does, I think the whole remote command as it is does not fit in well with our UI, but that applies to other commands with non-option subcommands as well (or bad). I would have hoped it is set-url and set-pushurl to set those two config options. Maybe that's not useful. The logic above may look a bit strange regarding pushinsteadof, but it's really what makes this useful. An example: [url "git://github.com/"] insteadOf = github: [url "github:"] pushinsteadOf = github: This allows me to use git remote add github:/ and have fetches over git protocol, pushes over ssh since my ssh_config has: host github Hostname github.com User git IdentityFile ~/.ssh/repoor_dsa [url "https://bitbucket.org/";] insteadOf = bitbucket: [url "https://gru...@bitbucket.org/";] pushinsteadOf = bitbucket: This allows me to use git remote add bitbucket:/ and have fetches over https, pushes over https with my username preset. I rarely use pushurl, but could use it to override url, or to do the above magic on a case by case level, such as if I need to specify different usernames for the same host. So, that's my understanding of how these things are supposed to work and why it is useful. Michael -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Unexpected/unexplained difference between git pull --rebase and git rebase
On Tue, Mar 03, 2015 at 03:54:05PM -0800, Mike Botsko wrote: > Thanks, that clarifies a lot. > > I only have two follow-up questions: > > In your branch example, how does git determine that C/D have been > rewritten and need to be "replaced" with their current versions > existing upstream? In this scenario I've encountered, the commit hash > and the patch ID of those commits changed because the contents of the > patches had to be modified slightly due to merge conflicts which > occurred when the upstream branch was rebased. It uses the reflog. There's some discussion in git-merge-base(1) [0], although IIRC it steers away from being too explicit in case anyone comes up with a way to make it more intelligent in the future. Note that this means it will only work if you are performing the rebase in the same repository as you originally created the branch, since in other repositories the original branch point may not appear in the reflog of the upstream. [0] http://git-scm.com/docs/git-merge-base#_discussion_on_fork_point_mode > Also, you mentioned "not building off of upstream branches which might > be rewritten". We generally try to avoid this but I don't see any > alternative with the way we do things. > > upstream/master <- An always-clean copy of what's fully approved and live > upstreamfeature-A > upstreamfeature-B, etc <- Feature branches designed to organized > long-term new feature work > > Individual developers will then create local development branches > based on those feature branches. If three people are responsible for > tasks for feature-A, they'll create development branches for each > task, do their work, and(via github enterprise) submit a pull request > so we can properly review their work, test it, etc. > > The problem I have today stems from situations where a feature branch > has been merged with master. If feature-B is merged with master, and > someone rebases feature-A, there may be merge conflicts. If they fix > the conflicts, that may alter the commit history of the feature > branch, which then impacts all branches developers have based on it. > > Part of me feels like we should be able to never rebase feature > branches, they should exist outside of new work merged to master. > However, it's much easier to resolve merge conflicts in small doses, > and we're in a much better position to know that we're fully updated > and can catch other problems early. > > Is there a better way to do this, so that we never risk rewriting the > "middle tier"? Having two tiers of feature branches seems a bit weird to me (although I do know of another group that does something similar). Perhaps a throwaway integration branch would help, which will allow you to let git-rerere(1) remember resolutions to merge conflicts. The idea is that you periodically (say once a day or every few days) do something like this: $ git checkout integration $ git reset --keep master $ git merge feature-A $ git merge feature-B ... Then you can test the features together and get an early view of any conflicts that might hit when you merge them to master. I wrote a tool to help manage integration branches [1]. [1] http://johnkeeping.github.io/git-integration/ There's quite a lot of discussion on branch management in git-workflows(7). > On Tue, Mar 3, 2015 at 3:40 PM, John Keeping wrote: > > On Tue, Mar 03, 2015 at 03:20:48PM -0800, Mike Botsko wrote: > >> Maybe I'm lacking the distinction regarding what I'm being specific about. > >> > >> In both examples, I'm asking it specifically to rebase in changes from > >> the remote "upstream" and a named branch at that location. I'm giving > >> git the same information, it's just interpreting it differently - and > >> I'm not understanding why. > > > > Not quite. If you say: > > > > git rebase $sha1 > > > > then you're telling git-rebase to apply the commits $sha1..HEAD onto > > $sha1. > > > > If you say: > > > > git rebase > > > > then it will be re-written as: > > > > git rebase --fork-point @{upstream} > > > > in which case Git will apply more complicated logic so that you can > > recover from the case where @{upstream} has been re-written. > > > > Consider the following scenario: > > > > F branch > > / > >C -- D master@{1} > > / > > A -- B -- C' -- D' -- E master > > > > where C' and D' are rewritten versions of C and D. > > > > In this case, imagine you are at F on "branch", "git rebase master" will > > replace C, D and F onto E because you have explicitly selected to replay > > master..branch onto master. > > > > "git rebase" will apply the fork-point logic and realise that D is a > > previous version of master, so it will only replay F onto E. > > > > In general if you just want to rebase onto your upstream it is simpler > > to just call "git rebase" which will do the ri
Re: [PATCH v2] diff --shortstat --dirstat: remove duplicate output
On Mon, Mar 02, 2015 at 02:00:09AM +0100, SZEDER Gábor wrote: > It's not just 'grep -c' but the 'test' checking its output as well. > > If something goes wrong and the line count doesn't match expectations > 'test' fails silently leaving the developer clueless as to what went > wrong. > > 'test_line_count', on the other hand, produces useful output in case > of a failure: > >$ printf 'foo\nbar\n' >actual >$ test_line_count = 1 actual >test_line_count: line count for actual != 1 >foo >bar Since we have test_line_count, I think it makes sense to use it. But for reference, I recently introduced the `verbose` function to test-lib.sh, which lets you write: $ verbose test 1 = 2 command failed: 'test' '1' '=' '2' You can use it with any command that might fail without printing a useful error message. The big downside is that it sees only the arguments to the command, so if you write: $ test "$(do_something)" = 123 you will only see: command failed: 'test '456' '=' '123' with no notion that $(do_something) was involved. So purpose-built helpers like test_line_count will produce better output. You may also be introduced in the "-x" option I recently introduced, which can help with "quiet" failures. E.g.: $ ./t0001-init.sh -x --verbose-only=15 [...] ok 13 - GIT_DIR & GIT_WORK_TREE (2) ok 14 - reinit expecting success: mkdir template-source && echo content >template-source/file && git init --template=../template-source template-custom && test_cmp template-source/file template-custom/.git/file + mkdir template-source + echo content + git init --template=../template-source template-custom Initialized empty Git repository in /home/peff/compile/git/t/trash directory.t0001-init/template-custom/.git/ + test_cmp template-source/file template-custom/.git/file + diff -u template-source/file template-custom/.git/file ok 15 - init with --template ok 16 - init with --template (blank) (ok, it's not that interesting because the test didn't fail, but hopefully you get the point). Now I'll stop hijacking your thread to advertise random test-lib features. :) -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] git: make "git -C '' " not to barf
On March 3, 2015 11:30:13 PM GMT+05:30, Junio C Hamano wrote: >On Tue, Mar 3, 2015 at 8:22 AM, Matthieu Moy > wrote: >> Karthik Nayak writes: >> >>> now it works like cd "" which silently succeeds >> >> Missing "." at end of sentence. > >And lack of capitalization at the beginning. Yes, noted, Will fix with next patch. > >More importantly, lack of justification--why is it a good idea to make >"it works like cd ''" in the first place? > >> >>> git.c | 4 +++- >> >> That would probably also deserve a test. Will include a patch with next patch version. > >Another "huh?" I had on the actual code was if this counts as >"envchanged", if >you are not actually changing the directory at all. > >Is this meant as a "micro" for GSoC, by the way? Yes, it's meant for GSoC. I did overlook that. Will fix in next patch version. Thanks for your 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: Git Newbie - GSoC mini Project
Dennis Kaarsemaker writes: > For the first step, I'd send a mail to this list asking people for > input, I'd even put that as step 2. Step 1 would be: watch the list and see if anyone is not already working on it. Hint, hint ... -- Matthieu Moy http://www-verimag.imag.fr/~moy/ -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html