Re: [PATCH 5/5] implement @{publish} shorthand
Jeff King p...@peff.net writes:
In a triangular workflow, you may have a distinct
@{upstream} that you pull changes from, but publish by
default (if you typed git push) to a different remote (or
a different branch on the remote). It may sometimes be
useful to be able to quickly refer to that publishing point
(e.g., to see which changes you have that have not yet been
published).
This patch introduces the branch@{publish} shorthand (or
@{pu} to be even shorter). It refers to the tracking
If @{u} can already be used for upstream, why not allow @{p} but
require two letters @{pu}? Just being curious---I am not advocating
strongly for a shorter short-hand.
Or is @{p} already taken by something and my memory is not
functioning well?
branch of the remote branch to which you would push if you
were to push the named branch. That's a mouthful to explain,
so here's an example:
$ git checkout -b foo origin/master
$ git config remote.pushdefault github
$ git push
Signed-off-by: Jeff King p...@peff.net
---
The implementation feels weird, like the where do we push to code
should be factored out from somewhere else. I think what we're doing
here is not _wrong_, but I don't like repeating what git push is doing
elsewhere. And I just punt on simple as a result. :)
sha1_name.c | 76
-
1 file changed, 75 insertions(+), 1 deletion(-)
diff --git a/sha1_name.c b/sha1_name.c
index 50df5d4..59ffa93 100644
--- a/sha1_name.c
+++ b/sha1_name.c
@@ -435,6 +435,12 @@ static inline int upstream_mark(const char *string, int
len)
return at_mark(string, len, suffix, ARRAY_SIZE(suffix));
}
+static inline int publish_mark(const char *string, int len)
+{
+ const char *suffix[] = { @{publish} };
+ return at_mark(string, len, suffix, ARRAY_SIZE(suffix));
+}
+
static int get_sha1_1(const char *name, int len, unsigned char *sha1,
unsigned lookup_flags);
static int interpret_nth_prior_checkout(const char *name, struct strbuf
*buf);
@@ -481,7 +487,8 @@ static int get_sha1_basic(const char *str, int len,
unsigned char *sha1)
nth_prior = 1;
continue;
}
- if (!upstream_mark(str + at, len - at)) {
+ if (!upstream_mark(str + at, len - at)
+ !publish_mark(str + at, len - at)) {
reflog_len = (len-1) - (at+2);
len = at;
}
@@ -1100,6 +1107,69 @@ static int interpret_upstream_mark(const char *name,
int namelen,
return len + at;
}
+static const char *get_publish_branch(const char *name_buf, int len)
+{
+ char *name = xstrndup(name_buf, len);
+ struct branch *b = branch_get(*name ? name : NULL);
+ struct remote *remote = b-pushremote;
+ const char *dst;
+ const char *track;
+
+ free(name);
+
+ if (!remote)
+ die(_(branch '%s' has no remote for pushing), b-name);
+
+ /* Figure out what we would call it on the remote side... */
+ if (remote-push_refspec_nr)
+ dst = apply_refspecs(remote-push, remote-push_refspec_nr,
+ b-refname);
+ else
+ dst = b-refname;
+ if (!dst)
+ die(_(unable to figure out how '%s' would be pushed),
+ b-name);
+
+ /* ...and then figure out what we would call that remote here */
+ track = apply_refspecs(remote-fetch, remote-fetch_refspec_nr, dst);
+ if (!track)
+ die(_(%s@{publish} has no tracking branch for '%s'),
+ b-name, dst);
+
+ return track;
+}
+
+static int interpret_publish_mark(const char *name, int namelen,
+ int at, struct strbuf *buf)
+{
+ int len;
+
+ len = publish_mark(name + at, namelen - at);
+ if (!len)
+ return -1;
+
+ switch (push_default) {
+ case PUSH_DEFAULT_NOTHING:
+ die(_(cannot use @{publish} with push.default of 'nothing'));
+
+ case PUSH_DEFAULT_UNSPECIFIED:
+ case PUSH_DEFAULT_MATCHING:
+ case PUSH_DEFAULT_CURRENT:
+ set_shortened_ref(buf, get_publish_branch(name, at));
+ break;
+
+ case PUSH_DEFAULT_UPSTREAM:
+ set_shortened_ref(buf, get_upstream_branch(name, at));
+ break;
+
+ case PUSH_DEFAULT_SIMPLE:
+ /* ??? */
+ die(@{publish} with simple unimplemented);
+ }
+
+ return at + len;
+}
+
/*
* This reads short-hand syntax that not only evaluates to a commit
* object name, but also can act as if the end user spelled the name
@@ -1150,6 +1220,10 @@ int interpret_branch_name(const char *name, int
namelen, struct strbuf
Re: [PATCH v3 3/5] refs: teach for_each_ref a flag to avoid recursion
Jeff King p...@peff.net writes:
On Tue, Jan 07, 2014 at 06:58:50PM -0500, Jeff King wrote:
+if (flags DO_FOR_EACH_NO_RECURSE) {
+struct ref_dir *subdir = get_ref_dir(entry);
+sort_ref_dir(subdir);
+retval = do_for_each_entry_in_dir(subdir, 0,
Obviously this is totally wrong and inverts the point of the flag. And
causes something like half of the test suite to fail.
Michael was nice enough to point it out to me off-list, but well, I have
to face the brown paper bag at some point. :) In my defense, it was a
last minute refactor before going to dinner. That is what I get for
rushing out the series.
And perhaps a bad naming that calls for double-negation in the
normal cases, which might have been less likely to happen it the new
flag were called onelevel only or something, perhaps?
Here's a fixed version of patch 3/5.
-- 8 --
Subject: refs: teach for_each_ref a flag to avoid recursion
The normal for_each_ref traversal descends into
subdirectories, returning each ref it finds. However, in
some cases we may want to just iterate over the top-level of
a certain part of the tree.
The introduction of the flags option is a little
mysterious. We already have a flags option that gets stuck
in a callback struct and ends up interpreted in do_one_ref.
But the traversal itself does not currently have any flags,
and it needs to know about this new flag.
We _could_ introduce this as a completely separate flag
parameter. But instead, we simply put both flag types into a
single namespace, and make it available at both sites. This
is simple, and given that we do not have a proliferation of
flags (we have had exactly one until now), it is probably
sufficient.
Signed-off-by: Jeff King p...@peff.net
---
refs.c | 61 ++---
1 file changed, 38 insertions(+), 23 deletions(-)
diff --git a/refs.c b/refs.c
index 3926136..b70b018 100644
--- a/refs.c
+++ b/refs.c
@@ -589,6 +589,8 @@ static void sort_ref_dir(struct ref_dir *dir)
/* Include broken references in a do_for_each_ref*() iteration: */
#define DO_FOR_EACH_INCLUDE_BROKEN 0x01
+/* Do not recurse into subdirs, just iterate at a single level. */
+#define DO_FOR_EACH_NO_RECURSE 0x02
/*
* Return true iff the reference described by entry can be resolved to
@@ -661,7 +663,8 @@ static int do_one_ref(struct ref_entry *entry, void
*cb_data)
* called for all references, including broken ones.
*/
static int do_for_each_entry_in_dir(struct ref_dir *dir, int offset,
- each_ref_entry_fn fn, void *cb_data)
+ each_ref_entry_fn fn, void *cb_data,
+ int flags)
{
int i;
assert(dir-sorted == dir-nr);
@@ -669,9 +672,13 @@ static int do_for_each_entry_in_dir(struct ref_dir *dir,
int offset,
struct ref_entry *entry = dir-entries[i];
int retval;
if (entry-flag REF_DIR) {
- struct ref_dir *subdir = get_ref_dir(entry);
- sort_ref_dir(subdir);
- retval = do_for_each_entry_in_dir(subdir, 0, fn,
cb_data);
+ if (!(flags DO_FOR_EACH_NO_RECURSE)) {
+ struct ref_dir *subdir = get_ref_dir(entry);
+ sort_ref_dir(subdir);
+ retval = do_for_each_entry_in_dir(subdir, 0,
+ fn, cb_data,
+ flags);
+ }
} else {
retval = fn(entry, cb_data);
}
@@ -691,7 +698,8 @@ static int do_for_each_entry_in_dir(struct ref_dir *dir,
int offset,
*/
static int do_for_each_entry_in_dirs(struct ref_dir *dir1,
struct ref_dir *dir2,
- each_ref_entry_fn fn, void *cb_data)
+ each_ref_entry_fn fn, void *cb_data,
+ int flags)
{
int retval;
int i1 = 0, i2 = 0;
@@ -702,10 +710,12 @@ static int do_for_each_entry_in_dirs(struct ref_dir
*dir1,
struct ref_entry *e1, *e2;
int cmp;
if (i1 == dir1-nr) {
- return do_for_each_entry_in_dir(dir2, i2, fn, cb_data);
+ return do_for_each_entry_in_dir(dir2, i2, fn, cb_data,
+ flags);
}
if (i2 == dir2-nr) {
- return do_for_each_entry_in_dir(dir1, i1, fn, cb_data);
+ return do_for_each_entry_in_dir(dir1, i1, fn, cb_data,
+
Re: [PATCH v2 4/5] get_sha1: speed up ambiguous 40-hex test
Michael Haggerty mhag...@alum.mit.edu writes: It's not only racy WRT other processes. If the current git process would create a new reference, it wouldn't be reflected in the cache. It's true that the main ref_cache doesn't invalidate itself automatically either when a new reference is created, so it's not really a fair complaint. However, as we add places where the cache is invalidated, it is easy to overlook this cache that is stuck in static variables within a function definition and it is impossible to invalidate it. Might it not be better to attach the cache to the ref_cache structure instead, and couple its lifetime to that object? Alternatively, the cache could be created and managed on the caller side, since the caller would know when the cache would have to be invalidated. Also, different callers are likely to have different performance characteristics. It is unlikely that the time to initialize the cache will be amortized in most cases; in fact, rev-list --stdin might be the *only* plausible use case. True. Regarding the overall strategy: you gather all refnames that could be confused with an SHA-1 into a sha1_array, then later look up SHA-1s in the array to see if they are ambiguous. This is a very special-case optimization for SHA-1s. I wonder whether another approach would gain almost the same amount of performance but be more general. We could change dwim_ref() (or a version of it?) to read its data out of a ref_cache instead of going to disk every time. Then, at the cost of populating the relevant parts of the ref_cache once, we would have fast dwim_ref() calls for all strings. If opendir-readdir to grab only the names (but not values) of many refs is a lot faster than stat-open-read a handful of dwim-ref locations for a given name, that optimization might be worthwhile, but I think that requires an update to read_loose_refs() not to read_ref_full() and the users of refs API to instead lazily resolve the refs, no? If I ask for five names (say 'maint', 'master', 'next', 'pu', 'jch'), the current code will do 5 dwim_ref()s, each of which will consult 6 locations with resolve_ref_unsafe(), totalling 30 calls to resolve_ref_unsafe(), each of which in turn is essentially an open followed by either an return on ENOENT or a read. So 30 opens and 5 reads in total. With your lazy ref_cache scheme, instead we would enumerate all the loose ones in the same 6 directories (e.g. refs/tags/, refs/heads), so 6 opendir()s with as many readdir()s as I have loose refs, plus we open-read them in read_loose_refs() called from get_ref_dir() with the current ref_cache code. For me, find .git/refs/heads gives 500+ lines of output, which suggests that using the ref_cache mechanism for dwim_ref() may not be a huge win, unless it is updated to be extremely lazy, and readdir()s turns out to be extremely less heavier than open-read. Also it is unlikely that the cost to initialize the cache is amortized to be a net win unless we are dealing with tons of dwim_ref()s. -- To unsubscribe from this list: send the line 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] stash: handle specifying stashes with spaces
Øystein Walle oys...@gmail.com writes:
But it's seems the spaces trigger some other way of interpreting the
selector. In my git.git, git rev-parse HEAD{0} gives me the same result
as HEAD@{ 0 } but HEAD@{1} and HEAD@{ 1 } are different.
The integer to specify the nth reflog entry (or nth prior checkout)
are never spelled with any SP stuffing. HEAD@{1} is the prior state,
HEAD@{-1} is the previous branch; HEAD@{ 1 } nor HEAD@{ -1 } do not
mean these things.
Any string inside @{...} that is _not_ a valid nth reflog entry
specifier is interpreted as a human-readable timestamp via the
approxidate logic (and used only when it makes sense). 1 happens
to mean the first day of the month.
--
To unsubscribe from this list: send the line 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] fetch: Print full url in header
Tom Miller jacker...@gmail.com writes: After reading this and trying different things with the code. I believe it would make sense to continue to anonymize the url for output to the terminal. Yes. That is what the anonymize bit is all about. I also chose to continue to strip the trailing characters for the FETCH_HEAD file. I wanted the input of the mailing list to see if we should also stop striping the trailing characters off of the url written to FETCH_HEAD? If so, I'll do it as a seperate patch. These strings are used to come up with the log subject line for merges, and there is a value in keeping them as short as possible by removing unnecessary bits. I wouldn't mind, and actually I suspect that it is more preferrable, to make the consistency go the other way, that is ... Do not remove / and .git from the end of the header url when fetching. This affects the output of fetch and fetch --prune making the header url more consistent with remote --verbose. ... to make remote --verbose abbreviate to match what you see from fetch. Having said all that, the difference between the full URL shown by remote --verbose (which is used to interact with the remote in this repository) and the abbreviated URL (which is shown by fetch and is designed to be sharable with others with a simple cutpaste) matters only when there are a pair of ambiguously configured repositories (e.g. there are two repositories git://host/a.git/ and git://host/a/.git) that serve different things and you are debugging the situation. And to me, remote --verbose looks more or less a debugging aid, nothing more. So another alternative that may be to leave everything as-is. 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: Verifiable git archives?
Andy Lutomirski l...@amacapital.net writes: It's possible, in principle, to shove enough metadata into the output of 'git archive' to allow anyone to verify (without cloning the repo) to verify that the archive is a correct copy of a given commit. Would this be considered a useful feature? Presumably there would be a 'git untar' command that would report failure if it fails to verify the archive contents. This could be as simple as including copies of the commit object and all relevant tree objects and checking all of the hashes when untarring. You only need the object name of the top-level tree. After untar the archive into an empty directory, make it a new repository and git add . git write-tree---the result should match the top-level tree the archive was supposed to contain. Of course, you can write git verify-archive that does the same computation all in-core, without actually extracting the archive into an empty directory. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 5/5] implement @{publish} shorthand
Jeff King p...@peff.net writes:
Or is @{p} already taken by something and my memory is not
functioning well?
It is my brain that was not functioning well. I somehow thought well,
@{u} is already taken, so we must use @{pu}. Which of course makes no
sense, unless you are middle-endian. :)
We may want to be cautious about giving up a short-and-sweet
single-letter, though, until the feature has proved itself. We could
also teach upstream_mark and friends to match unambiguous prefixes (so
@{u}, @{up}, @{upst}, etc). That means @{p} would work
immediately, but scripts should use @{publish} for future-proofing.
I recall we wanted to start only with @{upstream} without @{u};
justification being if the concept is solid and useful enough, the
latter will come later as a natural user-desire, during the
discussion that ended up introducing them.
I am OK with the unambigous prefix string.
Thanks for sanity-checking.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 5/5] implement @{publish} shorthand
Philip Oakley philipoak...@iee.org writes:
From: Jeff King p...@peff.net
Sent: Wednesday, January 08, 2014 9:37 AM
In a triangular workflow, you may have a distinct
@{upstream} that you pull changes from, but publish by
default (if you typed git push) to a different remote (or
a different branch on the remote).
One of the broader issues is the lack of _documenation_ about what the
normal' naming convention is for the uspstream remote. Especially the
implicit convention used within our documentation (and workflow).
Sure, let's start trying to come up with what the eventual
documentation patch may want to say.
* The upstream is the place the updates by the project-as-a-whole
(including others' work but also your previous work) come from.
It is what you use git pull [--rebase] to integrate the work on
your current branch with in order to keep it in sync with the
outside world. Such a repository (often called origin, and
git clone sets it up for you) may be called upstream
repository.
Each of your branch would often have a single branch in that
repository (e.g. master, which you locally use the
origin/master remote-tracking branch to keep track of its most
recently observed state). In the simplest case, you clone from
your origin, you get your own master branch, which is set to
integrate with the master branch at the origin. Their
master (i.e. what you view as origin/master) would be the
upstream branch for your master branch.
For a branch B, B@{upstream} names the remote-tracking branch
used for the upstream branch of B. For example, to fork a new
branch 'foo' that has the same upstream branch as your branch
'master' does, git checkout -t -b foo master@{upstream} can be
used.
* If you and others are using the same repository to advance the
project, the repository you cloned from, i.e. your upstream
repository, is the same repository you push your changes back
to. There is no other repository you have to worry about.
In such a centralized setting, it is likely that you may want
to update one of three possible branches at the upstream
repository when you push your changes back, if your local branch
is named differently from its upstream branch. Either:
(1) You started working on a topic (e.g. your fix-bug-2431
branch) based on an integration branch (e.g. master at the
upstream, i.e. origin/master to you), and you want to
publish it so that others can take a look at it and help you
polish it while it is still not suitable for the integration
branch. As long as you gave a name to that topic branch that
is descriptive and good enough for public consumption, you
would want it to go to the same name (e.g. you would want to
push to fix-bug-2431 branch at the upstream repository from
your fix-bug-2431 branch); or
(2) You are working on your copy (e.g. your master branch) of
an integration branch (e.g. origin/master to you), and you
want to update the master branch at the upstream
repository.
(3) There is another possibilty, in which you are working on a
topic forked from an integration branch (as in (1)), and are
done with the topic and want to push the result out directly
to the integration branch. Your fix-bug-2431 branch may
have started from origin/master and git pull [--rebase]
on the branch would integrate with master branch at the
upstream repository, and your git push on the
fix-bug-2431 branch will update that master branch at the
upstream repository, which makes it look symmetric.
The default in Git 2.0 will allow you to do (2) without any
further set-up, and you can start living in the future by
setting push.default to simple. Your current branch, when you
run git push, and its upstream branch must share the same
name.
If you want to do (1), you would want to set push.default to
current. Your current branch, when you run git push may not
have an explicit upstream branch (hence git pull without any
other argument may fail), but the work on your branch will be
pushed to the branch of the same name at the upstream
repository.
For (3), you would set push.default to upstream. Your current
branch, when you run git push, must have an explicit upstream
branch specified and you must be pushing to the upstream
repository for this to work for obvious reasons.
* If you originally clone from somewhere you cannot (or do not want
to even if you could) push to, you would want your git push to
go to a repository that is different from your upstream. In
such a triangular setting, the result of your work is published
to your own repository (we'd call it publish), and others
interested in your work would pull from there to integrate it to
their work. Among these other people there may be
Re: [PATCH mm/mv-file-to-no-such-dir-with-slash] mv: let 'git mv file no-such-dir/' error out on Windows, too
Johannes Sixt j...@kdbg.org writes: The previous commit c57f628 (mv: let 'git mv file no-such-dir/' error out) relies on that rename(src, dst/) fails if directory dst does not exist (note the trailing slash). This does not work as expected on Windows: This rename() call is successful. Insert an explicit check for this case. Could you care to explain Successful how a bit here? Do we see no-such-dir mkdir'ed and then no-such-dir/file created? Do we see file moved to a new file whose name is no-such-dir/? I am guessing that it would be the latter, but if that is the case we would need at least an air-quote around successful. This changes the error message from $ git mv file no-such-dir/ fatal: renaming 'file' failed: Not a directory to $ git mv file no-such-dir/ fatal: destination directory does not exist, source=file, destination=no-such-dir/ Signed-off-by: Johannes Sixt j...@kdbg.org --- builtin/mv.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/builtin/mv.c b/builtin/mv.c index 08fbc03..21c46d1 100644 --- a/builtin/mv.c +++ b/builtin/mv.c @@ -214,6 +214,8 @@ int cmd_mv(int argc, const char **argv, const char *prefix) } } else if (string_list_has_string(src_for_dst, dst)) bad = _(multiple sources for the same target); + else if (is_dir_sep(dst[strlen(dst) - 1])) + bad = _(destination directory does not exist); else string_list_insert(src_for_dst, dst); -- To unsubscribe from this list: send the line 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: Verifiable git archives?
Andy Lutomirski l...@amacapital.net writes: You only need the object name of the top-level tree. After untar the archive into an empty directory, make it a new repository and git add . git write-tree---the result should match the top-level tree the archive was supposed to contain. Hmm. I didn't realize that there was enough metadata in the 'git archive' output to reproduce the final tree. We do record the commit object name in the extended header when writing a tar archive already, but you have to grab the commit object from somewhere in order to read the top-level tree object name, which we do not record. Also, if you used keyword substitution and such when creating an archive, then the filesystem entities resulting from expanding it would not match the original. -- To unsubscribe from this list: send the line 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/3] gen_scanf_fmt(): delete function and use snprintf() instead
Michael Haggerty mhag...@alum.mit.edu writes: To replace %.*s with %s, all we have to do is use snprintf() to interpolate %s into the pattern. Cute ;-). 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 0/6] Make 'git help everyday' work
I think we already use a nicer way to set up a page alias to keep old links working than making a copy in Documentation/; please mimic that if possible. It may be overdue to refresh the suggested set of top 20 commands, as things have vastly changed over the past 8 years. Perhaps we should do that after reorganizing with something like this series. -- To unsubscribe from this list: send the line 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 3/3] shorten_unambiguous_ref(): tighten up pointer arithmetic
Michael Haggerty mhag...@alum.mit.edu writes: As for removing the third argument of refname_match(): although all callers pass it ref_ref_parse_rules, that array is sometimes passed to the function via the alias ref_fetch_rules. So I suppose somebody wanted to leave the way open to make these two rule sets diverge (though I don't know how likely that is to occur). It is the other way around. We used to use two separate rules for the normal ref resolution dwimming and dwimming done to decide which remote ref to grab. For example, 'x' in 'git log x' can mean 'refs/remotes/x/HEAD', but 'git fetch origin x' would not grab 'refs/remotes/x/HEAD' at 'origin'. Also, 'git fetch origin v1.0' did not look into 'refs/tags/v1.0' in the 'origin' repository back when these two rules were different. This was originally done very much on purpose, in order to avoid surprises and to discourage meaningless usage---you would not expect us to be interested in the state of a third repository that was observed by our 'origin' the last time (which we do not even know when). When we harmonized these two rules later and we #defined out ref_fetch_rules away to avoid potential breakages for in-flight topics. The old synonym can now go safely. -- To unsubscribe from this list: send the line 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 0/6] Make 'git help everyday' work
Philip Oakley philipoak...@iee.org writes:
From: Junio C Hamano gits...@pobox.com
I think we already use a nicer way to set up a page alias to keep
old links working than making a copy in Documentation/; please mimic
that if possible.
This was mainly about ensuring that the 'git help' command could
access these extra extra guides that it currently misses. (Tt also
misses the 'user-manual', which isn't a man page, but could have a
link page to guide the seeker of truth between 'git help' and the
actual user-manual)
The only method I can see for that (via help.c) is to get the filename
format correct. Where you thinking of something else?
I do not have an objection against the creation of giteveryday.txt;
I was questioning the way the original everyday.txt was left behind
to bit-rot. It is good to keep _something_ there, because there may
be old URLs floating around that point at Documentation/everyday.txt,
but the contents of that file does not have to be a stale copy.
Cf. bd4a3d61 (Rename {git- = git}remote-helpers.txt, 2013-01-31)
for how we renamed git-remote-helpers.txt to gitremote-helpers.txt
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
What's cooking in git.git (Jan 2014, #02; Fri, 10)
What's cooking in git.git (Jan 2014, #02; Fri, 10) -- Here are the topics that have been cooking. Commits prefixed with '-' are only in 'pu' (proposed updates) while commits prefixed with '+' are in 'next'. Quite a few topics have graduated to 'master' in this round and the upcoming release is starting to take shape. You can find the changes described here in the integration branches of the repositories listed at http://git-blame.blogspot.com/p/git-public-repositories.html -- [Graduated to master] * ap/path-max (2013-12-16) 1 commit (merged to 'next' on 2014-01-03 at affc620) + Prevent buffer overflows when path is too long * bc/log-decoration (2013-12-20) 1 commit (merged to 'next' on 2014-01-03 at ff8873f) + log: properly handle decorations with chained tags git log --decorate did not handle a tag pointed by another tag nicely. * bm/merge-base-octopus-dedup (2013-12-30) 2 commits (merged to 'next' on 2014-01-06 at 355d62b) + merge-base --octopus: reduce the result from get_octopus_merge_bases() + merge-base: separate --independent codepath into its own helper git merge-base --octopus used to leave cleaning up suboptimal result to the caller, but now it does the clean-up itself. * bs/mirbsd (2014-01-02) 1 commit (merged to 'next' on 2014-01-06 at d5cecbb) + Add MirBSD support to the build system. * cc/replace-object-info (2013-12-30) 11 commits (merged to 'next' on 2014-01-03 at 4473803) + replace info: rename 'full' to 'long' and clarify in-code symbols (merged to 'next' on 2013-12-17 at aeb9e18) + Documentation/git-replace: describe --format option + builtin/replace: unset read_replace_refs + t6050: add tests for listing with --format + builtin/replace: teach listing using short, medium or full formats + sha1_file: perform object replacement in sha1_object_info_extended() + t6050: show that git cat-file --batch fails with replace objects + sha1_object_info_extended(): add an unsigned flags parameter + sha1_file.c: add lookup_replace_object_extended() to pass flags + replace_object: don't check read_replace_refs twice + rename READ_SHA1_FILE_REPLACE flag to LOOKUP_REPLACE_OBJECT read_sha1_file() that is the workhorse to read the contents given an object name honoured object replacements, but there is no corresponding mechanism to sha1_object_info() that is used to obtain the metainfo (e.g. type size) about the object, leading callers to weird inconsistencies. * jh/rlimit-nofile-fallback (2013-12-18) 1 commit (merged to 'next' on 2014-01-03 at b56ae0c) + get_max_fd_limit(): fall back to OPEN_MAX upon getrlimit/sysconf failure When we figure out how many file descriptors to allocate for keeping packfiles open, a system with non-working getrlimit() could cause us to die(), but because we make this call only to get a rough estimate of how many is available and we do not even attempt to use up all file descriptors available ourselves, it is nicer to fall back to a reasonable low value rather than dying. * jk/credential-plug-leak (2014-01-02) 1 commit (merged to 'next' on 2014-01-06 at 88e29a3) + Revert prompt: clean up strbuf usage An earlier clean-up introduced an unnecessary memory leak. * jk/http-auth-tests-robustify (2014-01-02) 1 commit (merged to 'next' on 2014-01-06 at 7e87bba) + use distinct username/password for http auth tests Using the same username and password during the tests would not catch a potential breakage of sending one when we should be sending the other. * jk/oi-delta-base (2013-12-26) 2 commits (merged to 'next' on 2014-01-06 at 8cf3ed2) + cat-file: provide %(deltabase) batch format + sha1_object_info_extended: provide delta base sha1s Teach cat-file --batch to show delta-base object name for a packed object that is represented as a delta. * jk/sha1write-void (2013-12-26) 1 commit (merged to 'next' on 2014-01-06 at d8cd8ff) + do not pretend sha1write returns errors Code clean-up. * jk/test-framework-updates (2014-01-02) 3 commits (merged to 'next' on 2014-01-06 at f81f373) + t: drop known breakage test + t: simplify HARNESS_ACTIVE hack + t: set TEST_OUTPUT_DIRECTORY for sub-tests The basic test used to leave unnecessary trash directories in the t/ directory. * js/lift-parent-count-limit (2013-12-27) 1 commit (merged to 'next' on 2014-01-06 at b74133c) + Remove the line length limit for graft files There is no reason to have a hardcoded upper limit of the number of parents for an octopus merge, created via the graft mechanism. * km/gc-eperm (2014-01-02) 1 commit (merged to 'next' on 2014-01-06 at fe107de) + gc: notice gc processes run by other users A gc process running as a different user should be able to stop a new gc process from starting. * mh/path-max (2013-12-18) 2 commits (merged to 'next' on 2014-01-03 at 5227c9b) + builtin/prune.c: use
Re: [PATCH sb/diff-orderfile-config] diff test: reading a directory as a file need not error out
Jonathan Nieder jrnie...@gmail.com writes: There is no guarantee that strbuf_read_file must error out for directories. On some operating systems (e.g., Debian GNU/kFreeBSD wheezy), reading a directory gives its raw content: $ head -c5 / | cat -A ^AM-|^_^@^L$ As a result, 'git diff -O/' succeeds instead of erroring out on these systems, causing t4056.5 orderfile is a directory to fail. On some weird OS it might even make sense to pass a directory to the -O option and this is not a common user mistake that needs catching. Remove the test. Signed-off-by: Jonathan Nieder jrnie...@gmail.com --- Hi, t4056 is failing on systems using glibc with the kernel of FreeBSD[1]: | expecting success: | test_must_fail git diff -O/ --name-only HEAD^..HEAD | | a.h | b.c | c/Makefile | d.txt | test_must_fail: command succeeded: git diff -O/ --name-only HEAD^..HEAD | not ok 5 - orderfile is a directory How about this patch? Sounds sensible. Thanks. Thanks, Jonathan [1] https://buildd.debian.org/status/fetch.php?pkg=gitarch=kfreebsd-amd64ver=1%3A2.0~next.20140107-1stamp=1389379274 t/t4056-diff-order.sh | 4 1 file changed, 4 deletions(-) diff --git a/t/t4056-diff-order.sh b/t/t4056-diff-order.sh index 1ddd226..9e2b29e 100755 --- a/t/t4056-diff-order.sh +++ b/t/t4056-diff-order.sh @@ -68,10 +68,6 @@ test_expect_success POSIXPERM,SANITY 'unreadable orderfile' ' test_must_fail git diff -Ounreadable_file --name-only HEAD^..HEAD ' -test_expect_success 'orderfile is a directory' ' - test_must_fail git diff -O/ --name-only HEAD^..HEAD -' - for i in 1 2 do test_expect_success orderfile using option ($i) ' -- To unsubscribe from this list: send the line 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] t5531: further matching fixups
Jeff King p...@peff.net writes: ... but the failing test is actually somewhat broken in 'next' already. Hmph, in what way? I haven't seen t5531 breakage on 'next', with or without your series... fixes it, and should be done regardless of the other series. t/t5531-deep-submodule-push.sh | 1 + 1 file changed, 1 insertion(+) diff --git a/t/t5531-deep-submodule-push.sh b/t/t5531-deep-submodule-push.sh index 8c16e04..445bb5f 100755 --- a/t/t5531-deep-submodule-push.sh +++ b/t/t5531-deep-submodule-push.sh @@ -12,6 +12,7 @@ test_expect_success setup ' ( cd work git init + git config push.default matching mkdir -p gar/bage ( cd gar/bage -- To unsubscribe from this list: send the line 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/4] t7505: ensure cleanup after hook blocks merge
Matthieu Moy matthieu@grenoble-inp.fr writes:
Ryan Biesemeyer r...@yaauie.com writes:
+ test_when_finished git merge --abort
+ (
+git checkout -B other HEAD@{1}
Weird indentation (space/tab mix).
Also I do not quite see why the body has to be in a subshell.
--
To unsubscribe from this list: send the line 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 3/4] merge: make prepare_to_commit responsible for write_merge_state
Ryan Biesemeyer r...@yaauie.com writes:
When merging, make the prepare-commit-msg hook have access to the merge
state in order to make decisions about the commit message it is preparing.
What information is currently not available, and if available how
would that help the hook to formulate a better message?
I am not asking you to answer the question to me in an
e-mail response. The above is an example of a natural
question a reader of the above statement would have and
would wish the log message already answered when the reader
read it.
Since `abort_commit` is *only* called after `write_merge_state`, and a
successful commit *always* calls `drop_save` to clear the saved state, this
change effectively ensures that the merge state is also available to the
prepare-commit-msg hook and while the message is being edited.
Signed-off-by: Ryan Biesemeyer r...@yaauie.com
---
OK. The most important part is that this makes sure that these
intermediate state files never remains after the normal codepath
finishes what it does.
You seem to be only interested in prepare-commit-msg hook, but
because of your calling write_merge_state() early, these state files
will persist while we call finish() and they are also visible while
the post-merge hook is run. While I think it may be a good thing
that the post-merge hook too can view that information, the log
message makes it sound as if it is an unintended side effect of the
change.
builtin/merge.c| 3 ++-
t/t7505-prepare-commit-msg-hook.sh | 21 +
2 files changed, 23 insertions(+), 1 deletion(-)
diff --git a/builtin/merge.c b/builtin/merge.c
index 4941a6c..b7bfc9c 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -802,7 +802,6 @@ static void abort_commit(struct commit_list *remoteheads,
const char *err_msg)
error(%s, err_msg);
fprintf(stderr,
_(Not committing merge; use 'git commit' to complete the
merge.\n));
- write_merge_state(remoteheads);
exit(1);
}
@@ -816,6 +815,8 @@ N_(Please enter a commit message to explain why this
merge is necessary,\n
static void prepare_to_commit(struct commit_list *remoteheads)
{
struct strbuf msg = STRBUF_INIT;
+
+ write_merge_state(remoteheads);
strbuf_addbuf(msg, merge_msg);
strbuf_addch(msg, '\n');
if (0 option_edit)
diff --git a/t/t7505-prepare-commit-msg-hook.sh
b/t/t7505-prepare-commit-msg-hook.sh
index 697ecc0..7ccf870 100755
--- a/t/t7505-prepare-commit-msg-hook.sh
+++ b/t/t7505-prepare-commit-msg-hook.sh
@@ -183,4 +183,25 @@ test_expect_success 'with failing hook (merge)' '
)
'
+test_expect_success 'should have MERGE_HEAD (merge)' '
+
+ git checkout -B other HEAD@{1}
+ echo more file
Style: no SP between the redirection operator and its target, i.e.
echo more file
+ git add file
+ rm -f $HOOK
+ git commit -m other
+ git checkout -
+ write_script $HOOK -EOF
+ if [ -s $(git rev-parse --git-dir)/MERGE_HEAD ]; then
Style: no [], and no semicolon to join two lines of control
statement into one, i.e.
if test -s ...
then
+ exit 0
+ else
+ exit 1
+ fi
+ EOF
+ git merge other
+ test `git log -1 --pretty=format:%s` = Merge branch '''other'''
Style:
- After sh t7505-*.sh v -i fails for whatever reason, being
able to inspect the trash directory to see what actually was
produced is much easier way to debug than having to re-run the
command with sh -x to peek into what the test statement is
getting.
- $(...) is much easier to read than `...`, but you do not have
to use either if you follow the above.
i.e.
git log -1 --format=%s actual
echo Merge branch '\''other'\'' expect
test_cmp expect actual
+ test ! -s $(git rev-parse --git-dir)/MERGE_HEAD
+
+'
+
test_done
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 3/4] merge: make prepare_to_commit responsible for write_merge_state
Ryan Biesemeyer r...@yaauie.com writes: + write_script $HOOK -EOF + if [ -s $(git rev-parse --git-dir)/MERGE_HEAD ]; then + exit 0 + else + exit 1 + fi + EOF The script can be a one-liner write_scirpt $HOOK -\EOF test -s $(git rev-parse --git-dir)/MERGE_HEAD EOF can't it? I also do not think you want to have the rev-parse run while writing the script (rather, you would want it run inside the script, no?) + git merge other + test `git log -1 --pretty=format:%s` = Merge branch '''other''' + test ! -s $(git rev-parse --git-dir)/MERGE_HEAD + +' + test_done -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH/RFC] Introduce git submodule add|update --attach
Francesco Pretto cez...@gmail.com writes: Thanks for the comments, my replies below. Before, a couple of general questions: - I'm also writing some tests, should I commit them together with the feature patch? - to determine the attached/detached state I did this: head_detached= if test $(rev-parse --abbrev-ref HEAD) = HEAD then head_detached=true fi Use git symbolic-ref HEAD to read off of 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: [PATCH] Create HTML for technical/http-protocol.txt
Thomas Ackermann th.ac...@arcor.de writes: - Add to Documentation/Makefile - Start every TODO with a new line - Fix indentation error Signed-off-by: Thomas Ackermann th.ac...@arcor.de --- Documentation/Makefile| 1 + Documentation/technical/http-protocol.txt | 3 +-- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/Documentation/Makefile b/Documentation/Makefile index 36c58fc..b4a4c82 100644 --- a/Documentation/Makefile +++ b/Documentation/Makefile @@ -70,6 +70,7 @@ TECH_DOCS += technical/racy-git TECH_DOCS += technical/send-pack-pipeline TECH_DOCS += technical/shallow TECH_DOCS += technical/trivial-merge +TECH_DOCS += technical/http-protocol The output from ls Documentation/technical/[b-z]*.txt tells me that this patch makes the TECH_DOCS cover everything (in other words, this is not I was intereted in http-protocol.txt; I did not bother checking if there are others that are not formatted.). Which is good, but that is something you could have said at the very beginning as the motivation (which cannot be read from the patch) before going into details of what you did (which can be read in the patch). Also let's keep them in order and move the new line before the index-format, as h comes before i. SP_ARTICLES += $(TECH_DOCS) SP_ARTICLES += technical/api-index diff --git a/Documentation/technical/http-protocol.txt b/Documentation/technical/http-protocol.txt index d21d77d..c64a5e9 100644 --- a/Documentation/technical/http-protocol.txt +++ b/Documentation/technical/http-protocol.txt @@ -332,6 +332,7 @@ server advertises capability allow-tip-sha1-in-want. have_list = *PKT-LINE(have SP id LF) TODO: Document this further. + TODO: Don't use uppercase for variable names below. The Negotiation Algorithm @@ -376,10 +377,8 @@ The computation to select the minimal pack proceeds as follows Commands MUST appear in the following order, if they appear at all in the request stream: - * want * have - The stream is terminated by a pkt-line flush (). A single want or have command MUST have one hex formatted The resulting _source_ file read as txt becomes somewhat harder to read with these changes. Admittedly, it is mostly because the original text was meant to be easy to read in TEXT, not to be formatted via AsciiDoc into HTML. We can see that in many places in the formatted output with your patch. For example: - Strings like 200 OK, 304 Not Modified, HEAD are italicized (Discovering References section); GET request is shown as Roman body text. I think in our documentation it is customery to typeset these as-is things in monospaced. - dumb server reply:, smart server reply: (Smart Clients section) are shown in monospace just like the actual protocol message examples that follow them, but we would most likely want to see them as part of the body text. - As all the descriptions of steps in the Negotiation Algorithm are indented, the AsciiDoc formatted result becomes just a series of fixed-font blocks around there. So,... I think this patch may be a good first step, but it needs to be followed by another that cleans up the mark-up in order for the resulting HTML to be truly usable. 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 3/3] remote: introduce and fill branch-pushremote
Jeff King p...@peff.net writes:
It does not matter for actually pushing, because to do a non-default
push, you must always specify a remote. But @{publish} will ask the
question even if I am on 'side' now, what would happen if I were to
default-push on 'master'?.
In a similar wording to yours, it can be said that B@{upstream} is
what would happen if I were to default-pull on 'B'?.
A related tangent is what should B@{publish} should yield when there
is no triangular configuration variables like remote.pushdefault,
branch.B.pushremote and a possible future extension branch.B.push
are defined. The definition you gave, i.e. if I were to
default-push, gives a good guideline, I think.
I.e. git push origin master does tell us to push out 'master', but
it does not explicitly say what ref to update. It may be set to
update their remotes/satellite/master when we are emulating a fetch
in reverse by pushing, via e.g.
[remote origin]
push = refs/heads/master:refs/remotes/satellite/master
and it would be intuitive if we make master@{publish} resolve to
refs/remotes/satellite/master in such a case.
One thing that makes things a bit fuzzy is what should happen if
you have more than one push destinations. For example:
[remote home]
pushurl = ...
push = refs/heads/master:refs/remotes/satellite/master
[remote github]
pushurl = ...
mirror
[remote]
pushdefault = ???
git push home updates their 'refs/remotes/satellite/master' with
my 'master' with the above, while git push github will update
their 'refs/heads/master' with 'master'.
We can say master@{publish} is 'remotes/satellite/master' if
remote.pushdefault (or 'branch.master.pushremote) is set to 'home',
it is 'master' if it is 'github', and if git push while sitting on
'master' does not push it anywhere then master@{publish} is an
error. There may be a better definition of what if I were to
default-push really means, but I don't think of any offhand.
--
To unsubscribe from this list: send the line 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: Submodule relative URL problems
Jonathan Nieder jrnie...@gmail.com writes: * More typical usage is to clone from a bare repository (A.git), which wouldn't have this problem. But I think your case is worth supporting, too. I think the relative URL among nested submodules was specifically designed for hosting environments that have forest of bare repositories to serve as publishing or meeting points. I personally do not know where that worth supporting comes from, but if the change can be done without confusing the codepaths involved, I wouldn't object too much ;-) * Perhaps as a special case when the superproject is foo/.git, git should treat relative submodule paths as relative to foo/ instead of relative to foo/.git/. I think that would take care of your case without breaking existing normal practices, though after the patch is made it still wouldn't take care of people using old versions of git without that patch. What do you think? If we were to start adding special cases, it may also be sensible to clean up the more normal case of using subdirectories of bare repositories to represent nestedness (i.e. you can have a submodule logs.git, but not logs). We could reuse the $GIT_DIR/modules/$sm_name convention somehow perhaps? Any change to implement the special case you suggest likely has to touch the same place as such a change does, as both require some reorganization of the code that traverses the pathnames to find related repositories. -- To unsubscribe from this list: send the line 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: Tight submodule bindings
W. Trevor King wk...@tremily.us writes: Additional metadata, the initial checkout, and syncing down --- However, folks who do local submodule development will care about which submodule commit is responsible for that tree, because that's going to be the base of their local development. They also care about additional out-of-tree information, including the branch that commit is on. Well, please step back a bit. They do not have to care about what local branch they use to build follow-up work based on that commit. In fact, they would want to be able to develop more than one histories on top, which means more than one branches they can name themselves. The only thing they care about is where the result of their development _goes_, that is the URL and the branch of the remote they are pushing back to. I have a feeling that this is not specific for submodules---if you did this: git init here cd here git fetch $there master git reset --hard FETCH_HEAD and are given the resulting working tree to start hacking on, you would not know where the history came from, or where your result wants to go. So the branch that commit is on is a wrong thing to focus on. The branch the history built on top of the commit wants to go may be closer and these two are different. For already-initialized submodules, there are existing places in the submodule config to store this configuration: 1. HEAD for the checked-out branch, 2. branch.name.remote → remote.name.url for the upstream subproject URL, 4. branch.name.rebase (or pull.rebase) to prefer rebase over merge for integration, 5. … What happened to 3 ;-)? And also branch.name.merge may say on which of _their_ branch the commit you learn in the superproject tree would be found. If you are using centralized workflow, that would be the branch at your central repository to update with your push, too. In any case, local-branch is wrong from two aspects: 1. (obvious) It does not follow our naming convention not to use dashed-names for configuration variables. 2. You do not care about the names you use locally. The only thing you care about is where people meet at the central repository, i.e. where your result is pushed to. Syncing up -- In the previous section I explained how data should flow from .gitmodules into out-of-tree configs. s/should/you think should/, I think, but another way may be not to copy and read from there, which may be a lot simpler. Then upon switching branches of top-level superproject (which would update .gitmodules to the version on the new branch), you may get different settings automatically. But see below. ... Since you *will* want to share the upstream URL, I proposed using an explicit submodule.name.active setting to store the “do I care” information [2], instead of overloading submodule.name.url (I'd auto-sync the .gitmodule's submodule.name.url with the subproject's remote.origin.url unless the user opted out of .gitmodules syncing). It may not be a good idea to blindly update to whatever happens to be in .gitmodules, especially once submodule.*.url is initialized. I think we would need a bit more sophisticated mechanism than use from .git/config if set, otherwise use from .gitmodules, at least for the URL. It may not be limited to the URL, and other pieces of metainformation about submodules may need similar handling, but I'd refrain from extending the scope of discussion needlessly at this point. Imagine that your embedded appliance project used to use a submodule from git://k.org/linux-2.6 as its kernel component and now the upstream of it is instead called just git://k.org/linux; the URL specified by submodule.kernel.url in .gitmodules for the entry submodule.kernel.path=kernel would have changed from the former to the latter sometime in the superproject's history. Switching back to an old version in the superproject to fix an old bug in the maintenance track of the superproject would still want to push associated fixes to the kernel to k.org/linux, not linux-2.6, the latter of which may now be defunct [*1*]. One way to make it work semi-automatically is to keep track of what the user has seen in .gitmodules and offer chances to update entries in .git/config. If you cloned the superproject recently, you would only know about the new git://k.org/linux URL and that would be copied to .git/config (which the current code does). In addition, you would remember that we saw git://k.org/linux URL (which the current code does not). Upon switching back to an old version, we could notice that the URL in .gitmodules, which is git://k.org/linux-2.6, is not something the user has seen, and at that point we could ask the user to tell us what URL should be used, record the answer _and_ the fact that we saw that old URL as well. Then until the superproject updates the URL the next time to a
Re: [RFC v2] blame: new option --prefer-first to better handle merged cherry-picks
Bernhard R. Link brl...@debian.org writes: Allows to disable the git blame optimization of assuming that if there is a parent of a merge commit that has the exactly same file content, then only this parent is to be looked at. This optimization, while being faster in the usual case, means that in the case of cherry-picks the blamed commit depends on which other commits touched a file. If for example one commit A modified both files b and c. And there are commits B and C, B only modifies file b and C only modifies file c (so that no conflicts happen), and assume A is cherry-picked as A' and the two branches then merged: --o-B---A \ \ ---C---A'--M--- Then without this new option git blame blames the A|A' changes of file b to A while blaming the changes of c to A'. With the new option --prefer-first it blames both changes to the same commit and to the one more on the left side of the graph. Signed-off-by: Bernhard R. Link brl...@debian.org --- Documentation/blame-options.txt | 6 ++ builtin/blame.c | 7 +-- 2 files changed, 11 insertions(+), 2 deletions(-) Differences to first round: rename option and describe the effect instead of the implementation in documentation. I read the updated documentation three times but it still does not answer any of my questions I had in $gmane/239888, the most important part of which was: Yeah, the cherry-picked one will introduce the same change as the one that was cherry-picked, so if you look at the end result and ask where did _this_ line come from?, there are two equally plausible candidates, as blame output can give only one answer to each line. I still do not see why the one that is picked with the new option is better. At best, it looks to me that it is saying running with this option may (or may not) give a different answer, so run the command with and without it and see which one you like, which does not sound too useful to the end users. To put it another way, why/when would an end user choose to use this option? If the result of using this option is always better than without, why/when would an end user choose not to use this option? diff --git a/Documentation/blame-options.txt b/Documentation/blame-options.txt index 0cebc4f..b2e7fb8 100644 --- a/Documentation/blame-options.txt +++ b/Documentation/blame-options.txt @@ -48,6 +48,12 @@ include::line-range-format.txt[] Show the result incrementally in a format designed for machine consumption. +--prefer-first:: + If a line was introduced by two commits (for example via + a merged cherry-pick), prefer the commit that was + first merged in the history of always following the + first parent. + --encoding=encoding:: Specifies the encoding used to output author names and commit summaries. Setting it to `none` makes blame -- To unsubscribe from this list: send the line 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 v2] blame: new option --prefer-first to better handle merged cherry-picks
Bernhard R. Link brl+...@mail.brlink.eu writes: * Junio C Hamano gits...@pobox.com [140113 23:31]: I read the updated documentation three times but it still does not answer any of my questions I had in $gmane/239888, the most important part of which was: Yeah, the cherry-picked one will introduce the same change as the one that was cherry-picked, so if you look at the end result and ask where did _this_ line come from?, there are two equally plausible candidates, as blame output can give only one answer to each line. I still do not see why the one that is picked with the new option is better. Because: - it will blame the modifications of merged cherry-picked commit to only one commit. Without the option parts of the modification will be reported as coming from the one, parts will be reported to be from the other. With the option only one of those two commits is reported as the origin at the same time and not both. - it is more predictable which commit is blamed, so if one is interested in where some commit was introduced first into a mainline, one gets this information, and not somtimes a different one due to unrelated reasons. To put it another way, why/when would an end user choose to use this option? If the result of using this option is always better than without, why/when would an end user choose not to use this option? While the result is more consistent and more predictable in the case of merged cherry picks, it is also slower in every case. Consistent and predictable, perhaps, but I am not sure exact would be a good word. Wouldn't the result depend on which way the cherry pick went, and then the later merge went? In the particular topology you depicted in the log message, the end result may happen to point at the same commit for these two paths, but I am not sure how the change guarantees that we always point at the same original commit not the cherry-picked one, which was implied by the log message, if your cherry-pick and merge went in different direction in similar topologies. And that is why I said: At best, it looks to me that it is saying running with this option may (or may not) give a different answer, so run the command with and without it and see which one you like With the stress on different answer; it the change were with the option the result is always better, albeit you will have to wait longer, I would not have this much trouble accepting the change, though. -- To unsubscribe from this list: send the line 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 v2] blame: new option --prefer-first to better handle merged cherry-picks
Junio C Hamano gits...@pobox.com writes: Junio C Hamano gits...@pobox.com writes: While the result is more consistent and more predictable in the case of merged cherry picks, it is also slower in every case. Consistent and predictable, perhaps, but I am not sure exact would be a good word. Another thing I am not enthusiasitc about this change is that I am afraid that this may make git blame -- path and git log -- path work inconsistenly. The both cull side branches whenever one of the parents gave the resulting blob, even that parent is not the first one. But git blame --prefer-first -- path, afaict, behaves quite differently from git log --first-parent -- path, even though they share similar option names, adding more to the confusion. I think I am starting to understand why this patch felt wrong to me. This wasn't about --first-parent at all, and you are correct that you didn't call the option as such), but I somehow thought that they were related; perhaps the fact that both disable the if the result exactly matches one parent, all the other parents can be culled to simplify the history logic blinded me. In reality, the new flag is a lot closer in spirit to the total opposite of --first-parent, i.e. --full-history. That option also disables that if same to one parent, other parents do not matter logic, but its effect is quite different. It makes the other histories that did not have to have contributed the end result shown in the output. Now, when we step back and think about how the normal git blame logic apportions the blame to multiple parents when there is no exact match, it does so in a pretty arbitrary way. It lets earlier parents to claim the responsibility and later parents only get leftover contents that weren't claimed by the earlier ones. We can call that favouring earler ones, i.e. --prefer-first. It was implemented this way, not because this order makes any sense, but primarily because no order is particularly better than any other, and the designer (me) happened to have picked the easiest one at random. The pick the one that exactly matches if exists can be thought of an easy hack to hide the problems that come from this arbitrary choice. Without it, if the result matches the second parent (i.e. a typical merge of a work done on the topic branch while the mainline has been quiescent in the same area), the give earlier parents a chance to claim responsiblity before later ones rule would have split the blame for parts that weren't changed in the side branch topic to the mainline and blame would have been passed to the side branch only for the portion that were changed by the side branch. Instead, pass the whole blame to the one that exactly matches hack keeps larger blocks of text unsplit, clumping related contents together as long as possible while we traverse the history. It is an easy hack, because we only need to compare the object name, but a logical extension to it would have been to compute the similarity scores between the result and each of the parents, sort the parents by that similarity score order, and give more similar ones a chance to claim responsibility before less similar ones. We could call it favouring similar ones, i.e. --prefer-similar or something. That would have made the result more stable. Imagine that in one history, a merge's result matchs exactly the second parent, and in another history, a merge's result almost matches exactly the second parent but the difference is the result adds one blank line at the end of the file relative to what the second parent has. With the current code, blaming the file will get quite a different result. In the former history, the sub-history leading to the second parent of the merge will get all the blame, but in the latter history, the sub-history leading to the first parent of the merge will have a chance to claim the responsibility for the shared part before the second parent has a say in the output. If we sorted the parents in the similarity order and gave the first refusal right to more similar parents before less similar ones, then the resulting output from git blame would be very similar in these two histories, which would be a very desirable property. If the only difference between the results of the merge in the former and the latter histories is one blank line at the end of the file in question, blames for the remaining part of the file should be assigned the same between the two histories, but the pass the entire blame to the second parent only when the second parent exactly matches hack gets in the way for that ideal, and sort the parents in similarity order will fix that. Of course, it would make the computation a lot more costly, but it would make the behaviour more predictable and understandable. But that is a different tangent. I think the new feature introduced by your change can be explained as 'git blame' uses the same history simplification as the commands in the 'git log
[ANNOUNCE] Git v1.8.5.3
The latest maintenance release Git v1.8.5.3 is now available at the usual places, backporting the fixes that happened on the 'master' front. The release tarballs are found at: http://code.google.com/p/git-core/downloads/list and their SHA-1 checksums are: 767aa30c0f569f9b6e04cb215dfeec0c013c355a git-1.8.5.3.tar.gz 47da8e2b1d23ae501ee2c03414c04f8225079037 git-htmldocs-1.8.5.3.tar.gz e4b66ca3ab1b089af651bf742aa030718e9af978 git-manpages-1.8.5.3.tar.gz The following public repositories all have a copy of the v1.8.5.3 tag and the maint branch that the tag points at: url = https://kernel.googlesource.com/pub/scm/git/git url = git://repo.or.cz/alt-git.git url = https://code.google.com/p/git-core/ url = git://git.sourceforge.jp/gitroot/git-core/git.git url = git://git-core.git.sourceforge.net/gitroot/git-core/git-core url = https://github.com/gitster/git Also, http://www.kernel.org/pub/software/scm/git/ has copies of the release tarballs. Git v1.8.5.3 Release Notes == Fixes since v1.8.5.2 * The --[no-]informative-errors options to git daemon were parsed a bit too loosely, allowing any other string after these option names. * A gc process running as a different user should be able to stop a new gc process from starting. * An earlier clean-up introduced an unnecessary memory leak to the credential subsystem. * git mv A B/, when B does not exist as a directory, should error out, but it didn't. * git rev-parse revs -- paths did not implement the usual disambiguation rules the commands in the git log family used in the same way. * git cat-file --batch=, an admittedly useless command, did not behave very well. Also contains typofixes, documentation updates and trivial code clean-ups. Changes since v1.8.5.2 are as follows: Jeff King (5): rev-parse: correctly diagnose revision errors before -- rev-parse: be more careful with munging arguments cat-file: pass expand_data to print_object_or_die cat-file: handle --batch format with missing type/size Revert prompt: clean up strbuf usage Johannes Sixt (1): mv: let 'git mv file no-such-dir/' error out on Windows, too Junio C Hamano (1): Git 1.8.5.3 Kyle J. McKay (1): gc: notice gc processes run by other users Matthieu Moy (1): mv: let 'git mv file no-such-dir/' error out Nguyễn Thái Ngọc Duy (1): daemon: be strict at parsing parameters --[no-]informative-errors Ralf Thielow (1): l10n: de.po: fix translation of 'prefix' Ramkumar Ramachandra (1): for-each-ref: remove unused variable Thomas Ackermann (1): pack-heuristics.txt: mark up the file header properly W. Trevor King (1): Documentation/gitmodules: Only 'update' and 'url' are required -- To unsubscribe from this list: send the line 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* manpage for git-pull mentions a non-valid option -m in a comment
Ivan Zakharyaschev i...@altlinux.org writes: Hello! git-1.8.4.4 The manpage for git-pull mentions -m in a comment: --edit, -e, --no-edit Invoke an editor before committing successful mechanical merge to further edit the auto-generated merge message, so that the user can explain and justify the merge. The --no-edit option can be used to accept the auto-generated message (this is generally discouraged). The --edit (or -e) option is still useful if you are giving a draft message with the -m option from the command line and want to edit it in the editor. but it is not accepted actually: I do not think git pull ever had the -m message option; what you are seeing probably is a fallout from attempting to share the documentation pieces between git pull and git merge too agressively without proofreading. It seems that there are two issues; here is an untested attempt to fix. -- 8 -- Subject: Documentaiotn: exclude irrelevant options from git pull 10eb64f5 (git pull manpage: don't include -n from fetch-options.txt, 2008-01-25) introduced a way to exclude some parts of included source when building git-pull documentation, and later 409b8d82 (Documentation/git-pull: put verbosity options before merge/fetch ones, 2010-02-24) attempted to use the mechanism to exclude some parts of merge-options.txt when used from git-pull.txt. However, the latter did not have an intended effect, because the macro git-pull used to decide if the source is included in git-pull documentation were defined a bit too late. Define the macro before it is used to fix this. Even though --[no-]edit can be used with git pull, the explanation of the interaction between this option and the -m option does not make sense within the context of git pull. Use the same conditional inclusion mechanism to remove this part from git pull documentation, while keeping it for git merge. Reported-by: Ivan Zakharyaschev Signed-off-by: Junio C Hamano gits...@pobox.com --- Documentation/git-pull.txt | 4 ++-- Documentation/merge-options.txt | 9 ++--- 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/Documentation/git-pull.txt b/Documentation/git-pull.txt index 6083aab..200eb22 100644 --- a/Documentation/git-pull.txt +++ b/Documentation/git-pull.txt @@ -99,10 +99,10 @@ must be given before the options meant for 'git fetch'. Options related to merging ~~ -include::merge-options.txt[] - :git-pull: 1 +include::merge-options.txt[] + -r:: --rebase[=false|true|preserve]:: When true, rebase the current branch on top of the upstream diff --git a/Documentation/merge-options.txt b/Documentation/merge-options.txt index afba8d4..e134315 100644 --- a/Documentation/merge-options.txt +++ b/Documentation/merge-options.txt @@ -14,9 +14,12 @@ inspect and further tweak the merge result before committing. further edit the auto-generated merge message, so that the user can explain and justify the merge. The `--no-edit` option can be used to accept the auto-generated message (this is generally - discouraged). The `--edit` (or `-e`) option is still useful if you are - giving a draft message with the `-m` option from the command line - and want to edit it in the editor. + discouraged). +ifndef::git-pull[] +The `--edit` (or `-e`) option is still useful if you are +giving a draft message with the `-m` option from the command line +and want to edit it in the editor. +endif::git-pull[] + Older scripts may depend on the historical behaviour of not allowing the user to edit the merge log message. They will see an editor opened when -- To unsubscribe from this list: send the line 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 v2] blame: new option --prefer-first to better handle merged cherry-picks
Junio C Hamano gits...@pobox.com writes: The pick the one that exactly matches if exists can be thought of an easy hack to hide the problems that come from this arbitrary choice. ... Instead, pass the whole blame to the one that exactly matches hack keeps larger blocks of text unsplit, clumping related contents together as long as possible while we traverse the history. It is an easy hack, because we only need to compare the object name, but a logical extension to it would have been to compute the similarity scores between the result and each of the parents, sort the parents by that similarity score order, and give more similar ones a chance to claim responsibility before less similar ones. We could call it favouring similar ones, i.e. --prefer-similar or something. Extending along the tangent further. Another thing that I found the argument in the proposed log message of the patch weak was that the claim that changed code will assign the blame to the same commit for both path b and c. There are two reasons why. One is that we do not look at b while chasing the ancestry of c, so if a different traverse order assigns the blame to the same commit for them, it is a mere happenstance. But a more important reason is that the changed code will still assign the blame for different commits if the final merge were made in the opposite direction. In your original topology, we skip over the first parent and give the whole blame to the second parent without the change, and with the change, we stop doing so and instead give some blame to the first parent and then allow the second parent a chance to claim the blame for the remainder. But in a history where the final merge went in the opposite direction, even with the change, we compare with the first parent (which was the second one in your original topology) with the result, find out that the contents exactly match, and that parent grabs the whole blame. So in that sense, the updated code that consistently gives earlier parents chance to claim the blame before later ones does not behave consitently on the same history with different merge parent order. That makes me think that the reason why the result you got with the change is better (assuming it is better) is _not_ because the updated code lets earlier parents give chance to claim the blame; it could be an indication that the keep larger blocks of text unsplit, clumping related contents together as long as possible heuristics is what prevents us from having a better result. If that is really the case, that would mean that letting the blame split early would give us a better result. I alluded to give more similar parents first chance to claim responsibility before less similar ones in the previous message, but perhaps this is indicating that we might get a better result if we did the opposite---instead of assigning blames to earlier parents and then to later ones, compare the result with each parent, order the parents by how few lines of blame they could claim if each of them were allowed to go first, and then actually compute and assign the blame in that order, favouring dissimilar ones. That may produce the result you are after in a more consistent way, regardless of the merge order. I think I've done thinking about this issue, at least for now. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Re* manpage for git-pull mentions a non-valid option -m in a comment
Torsten Bögershausen tbo...@web.de writes: Subject: Documentaiotn: exclude irrelevant options from git pull ^^ (Small nit ??) ;-). They are now a small two patch series, with typofix for the above. -- To unsubscribe from this list: send the line 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] Documentation: exclude irrelevant options from git pull
10eb64f5 (git pull manpage: don't include -n from fetch-options.txt, 2008-01-25) introduced a way to exclude some parts of included source when building git-pull documentation, and later 409b8d82 (Documentation/git-pull: put verbosity options before merge/fetch ones, 2010-02-24) attempted to use the mechanism to exclude some parts of merge-options.txt when used from git-pull.txt. However, the latter did not have an intended effect, because the macro git-pull used to decide if the source is included in git-pull documentation were defined a bit too late. Define the macro before it is used to fix this. Signed-off-by: Junio C Hamano gits...@pobox.com --- * To be applied on top of 409b8d82 (Documentation/git-pull: put verbosity options before merge/fetch ones, 2010-02-24) Documentation/git-pull.txt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Documentation/git-pull.txt b/Documentation/git-pull.txt index d47f9dd..0e7a1fe 100644 --- a/Documentation/git-pull.txt +++ b/Documentation/git-pull.txt @@ -42,10 +42,10 @@ OPTIONS Options related to merging ~~ -include::merge-options.txt[] - :git-pull: 1 +include::merge-options.txt[] + --rebase:: Instead of a merge, perform a rebase after fetching. If there is a remote ref for the upstream branch, and this branch -- 1.8.5.3-493-gb139ac2 -- To unsubscribe from this list: send the line 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] Documentation: git pull does not have the -m option
Even though --[no-]edit can be used with git pull, the explanation of the interaction between this option and the -m option does not make sense within the context of git pull. Use the conditional inclusion mechanism to remove this part from git pull documentation, while keeping it for git merge. Reported-by: Ivan Zakharyaschev Signed-off-by: Junio C Hamano gits...@pobox.com --- * Merge the result of applying 1/2 on top of 409b8d8 to 66fa1b2, and then apply this. Documentation/merge-options.txt | 9 ++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/Documentation/merge-options.txt b/Documentation/merge-options.txt index f192cd2..d462bcc 100644 --- a/Documentation/merge-options.txt +++ b/Documentation/merge-options.txt @@ -14,9 +14,12 @@ inspect and further tweak the merge result before committing. further edit the auto-generated merge message, so that the user can explain and justify the merge. The `--no-edit` option can be used to accept the auto-generated message (this is generally - discouraged). The `--edit` (or `-e`) option is still useful if you are - giving a draft message with the `-m` option from the command line - and want to edit it in the editor. + discouraged). +ifndef::git-pull[] +The `--edit` (or `-e`) option is still useful if you are +giving a draft message with the `-m` option from the command line +and want to edit it in the editor. +endif::git-pull[] + Older scripts may depend on the historical behaviour of not allowing the user to edit the merge log message. They will see an editor opened when -- 1.8.5.3-493-gb139ac2 -- To unsubscribe from this list: send the line 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] refname_match(): always use the rules in ref_rev_parse_rules
Michael Haggerty mhag...@alum.mit.edu writes:
We used to use two separate rules for the normal ref resolution
dwimming and dwimming done to decide which remote ref to grab. The
third parameter to refname_match() selected which rules to use.
When these two rules were harmonized in
2011-11-04 dd621df9cd refs DWIMmery: use the same rule for both git
fetch and others
, ref_fetch_rules was #defined to avoid potential breakages for
in-flight topics.
It is now safe to remove the backwards-compatibility code, so remove
refname_match()'s third parameter, make ref_rev_parse_rules private to
refs.c, and remove ref_fetch_rules entirely.
Suggested-by: Junio C Hamano gits...@pobox.com
Signed-off-by: Michael Haggerty mhag...@alum.mit.edu
---
See
http://article.gmane.org/gmane.comp.version-control.git/240305
in which Junio made the suggestion and wrote most of the commit
message :-)
;-) ...and on top of it this may be an obvious endgame follow-up.
was done mindlessly and mechanically, so there may be some slip-ups,
though.
refs.c | 24 +---
1 file changed, 13 insertions(+), 11 deletions(-)
diff --git a/refs.c b/refs.c
index 5a10c25..b1c9cf5 100644
--- a/refs.c
+++ b/refs.c
@@ -1886,16 +1886,16 @@ static const char *ref_rev_parse_rules[] = {
refs/tags/%.*s,
refs/heads/%.*s,
refs/remotes/%.*s,
- refs/remotes/%.*s/HEAD,
- NULL
+ refs/remotes/%.*s/HEAD
};
int refname_match(const char *abbrev_name, const char *full_name)
{
- const char **p;
+ int i;
const int abbrev_name_len = strlen(abbrev_name);
- for (p = ref_rev_parse_rules; *p; p++) {
+ for (i = 0; i ARRAY_SIZE(ref_rev_parse_rules); i++) {
+ const char **p = ref_rev_parse_rules[i];
if (!strcmp(full_name, mkpath(*p, abbrev_name_len,
abbrev_name))) {
return 1;
}
@@ -1963,11 +1963,13 @@ static char *substitute_branch_name(const char
**string, int *len)
int dwim_ref(const char *str, int len, unsigned char *sha1, char **ref)
{
char *last_branch = substitute_branch_name(str, len);
- const char **p, *r;
+ int i;
+ const char *r;
int refs_found = 0;
*ref = NULL;
- for (p = ref_rev_parse_rules; *p; p++) {
+ for (i = 0; i ARRAY_SIZE(ref_rev_parse_rules); i++) {
+ const char **p = ref_rev_parse_rules[i];
char fullref[PATH_MAX];
unsigned char sha1_from_ref[20];
unsigned char *this_result;
@@ -1994,11 +1996,11 @@ int dwim_ref(const char *str, int len, unsigned char
*sha1, char **ref)
int dwim_log(const char *str, int len, unsigned char *sha1, char **log)
{
char *last_branch = substitute_branch_name(str, len);
- const char **p;
- int logs_found = 0;
+ int logs_found = 0, i;
*log = NULL;
- for (p = ref_rev_parse_rules; *p; p++) {
+ for (i = 0; i ARRAY_SIZE(ref_rev_parse_rules); i++) {
+ const char **p = ref_rev_parse_rules[i];
struct stat st;
unsigned char hash[20];
char path[PATH_MAX];
@@ -3368,8 +3370,8 @@ char *shorten_unambiguous_ref(const char *refname, int
strict)
if (!nr_rules) {
size_t total_len = 0;
- /* the rule list is NULL terminated, count them first */
- for (nr_rules = 0; ref_rev_parse_rules[nr_rules]; nr_rules++)
+ /* Count the bytesize needed to hold rule strings */
+ for (nr_rules = 0; ARRAY_SIZE(ref_rev_parse_rules); nr_rules++)
/* no +1 because strlen(%s) strlen(%.*s) */
total_len += strlen(ref_rev_parse_rules[nr_rules]);
--
To unsubscribe from this list: send the line 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: check-ref-format and rev-parse can not handle branches with an @ in their name combined with @{u}
Keith Derrick keith.derr...@lge.com writes:
I couldn't find a duplicate in the JIRA instance.
Don't worry, we do not use any JIRA instance ;-)
According to the documentation of check-ref-format, a branch name such
as @mybranch is valid.
Correct.
Yet 'git check-ref-format --branch @mybranch@{u}'
claims @mybranch is an invalid branch name.
I do not think it claims any such thing.
$ git check-ref-format --branch @foo@{u}; echo $?
fatal: '@foo@{u}' is not a valid branch name
128
$ git check-ref-format --branch @foo; echo $?
@foo
0
The former asks Is the string '@foo@{u}' a suitable name to give a
branch? and the answer is no. The latter asks the same question
about the string '@foo', and the answer is yes.
So I do not see any bug here.
--
To unsubscribe from this list: send the line 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: check-ref-format and rev-parse can not handle branches with an @ in their name combined with @{u}
Jeff King p...@peff.net writes: Is that what --branch does? I have never used it, but the manpage seems to suggest it is about _parsing_ (which, IMHO, means it probably should have been an option to rev-parse, but that is another issue altogether). Ahh, of course you are right. I never use it, and somehow thought it was just prepending refs/heads/ to its arguments, but it seems to want to do a lot more than 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: Diagnosing stray/stale .keep files -- explore what is in a pack?
Martin Langhoff martin.langh...@gmail.com writes: On Wed, Jan 15, 2014 at 4:12 AM, Jeff King p...@peff.net wrote: We see these occasionally at GitHub, too. I haven't yet figured out a definite cause, though whatever it is, it's relatively rare. Do you have a cleanup script to safely get rid of stale .keep and .lock files? I wonder what other stale bits merit a cleanup... As long as we can reliably determine that it is safe to do so without risking races, automatically cleaning .lock files is a good thing to do. Cleaning .keep files needs the same care and a bit more, though. You of course have to be sure that no other concurrent process is in the middle of doing something, but you also need to be sure that the .keep file is not a marker created by the end user to say keep this pack, do not subject its contents to repacking after a careful repacking of the stable part of the history. -- To unsubscribe from this list: send the line 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] send-email: If the ca path is not specified, use the defaults
Igor Gnatenko i.gnatenko.br...@gmail.com writes:
From: Ruben Kerkhof ru...@rubenkerkhof.com
I use gmail for sending patches.
If I have the following defined in my ~/.gitconfig:
[sendemail]
smtpencryption = tls
smtpserver = smtp.gmail.com
smtpuser = ru...@rubenkerkhof.com
smtpserverport = 587
and try to send a patch, this fails with:
STARTTLS failed! SSL connect attempt failed with unknown error
error:14090086:SSL routines:SSL3_GET_SERVER_CERTIFICATE:certificate
verify failed at /usr/libexec/git-core/git-send-email line 1236.
... because? Is it because the cert_path on your platform is
different from /etc/ssl/certs? What platform was this anyway?
I see in the original discussion in your bugzilla entry that
/etc/ssl/certs/ _is_ your ssl cert directory, but I wonder why
then this part of the existing codepath does not work for you:
if (!defined $smtp_ssl_cert_path) {
$smtp_ssl_cert_path = /etc/ssl/certs;
}
if ($smtp_ssl_cert_path eq ) {
return (SSL_verify_mode = SSL_VERIFY_NONE());
} elsif (-d $smtp_ssl_cert_path) {
return (SSL_verify_mode = SSL_VERIFY_PEER(),
SSL_ca_path = $smtp_ssl_cert_path);
} elsif (-f $smtp_ssl_cert_path) {
return (SSL_verify_mode = SSL_VERIFY_PEER(),
SSL_ca_file = $smtp_ssl_cert_path);
} else ...
We set cert_path to /etc/ssl/certs and return SSL_VERIFY_PEER() as
the SSL_verify_mode, and also return that directory as SSL_ca_path,
which means the callsites of the ssl_verify_params sub, which are
Net::SMTP:SSL-start_SSL() or IO::Socket::SSL::set_client_defaults(),
will be told with SSL_ca_path (not SSL_ca_file) that
/etc/ssl/certs is the directory to find the certificates in.
Now, http://search.cpan.org/~sullr/IO-Socket-SSL-1.964/lib/IO/Socket/SSL.pm
says:
SSL_ca_file | SSL_ca_path
Usually you want to verify that the peer certificate has been
signed by a trusted certificate authority. In this case you
should use this option to specify the file (SSL_ca_file) or
directory (SSL_ca_path) containing the certificate(s) of the
trusted certificate authorities.
So I have to wonder why isn't this working. Without knowing why
using SSL_ca_path for the certificate directory is not working on
your platform, the patch looks more like a band-aid that works
around an undiagnosed malfunction of IO::Socket::SSL on your
platform than a real solution to the breakage, no?
Puzzled...
By the way, please do not tell the readers of proposed log message
to refer to your custom Reference: footer to answer the
... because? question at the beginning of this message. Your
proposed log message should have allowed anybody who comments on
your patch to make the above observation without referring to
external website.
Having said all that.
Does this patch affect people on other distros, who never set the
cert_path in their configuration and have been relying on the
hardwired default? If so in what way?
My knee-jerk answer to that question is that it would negatively
affect folks only if their platform does have the certs in
/etc/ssl/certs/, but their Perl IO::Socket::SSL somehow defaults to
a wrong location, which is already a broken installation. In that
sense, I suspect that the potential downside of this patch may be
small, but I'd prefer to see evidence that the patch author thought
through ramifications of the change in the proposed log message ;-)
Also, if the above observation is correct, i.e. we are calling
IO::Socket::SSL with SSL_ca_path set to a correct directory but
somehow your platform is misbehaving and not detecting it as a
proper SSL_ca_path, then it could be argued that the code before
this patch catered people on platforms with one class of breakage,
namely IO::Socket::SSL does not work with default configuration
without SSL_ca_file nor SSL_ca_path, and the code with this patch
caters people on platforms with another class of breakage, namely
IO::Socket::SSL does not work when told with SSL_ca_path where the
certificate directory is (or it could be /etc/ssl/certs is a
directory that ought to be usable as SSL_ca_path, but it lacks
necessary hash symlinks). Sort of like robbing Peter to pay Paul,
which is not very nice in principle.
Tested-by: Igor Gnatenko i.gnatenko.br...@gmail.com
Signed-off-by: Ruben Kerkhof ru...@rubenkerkhof.com
Reference: https://bugzilla.redhat.com/show_bug.cgi?id=1043194
---
git-send-email.perl | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/git-send-email.perl b/git-send-email.perl
index 3782c3b..689944f 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -1095,7 +1095,8 @@ sub ssl_verify_params {
}
if (!defined $smtp_ssl_cert_path) {
- $smtp_ssl_cert_path = /etc/ssl/certs;
+ # use the OpenSSL defaults
+ return (SSL_verify_mode =
Re: git-log --cherry-pick gives different results when using tag or tag^{}
Jeff King p...@peff.net writes:
[+cc Junio, as the bug blames to him]
...
I think what is happening is that we used to apply the SYMMETRIC_LEFT
flag directly to the commit. Now we apply it to the tag, and it does not
seem to get propagated. The patch below fixes it for me, but I have no
idea if we actually need to be setting the other flags, or just
SYMMETRIC_LEFT. I also wonder if the non-symmetric two-dot case needs to
access any pointed-to commit and propagate flags in a similar way.
Thanks.
Where do we pass down other flags from tags to commits? For
example, if we do this:
$ git log ^v1.8.5 master
we mark v1.8.5 tag as UNINTERESTING, and throw that tag (not commit
v1.8.5^0) into revs-pending.objects[]. We do the same for 'master',
which is a commit.
Later, in prepare_revision_walk(), we call handle_commit() on them,
and unwrap the tag v1.8.5 to get v1.8.5^0, and then handles that
commit object with flags obtained from the tag object. This code
only cares about UNINTERESTING and manually propagates it.
Perhaps that code needs to propagate at least SYMMETRIC_LEFT down to
the commit object as well, no? With your patch, the topmost level
of tag object and the eventual commit object are marked with the
flag, but if we were dealing with a tag that points at another tag
that in turn points at a commit, the intermediate tag will not be
marked with SYMMETRIC_LEFT (nor UNINTERESTING for that matter),
which may not affect the final outcome, but it somewhat feels wrong.
How about doing it this way instead (totally untested, though)?
revision.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/revision.c b/revision.c
index a68fde6..def070e 100644
--- a/revision.c
+++ b/revision.c
@@ -276,6 +276,7 @@ static struct commit *handle_commit(struct rev_info *revs,
return NULL;
die(bad object %s, sha1_to_hex(tag-tagged-sha1));
}
+ object-flags |= flags;
}
/*
@@ -287,7 +288,6 @@ static struct commit *handle_commit(struct rev_info *revs,
if (parse_commit(commit) 0)
die(unable to parse commit %s, name);
if (flags UNINTERESTING) {
- commit-object.flags |= UNINTERESTING;
mark_parents_uninteresting(commit);
revs-limited = 1;
}
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
revision: propagate flag bits from tags to pointees
With the previous fix 895c5ba3 (revision: do not peel tags used in
range notation, 2013-09-19), handle_revision_arg() that processes
command line arguments for the git log family of commands no
longer directly places the object pointed by the tag in the pending
object array when it sees a tag object. We used to place pointee
there after copying the flag bits like UNINTERESTING and
SYMMETRIC_LEFT.
This change meant that any flag that is relevant to later history
traversal must now be propagated to the pointed objects (most often
these are commits) while starting the traversal, which is partly
done by handle_commit() that is called from prepare_revision_walk().
We did propagate UNINTERESTING, but did not do so for others, most
notably SYMMETRIC_LEFT. This caused git log --left-right v1.0...
(where v1.0 is a tag) to start losing the leftness from the
commit the tag points at.
Signed-off-by: Junio C Hamano gits...@pobox.com
---
* Comes directly on top of the faulty commit, so that we could
backport it to 1.8.4.x series.
revision.c | 2 +-
t/t6000-rev-list-misc.sh | 11 +++
2 files changed, 12 insertions(+), 1 deletion(-)
diff --git a/revision.c b/revision.c
index 7010aff..6d1c8f9 100644
--- a/revision.c
+++ b/revision.c
@@ -265,6 +265,7 @@ static struct commit *handle_commit(struct rev_info *revs,
struct object *object
return NULL;
die(bad object %s, sha1_to_hex(tag-tagged-sha1));
}
+ object-flags |= flags;
}
/*
@@ -276,7 +277,6 @@ static struct commit *handle_commit(struct rev_info *revs,
struct object *object
if (parse_commit(commit) 0)
die(unable to parse commit %s, name);
if (flags UNINTERESTING) {
- commit-object.flags |= UNINTERESTING;
mark_parents_uninteresting(commit);
revs-limited = 1;
}
diff --git a/t/t6000-rev-list-misc.sh b/t/t6000-rev-list-misc.sh
index 15e3d64..b84d6b0 100755
--- a/t/t6000-rev-list-misc.sh
+++ b/t/t6000-rev-list-misc.sh
@@ -56,4 +56,15 @@ test_expect_success 'rev-list A..B and rev-list ^A B are the
same' '
test_cmp expect actual
'
+test_expect_success 'symleft flag bit is propagated down from tag' '
+ git log --format=%m %s --left-right v1.0...master actual
+ cat expect -\EOF
+two
+one
+another
+that
+ EOF
+ test_cmp expect actual
+'
+
test_done
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/5] interpret_branch_name bug potpourri
Jeff King p...@peff.net writes:
On Wed, Jan 15, 2014 at 12:00:03AM -0500, Jeff King wrote:
$ git rev-parse --symbolic-full-name HEAD@{u}
refs/remotes/origin/master
$ git rev-parse --symbolic-full-name @mybranch@{u}
@mybranch@{u}
fatal: ambiguous argument '@mybranch@{u}': unknown revision or path
not in the working tree.
So I do think there is a bug. The interpret_branch_name parser somehow
gets confused by the @ in the name.
The somehow is because we only look for the first @, and never
consider any possible marks after that. The series below fixes it, along
with two other bugs I found while looking at this code. Ugh. Remind me
never to look at our object name parser ever again.
I feel pretty good that this is fixing real bugs and not regressing
anything else. I would not be surprised if there are other weird things
lurking, though. See the discussion in patch 4.
[1/5]: interpret_branch_name: factor out upstream handling
[2/5]: interpret_branch_name: rename cp variable to at
[3/5]: interpret_branch_name: always respect namelen parameter
[4/5]: interpret_branch_name: avoid @{upstream} past colon
[5/5]: interpret_branch_name: find all possible @-marks
-Peff
All the steps looked very sensible. Thanks for a pleasant read.
--
To unsubscribe from this list: send the line 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] send-email: If the ca path is not specified, use the defaults
Ruben Kerkhof ru...@rubenkerkhof.com writes: ... because? Is it because the cert_path on your platform is different from /etc/ssl/certs? What platform was this anyway? This is Fedora rawhide, git-1.8.5.2-1.fc21.x86_64, perl-IO-Socket-SSL-1.962-1.fc21.noarch I see in the original discussion in your bugzilla entry that /etc/ssl/certs/ _is_ your ssl cert directory, but I wonder why then this part of the existing codepath does not work for you: Actually, it's not a directory but a symlink to a directory: [ruben@vm ~]$ ls -l /etc/ssl/certs lrwxrwxrwx. 1 root root 16 Jan 11 12:04 /etc/ssl/certs - ../pki/tls/certs Just to rule that out I set smtpsslcertpath = /etc/pki/tls/certs, but that doesn't work either. Yeah, I suspect as much, as -d test would dereference symlinks and would see /etc/ssl/certs is a symlink pointing at a directory. ... I posted the patch to Fedora's bugzilla, since this was biting me on Fedora, and Igor took the liberty to forward it. Not that I mind of course, but please don't take this patch as a proper fix. I don't pretend to fully understand the code and the implications, much less the impact on other platforms. That is fine, and thanks for starting discussion for a proper fix (the thanks go to both of you). Also, if the above observation is correct, i.e. we are calling IO::Socket::SSL with SSL_ca_path set to a correct directory but somehow your platform is misbehaving and not detecting it as a proper SSL_ca_path, then it could be argued that the code before this patch catered people on platforms with one class of breakage, namely IO::Socket::SSL does not work with default configuration without SSL_ca_file nor SSL_ca_path, and the code with this patch caters people on platforms with another class of breakage, namely IO::Socket::SSL does not work when told with SSL_ca_path where the certificate directory is (or it could be /etc/ssl/certs is a directory that ought to be usable as SSL_ca_path, but it lacks necessary hash symlinks). Sort of like robbing Peter to pay Paul, which is not very nice in principle. I suspect that's exactly the case,... Actually, there is another possibility. Perhaps on your system, even though the current code thinks /etc/ssl/certs/ is usable as SSL_ca_path, the directory is not meant to be usable as such, and the default OpenSSL uses the equivalent of SSL_ca_file and uses the single certificate bundle file without consulting other stuff in the directory. And that is not a broken installation at all, which is sort of consistent with your observation here: ... As a last check, I set smtpsslcertpath = /etc/pki/tls/cert.pem in ~/.gitconfig and git-send-email works fine now. Which would mean that the existing code, by blindly defaulting to /etc/ssl/certs/ and misdiagnosing that the directory is meant to be used as SSL_ca_path, breaks a set-up that otherwise _should_ work. I see that the original change that introduced the defaulting to /etc/ssl/certs/, which is 35035bbf (send-email: be explicit with SSL certificate verification, 2013-07-18), primarily wanted to avoid letting Net::SMTP::SSL defaulting to no verification and causing it to emit warnings of the use of the insecure default. And I think the same outcome will result with your patch. By default, we still insist on using SSL_VERIFY_PEER(), not SSL_VERIFY_NONE(), which would avoid defaulting to insecure communication and triggering the warning. The way to disable the security by setting ssl_cert_path to an empty string is unchanged. Ram (who did 35035bbf), with the patch from Ruben in the thread, do you get either the warning or SSL failure? Conceptually, the resulting code is much better, I think, without blindly defaulting /etc/ssl/certs and instead of relying on the underlying platform just working out of the box with its own default, and I would be happy if we can apply the change without regression to existing users. -- To unsubscribe from this list: send the line 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: revision: propagate flag bits from tags to pointees
Jeff King p...@peff.net writes:
Looks good to me. As per my previous mail, I _think_ you could squash
in:
diff --git a/revision.c b/revision.c
index f786b51..2db906c 100644
--- a/revision.c
+++ b/revision.c
@@ -316,13 +316,10 @@ static struct commit *handle_commit(struct rev_info
*revs,
* Blob object? You know the drill by now..
*/
if (object-type == OBJ_BLOB) {
- struct blob *blob = (struct blob *)object;
if (!revs-blob_objects)
return NULL;
- if (flags UNINTERESTING) {
- mark_blob_uninteresting(blob);
+ if (flags UNINTERESTING)
return NULL;
- }
add_pending_object(revs, object, );
return NULL;
}
but that is not very much code reduction (and mark_blob_uninteresting is
very cheap). So it may not be worth the risk that my analysis is wrong.
:)
Your analysis is correct, but I think the pros-and-cons of the your
squashable change boils down to the choice between:
- leaving it in will keep similarity between tree and blob
codepaths (both have mark_X_uninteresting(); and
- reducing cycles by taking advantage of the explicit knowledge
that mark_X_uninteresting() recurses for a tree while it does not
for a blob.
But I have a suspicion that my patch may break if any codepath looks
at the current flag on the object and decides ah, it already is
marked and punts.
It indeed looks like mark_tree_uninteresting() does have that
property. When an uninteresting tag directly points at a tree, if
we propagate the UNINTERESTING bit to the pointee while peeling,
wouldn't we end up calling mark_tree_uninteresting() on a tree,
whose flags already have UNINTERESTING bit set, causing it not to
recurse?
--
To unsubscribe from this list: send the line 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: revision: propagate flag bits from tags to pointees
Junio C Hamano gits...@pobox.com writes:
But I have a suspicion that my patch may break if any codepath looks
at the current flag on the object and decides ah, it already is
marked and punts.
It indeed looks like mark_tree_uninteresting() does have that
property. When an uninteresting tag directly points at a tree, if
we propagate the UNINTERESTING bit to the pointee while peeling,
wouldn't we end up calling mark_tree_uninteresting() on a tree,
whose flags already have UNINTERESTING bit set, causing it not to
recurse?
Extending that line of thought further, what should this do?
git rev-list --objects ^HEAD^^{tree} HEAD^{tree} |
git pack-object --stdin pack
It says I am interested in the objects that is used in the tree of
HEAD, but I do not need those that already appear in HEAD^.
With the current code (with or without the fix under discussion, or
even without the faulty do not peel tags used in range notation),
the tree of the HEAD^ is marked in handle_revision_arg() as
UNINTERESTING when it is placed in revs-pending.objects[], and the
handle_commit() --- we should rename it to handle_pending_object()
or something, by the way --- will call mark_tree_uninteresting() on
that tree, which then would say It is already uninteresting and
return without marking the objects common to these two trees
uninteresting, no?
I think that is a related but separate bug that dates back to
prehistoric times, and the asymmetry between handle_commit() deals
with commits and trees should have been a clear clue that tells us
something is fishy. It calls mark PARENTS uninteresting, leaving
the responsibility of marking the commit itself to the caller, but
it calls mark_tree_uninteresting() whose caller is not supposed to
mark the tree itself.
Which suggest me that a right fix for this separate bug would be to
introduce mark_tree_contents_uninteresting() or something, which has
the parallel semantics to mark_parents_uninteresting(). Then
mark_blob_uninteresting() call in the function can clearly go. Such
a change will make it clear that handle_commit() is responsible for
handling the flags for the given object, and any helper functions
called by it should not peek and stop the flag of the object itself
when deciding to recurse into the objects linked to 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
[PATCH v2 0/2] Propagating flags carefully from the command line
So this is my second try. The second one now gets rid of the call to mark_blob_uninteresting() as Peff suggested, because the first patch makes the function very well aware that it only should mark the objects that are reachable from the object, and by definition blobs do not reach anything. Junio C Hamano (2): revision: mark contents of an uninteresting tree uninteresting revision: propagate flag bits from tags to pointees revision.c | 29 + t/t6000-rev-list-misc.sh | 17 + 2 files changed, 34 insertions(+), 12 deletions(-) -- 1.8.5.3-493-gb139ac2 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 2/2] revision: propagate flag bits from tags to pointees
With the previous fix 895c5ba3 (revision: do not peel tags used in
range notation, 2013-09-19), handle_revision_arg() that processes
command line arguments for the git log family of commands no
longer directly places the object pointed by the tag in the pending
object array when it sees a tag object. We used to place pointee
there after copying the flag bits like UNINTERESTING and
SYMMETRIC_LEFT.
This change meant that any flag that is relevant to later history
traversal must now be propagated to the pointed objects (most often
these are commits) while starting the traversal, which is partly
done by handle_commit() that is called from prepare_revision_walk().
We did propagate UNINTERESTING, but did not do so for others, most
notably SYMMETRIC_LEFT. This caused git log --left-right v1.0...
(where v1.0 is a tag) to start losing the leftness from the
commit the tag points at.
Signed-off-by: Junio C Hamano gits...@pobox.com
---
revision.c | 8 ++--
t/t6000-rev-list-misc.sh | 11 +++
2 files changed, 13 insertions(+), 6 deletions(-)
diff --git a/revision.c b/revision.c
index 28449c5..aec0333 100644
--- a/revision.c
+++ b/revision.c
@@ -273,6 +273,7 @@ static struct commit *handle_commit(struct rev_info *revs,
struct object *object
return NULL;
die(bad object %s, sha1_to_hex(tag-tagged-sha1));
}
+ object-flags |= flags;
}
/*
@@ -284,7 +285,6 @@ static struct commit *handle_commit(struct rev_info *revs,
struct object *object
if (parse_commit(commit) 0)
die(unable to parse commit %s, name);
if (flags UNINTERESTING) {
- commit-object.flags |= UNINTERESTING;
mark_parents_uninteresting(commit);
revs-limited = 1;
}
@@ -302,7 +302,6 @@ static struct commit *handle_commit(struct rev_info *revs,
struct object *object
if (!revs-tree_objects)
return NULL;
if (flags UNINTERESTING) {
- tree-object.flags |= UNINTERESTING;
mark_tree_contents_uninteresting(tree);
return NULL;
}
@@ -314,13 +313,10 @@ static struct commit *handle_commit(struct rev_info
*revs, struct object *object
* Blob object? You know the drill by now..
*/
if (object-type == OBJ_BLOB) {
- struct blob *blob = (struct blob *)object;
if (!revs-blob_objects)
return NULL;
- if (flags UNINTERESTING) {
- blob-object.flags |= UNINTERESTING;
+ if (flags UNINTERESTING)
return NULL;
- }
add_pending_object(revs, object, );
return NULL;
}
diff --git a/t/t6000-rev-list-misc.sh b/t/t6000-rev-list-misc.sh
index 9ad4971..3794e4c 100755
--- a/t/t6000-rev-list-misc.sh
+++ b/t/t6000-rev-list-misc.sh
@@ -62,4 +62,15 @@ test_expect_success 'propagate uninteresting flag down
correctly' '
test_cmp expect actual
'
+test_expect_success 'symleft flag bit is propagated down from tag' '
+ git log --format=%m %s --left-right v1.0...master actual
+ cat expect -\EOF
+two
+one
+another
+that
+ EOF
+ test_cmp expect actual
+'
+
test_done
--
1.8.5.3-493-gb139ac2
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 1/2] revision: mark contents of an uninteresting tree uninteresting
git rev-list --objects ^A^{tree} B^{tree} ought to mean I want a
list of objects inside B's tree, but please exclude the objects that
appear inside A's tree.
we see the top-level tree marked as uninteresting (i.e. ^A^{tree} in
the above example) and call mark_tree_uninteresting() on it; this
unfortunately prevents us from recursing into the tree and marking
the objects in the tree as uninteresting.
The reason why git log ^A A yields an empty set of commits,
i.e. we do not have a similar issue for commits, is because we call
mark_parents_uninteresting() after seeing an uninteresting commit.
The uninteresting-ness of the commit itself does not prevent its
parents from being marked as uninteresting.
Introduce mark_tree_contents_uninteresting() and structure the code
in handle_commit() in such a way that it makes it the responsibility
of the callchain leading to this function to mark commits, trees and
blobs as uninteresting, and also make it the responsibility of the
helpers called from this function to mark objects that are reachable
from them.
Note that this is a very old bug that probably dates back to the day
when rev-list --objects was introduced. The line to clear
tree-object.parsed at the end of mark_tree_contents_uninteresting()
can be removed when this fix is merged to the codebase after
6e454b9a (clear parsed flag when we free tree buffers, 2013-06-05).
Signed-off-by: Junio C Hamano gits...@pobox.com
---
revision.c | 25 +
t/t6000-rev-list-misc.sh | 6 ++
2 files changed, 23 insertions(+), 8 deletions(-)
diff --git a/revision.c b/revision.c
index 7010aff..28449c5 100644
--- a/revision.c
+++ b/revision.c
@@ -98,17 +98,12 @@ static void mark_blob_uninteresting(struct blob *blob)
blob-object.flags |= UNINTERESTING;
}
-void mark_tree_uninteresting(struct tree *tree)
+static void mark_tree_contents_uninteresting(struct tree *tree)
{
struct tree_desc desc;
struct name_entry entry;
struct object *obj = tree-object;
- if (!tree)
- return;
- if (obj-flags UNINTERESTING)
- return;
- obj-flags |= UNINTERESTING;
if (!has_sha1_file(obj-sha1))
return;
if (parse_tree(tree) 0)
@@ -135,6 +130,19 @@ void mark_tree_uninteresting(struct tree *tree)
*/
free(tree-buffer);
tree-buffer = NULL;
+ tree-object.parsed = 0;
+}
+
+void mark_tree_uninteresting(struct tree *tree)
+{
+ struct object *obj = tree-object;
+
+ if (!tree)
+ return;
+ if (obj-flags UNINTERESTING)
+ return;
+ obj-flags |= UNINTERESTING;
+ mark_tree_contents_uninteresting(tree);
}
void mark_parents_uninteresting(struct commit *commit)
@@ -294,7 +302,8 @@ static struct commit *handle_commit(struct rev_info *revs,
struct object *object
if (!revs-tree_objects)
return NULL;
if (flags UNINTERESTING) {
- mark_tree_uninteresting(tree);
+ tree-object.flags |= UNINTERESTING;
+ mark_tree_contents_uninteresting(tree);
return NULL;
}
add_pending_object(revs, object, );
@@ -309,7 +318,7 @@ static struct commit *handle_commit(struct rev_info *revs,
struct object *object
if (!revs-blob_objects)
return NULL;
if (flags UNINTERESTING) {
- mark_blob_uninteresting(blob);
+ blob-object.flags |= UNINTERESTING;
return NULL;
}
add_pending_object(revs, object, );
diff --git a/t/t6000-rev-list-misc.sh b/t/t6000-rev-list-misc.sh
index 15e3d64..9ad4971 100755
--- a/t/t6000-rev-list-misc.sh
+++ b/t/t6000-rev-list-misc.sh
@@ -56,4 +56,10 @@ test_expect_success 'rev-list A..B and rev-list ^A B are the
same' '
test_cmp expect actual
'
+test_expect_success 'propagate uninteresting flag down correctly' '
+ git rev-list --objects ^HEAD^{tree} HEAD^{tree} actual
+ expect
+ test_cmp expect actual
+'
+
test_done
--
1.8.5.3-493-gb139ac2
--
To unsubscribe from this list: send the line 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] Fix typesetting in Bugs section of 'git-rebase' man page (web version)
Kyle J. McKay mack...@gmail.com writes: in the docbook-xsl sources. Debian includes a patch file 8530_manpages_lists_indentation_fix.dpatch in [1]: ... And once I have applied that I suddenly get a correct git-config.1 file on System A. ... --Kyle [1] http://ftp.de.debian.org/debian/pool/main/d/docbook-xsl/docbook-xsl_1.75.2+dfsg-5.diff.gz Thanks for digging and sharing the result for others. -- To unsubscribe from this list: send the line 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 1/2] revision: mark contents of an uninteresting tree uninteresting
Jonathan Nieder jrnie...@gmail.com writes:
Junio C Hamano wrote:
we see the top-level tree marked as uninteresting (i.e. ^A^{tree} in
the above example) and call mark_tree_uninteresting() on it; this
unfortunately prevents us from recursing into the tree and marking
the objects in the tree as uninteresting.
So the tree is marked uninteresting twice --- once by setting in the
UNINTERESTING flag in handle_revision_arg() and a second attempted
time in mark_tree_uninteresting()? Makes sense.
It is that the original code, the setting of the mark on the object
itself was inconsistent. For commits, we did that ourselves; for
trees, we let the mark_tree_uninteresting() do so.
And mark_tree_uninteresting() has this quirk that it refuses to
recurse into the given tree, if the tree is already marked as
uninteresting by the caller.
We did not have the same problem on commits, because we make a
similar call to mark-parents-uninteresting but the function does not
care if the commit itself is already marked as uninteresting.
The distinction does not matter when tags are not involved. But
once we start propagating the flags from a tag to objects that the
tag points at, it starts to matter. The caller will mark the object
uninteresting in the loop that peels the tag, and the resulting
object is uninteresting. It is not a problem for commits. It was a
problem for trees, which used mark_tree_uninteresting() to mark all
the objects inside the tree uninteresting, including the tree itself.
--
To unsubscribe from this list: send the line 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 v4 1/6] submodule: Make 'checkout' update_module explicit
W. Trevor King wk...@tremily.us writes: This avoids the current awkwardness of having either '' or 'checkout' for checkout-mode updates, which makes testing for checkout-mode updates (or non-checkout-mode updates) easier. Signed-off-by: W. Trevor King wk...@tremily.us --- git-submodule.sh | 27 +++ 1 file changed, 11 insertions(+), 16 deletions(-) diff --git a/git-submodule.sh b/git-submodule.sh index 5247f78..5e8776c 100755 --- a/git-submodule.sh +++ b/git-submodule.sh @@ -803,17 +803,10 @@ cmd_update() update_module=$update else update_module=$(git config submodule.$name.update) - case $update_module in - '') - ;; # Unset update mode - checkout | rebase | merge | none) - ;; # Known update modes - !*) - ;; # Custom update command - *) - die $(eval_gettext Invalid update mode '$update_module' for submodule '$name') - ;; - esac + if test -z $update_module + then + update_module=checkout + fi fi Is this a good change? It removes the code to prevent a broken configuration value from slipping through. The code used to stop early to give the user a chance to fix it before actually letting submodule update to go into the time consuming part, e.g. a call to module_clone, but that code is now lost. displaypath=$(relative_path $prefix$sm_path) @@ -882,11 +875,16 @@ Maybe you want to use 'update --init'?) case ;$cloned_modules; in *;$name;*) # then there is no local change to integrate - update_module= ;; + update_module=checkout ;; esac must_die_on_failure= case $update_module in + checkout) + command=git checkout $subforce -q + die_msg=$(eval_gettext Unable to checkout '\$sha1' in submodule path '\$displaypath') + say_msg=$(eval_gettext Submodule path '\$displaypath': checked out '\$sha1') + ;; ... *) - command=git checkout $subforce -q - die_msg=$(eval_gettext Unable to checkout '\$sha1' in submodule path '\$displaypath') - say_msg=$(eval_gettext Submodule path '\$displaypath': checked out '\$sha1') - ;; + die $(eval_gettext Invalid update mode '$update_module' for submodule '$name') esac These two hunks make sense. It is clear in the updated code that checkout is what is implemented with that git checkout $subforce -q, and not any other random update mode. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 3/6] submodule: Explicit local branch creation in module_clone
W. Trevor King wk...@tremily.us writes:
The previous code only checked out branches in cmd_add. This commit
moves the branch-checkout logic into module_clone, where it can be
shared by cmd_add and cmd_update. I also update the initial checkout
command to use 'reset' to preserve branches setup during module_clone.
With this change, folks cloning submodules for the first time via:
$ git submodule update ...
will get a local branch instead of a detached HEAD, unless they are
using the default checkout-mode updates. This is a change from the
previous situation where cmd_update always used checkout-mode logic
(regardless of the requested update mode) for updates that triggered
an initial clone, which always resulted in a detached HEAD.
This commit does not change the logic for updates after the initial
clone, which will continue to create detached HEADs for checkout-mode
updates, and integrate remote work with the local HEAD (detached or
not) in other modes.
The motivation for the change is that developers doing local work
inside the submodule are likely to select a non-checkout-mode for
updates so their local work is integrated with upstream work.
Developers who are not doing local submodule work stick with
checkout-mode updates so any apparently local work is blown away
during updates. For example, if upstream rolls back the remote branch
or gitlinked commit to an earlier version, the checkout-mode developer
wants their old submodule checkout to be rolled back as well, instead
of getting a no-op merge/rebase with the rolled-back reference.
By using the update mode to distinguish submodule developers from
black-box submodule consumers, we can setup local branches for the
developers who will want local branches, and stick with detached HEADs
for the developers that don't care.
Signed-off-by: W. Trevor King wk...@tremily.us
---
git-submodule.sh | 53 -
1 file changed, 36 insertions(+), 17 deletions(-)
diff --git a/git-submodule.sh b/git-submodule.sh
index 68dcbe1..4a09951 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -246,6 +246,9 @@ module_name()
# $3 = URL to clone
# $4 = reference repository to reuse (empty for independent)
# $5 = depth argument for shallow clones (empty for deep)
+# $6 = (remote-tracking) starting point for the local branch (empty for HEAD)
+# $7 = local branch to create (empty for a detached HEAD, unless $6 is
+# also empty, in which case the local branch is left unchanged)
#
# Prior to calling, cmd_update checks that a possibly existing
# path is not a git repository.
@@ -259,6 +262,8 @@ module_clone()
url=$3
reference=$4
depth=$5
+ start_point=$6
+ local_branch=$7
quiet=
if test -n $GIT_QUIET
then
@@ -312,7 +317,16 @@ module_clone()
echo gitdir: $rel/$a $sm_path/.git
rel=$(echo $a | sed -e 's|[^/][^/]*|..|g')
- (clear_local_git_env; cd $sm_path GIT_WORK_TREE=. git config
core.worktree $rel/$b)
+ (
+ clear_local_git_env
+ cd $sm_path
+ GIT_WORK_TREE=. git config core.worktree $rel/$b
+ # ash fails to wordsplit ${local_branch:+-B $local_branch...}
Interesting...
+ case $local_branch in
+ '') git checkout -f -q ${start_point:+$start_point} ;;
+ ?*) git checkout -f -q -B $local_branch
${start_point:+$start_point} ;;
+ esac
I am wondering if it makes more sense if you did this instead:
git checkout -f -q ${start_point:+$start_point}
if test -n $local_branch
then
git checkout -B $local_branch HEAD
fi
The optional re-attaching to the local_branch done with the second
checkout would be a non-destructive no-op to the working tree and
to the index, but it does distinguish between creating the branch
anew and resetting the existing branch in its output (note that
there is no -q to squelch it). By doing it this way, when we
later teach git branch -f and git checkout -B to report more
about what commits have been lost by such a resetting, you will get
the safety for free if you made the switching with -B run without
-q here.
+ ) || die $(eval_gettext Unable to setup cloned submodule
'\$sm_path')
}
isnumber()
@@ -475,16 +489,14 @@ Use -f if you really want to add it. 2
echo $(eval_gettext Reactivating local git
directory for submodule '\$sm_name'.)
fi
fi
- module_clone $sm_path $sm_name $realrepo $reference
$depth || exit
- (
- clear_local_git_env
- cd $sm_path
- # ash fails to wordsplit ${branch:+-b $branch...}
- case $branch in
- '') git checkout -f -q ;;
- ?*) git checkout -f -q -B
Re: [PATCH v4 4/6] t7406: Just-cloned checkouts update to the gitlinked hash with 'reset'
W. Trevor King wk...@tremily.us writes: To preserve the local branch, for situations where we're not on a detached HEAD. Signed-off-by: W. Trevor King wk...@tremily.us --- This should be a part of some other change that actually changes how this git submodule update checks out the submodule, no? t/t7406-submodule-update.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/t/t7406-submodule-update.sh b/t/t7406-submodule-update.sh index 0825a92..5aa9591 100755 --- a/t/t7406-submodule-update.sh +++ b/t/t7406-submodule-update.sh @@ -703,7 +703,7 @@ test_expect_success 'submodule update places git-dir in superprojects git-dir re git clone super_update_r super_update_r2 (cd super_update_r2 git submodule update --init --recursive actual - test_i18ngrep Submodule path .submodule/subsubmodule.: checked out actual + test_i18ngrep Submodule path .submodule/subsubmodule.: .git reset --hard -q actual (cd submodule/subsubmodule git log ../../expected ) -- To unsubscribe from this list: send the line 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 v4 3/6] submodule: Explicit local branch creation in module_clone
W. Trevor King wk...@tremily.us writes: @@ -817,11 +831,15 @@ cmd_update() displaypath=$(relative_path $prefix$sm_path) - if test $update_module = none - then + case $update_module in + none) echo Skipping submodule '$displaypath' continue - fi + ;; + checkout) + local_branch= + ;; + esac I wonder if there is a way to avoid detaching (and you may need to update the coddpath that resets the submodule to the commit specified by the superproject tree) when it is safe to do so. For an end user, running submodule update is similar to running git pull in a project that does not use submodules, expressing I want to catch up with the work done by others. In a working tree with local changes, we do allow you to run git pull as long as your local changes do not overlap with the work done by others, and the result of the pull would look as if you did not have any of the local changes when you ran git pull and then you did the local changes on top of the state that is up-to-date with their work. Can't we design submodule update --checkout to work in a similar fashion? The updated superproject may say it wants $oSHA-1 at a submodule path P, and also its .gitmodules may say that commit is supposed to be at the tip of branch B=submodule.P.branch in the submodule repository. You may locally have advanced that branch in your submodule repository in the meantime to point at $ySHA-1 while others worked in the superproject and the submodule, and the difference $oSHA-1...$ySHA-1 can be considered as the local change made by you from the perspective of the superproject. Without thinking things through, if $ySHA-1 matches or is a descendant of $oSHA-1 (assuming that remote-tracking branch origin/$B in the submodule does point at $oSHA-1 in either case), it should be safe to do this update. And in a situation where you cannot do the checkout safely, it is perfectly fine to say the submodules X and Y have local changes; you cannot do 'submodule update' until you upstream them and fail the update, just like we fail a 'git pull' saying you cannot do pull until you commit them, no? Perhaps that kind of 'git submodule update' is parallel to 'git pull' in the project without submodules is better done with other update modes like --rebase or --merge. If so, how should we explain what 'submodule update --checkout' is to the end users? Is it supposed to be like git fetch git checkout origin/master? -- To unsubscribe from this list: send the line 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 v4 6/6] Documentation: Describe 'submodule update' modes in detail
W. Trevor King wk...@tremily.us writes: The old documentation did not distinguish between cloning and non-cloning updates and lacked clarity on which operations would lead to detached HEADs, and which would not. The new documentation addresses these issues while updating the docs to reflect the changes introduced by this branch's explicit local branch creation in module_clone. I also add '--checkout' to the usage summary and group the update-mode options into a single set. Signed-off-by: W. Trevor King wk...@tremily.us --- Documentation/git-submodule.txt | 36 +++- Documentation/gitmodules.txt| 4 2 files changed, 31 insertions(+), 9 deletions(-) diff --git a/Documentation/git-submodule.txt b/Documentation/git-submodule.txt index bfef8a0..02500b4 100644 --- a/Documentation/git-submodule.txt +++ b/Documentation/git-submodule.txt @@ -15,8 +15,8 @@ SYNOPSIS 'git submodule' [--quiet] init [--] [path...] 'git submodule' [--quiet] deinit [-f|--force] [--] path... 'git submodule' [--quiet] update [--init] [--remote] [-N|--no-fetch] - [-f|--force] [--rebase] [--reference repository] [--depth depth] - [--merge] [--recursive] [--] [path...] + [-f|--force] [--rebase|--merge|--checkout] [--reference repository] + [--depth depth] [--recursive] [--] [path...] 'git submodule' [--quiet] summary [--cached|--files] [(-n|--summary-limit) n] [commit] [--] [path...] 'git submodule' [--quiet] foreach [--recursive] command @@ -155,13 +155,31 @@ it contains local modifications. update:: Update the registered submodules, i.e. clone missing submodules and - checkout the commit specified in the index of the containing repository. - This will make the submodules HEAD be detached unless `--rebase` or - `--merge` is specified or the key `submodule.$name.update` is set to - `rebase`, `merge` or `none`. `none` can be overridden by specifying - `--checkout`. Setting the key `submodule.$name.update` to `!command` - will cause `command` to be run. `command` can be any arbitrary shell - command that takes a single argument, namely the sha1 to update to. + checkout the commit specified in the index of the containing + repository. The update mode defaults to 'checkout', but be + configured with the 'submodule.name.update' setting or the + '--rebase', '--merge', or 'checkout' options. Not '--checkout'? Other than that, the updated text above is far easier to understand. Good job. ++ +For updates that clone missing submodules, checkout-mode updates will +create submodules with detached HEADs; all other modes will create +submodules with a local branch named after 'submodule.path.branch'. ++ +For updates that do not clone missing submodules, the submodule's HEAD That is, updates that update submodules that are already checked out? +is only touched when the remote reference does not match the +submodule's HEAD (for none-mode updates, the submodule is never +touched). The remote reference is usually the gitlinked commit from +the superproject's tree, but with '--remote' it is the upstream +subproject's 'submodule.name.branch'. This remote reference is +integrated with the submodule's HEAD using the specified update mode. I think copying some motivation from the log message of 06b1abb5 (submodule update: add --remote for submodule's upstream changes, 2012-12-19) would help the readers here. A naïve expectation from a casual reader of the above would be The superproject's tree ought to point at the same commit as the tip of the branch used in the submodule (modulo mirroring delays and somesuch), if the repository of the superproject and submodules are maintained properly, which would lead to when would any sane person need to use --remote in the first place???. If I am reading 06b1abb5 correctly, the primary motivation behind --remote seems to be that it is exactly to help the person who wants to update superproject to satisify the ... are maintained properly part by fetching the latest in each of the submodules in his superproject in preparation to 'git add .' them. I still do not think --remote was a better way than the foreach, but that is a separate topic. If the person who works in the superproject does not control the progress of, and/or does not care what development is happening in, the submodules, he can push the superproject tree out without even bothering to update the commits in the submodules bound to his superproject tree, and the consumers of such a superproject could catch up with the advancement of submodule by using --remote individually to bring themselves up to date. But I do not think that is what you envisioned as the best recommended practice when you wrote 06b1abb5. +For checkout-mode updates, that will result in a detached HEAD. For +rebase- and merge-mode updates, the commit referenced by
Re: [PATCH v4 4/6] t7406: Just-cloned checkouts update to the gitlinked hash with 'reset'
W. Trevor King wk...@tremily.us writes: On Thu, Jan 16, 2014 at 11:22:52AM -0800, Junio C Hamano wrote: W. Trevor King wk...@tremily.us writes: To preserve the local branch, for situations where we're not on a detached HEAD. Signed-off-by: W. Trevor King wk...@tremily.us --- This should be a part of some other change that actually changes how this git submodule update checks out the submodule, no? Sure, we can squash both this test fix and the subsequent new test patch into patch #3 in v5. I was just splitting them out because backwards compatibility was a concern, and separate patches makes it easy for me to explain why the results changed here without getting lost in patch #3's implementation details. On the contrary, if we had this as part of patch #3, it would have helped reviewing the patch itself, as that would have served as one more clue to illustrate the effect that is externally visible to end users. Besides, having them separate will break bisection. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
What's cooking in git.git (Jan 2014, #03; Thu, 16)
Here are the topics that have been cooking. Commits prefixed with
'-' are only in 'pu' (proposed updates) while commits prefixed with
'+' are in 'next'.
I am planning to tag 1.9-rc0 preview release from the tip of
'master' soonish. Hopefully a few fix-up topics still in flight and
also updates to git-gui, gitk, git-svn, i18n, etc. from respective
area maintainers can be merged by the time 1.9-rc1 will be tagged.
You can find the changes described here in the integration branches
of the repositories listed at
http://git-blame.blogspot.com/p/git-public-repositories.html
--
[Graduated to master]
* br/sha1-name-40-hex-no-disambiguation (2014-01-07) 1 commit
(merged to 'next' on 2014-01-10 at 2a0973b)
+ sha1_name: don't resolve refs when core.warnambiguousrefs is false
(this branch is used by jk/warn-on-object-refname-ambiguity.)
When parsing a 40-hex string into the object name, the string is
checked to see if it can be interpreted as a ref so that a warning
can be given for ambiguity. The code kicked in even when the
core.warnambiguousrefs is set to false to squelch this warning, in
which case the cycles spent to look at the ref namespace were an
expensive no-op, as the result was discarded without being used.
* jk/pull-rebase-using-fork-point (2014-01-09) 1 commit
(merged to 'next' on 2014-01-10 at e86e59d)
+ rebase: fix fork-point with zero arguments
* jl/submodule-mv-checkout-caveat (2014-01-07) 2 commits
(merged to 'next' on 2014-01-10 at 8d3953c)
+ rm: better document side effects when removing a submodule
+ mv: better document side effects when moving a submodule
With a submodule that was initialized in an old fashioned way
without gitlinks, switching branches in the superproject between
the one with and without the submodule may leave the submodule
working tree with its embedded repository behind, as there may be
unexpendable state there. Document and warn users about this.
* jn/pager-lv-default-env (2014-01-07) 1 commit
(merged to 'next' on 2014-01-10 at 22d4755)
+ pager: set LV=-c alongside LESS=FRSX
Just like we give a reasonable default for less via the LESS
environment variable, specify a reasonable default for lv via the
LV environment variable when spawning the pager.
* mh/shorten-unambigous-ref (2014-01-09) 3 commits
+ shorten_unambiguous_ref(): tighten up pointer arithmetic
+ gen_scanf_fmt(): delete function and use snprintf() instead
+ shorten_unambiguous_ref(): introduce a new local variable
* mm/mv-file-to-no-such-dir-with-slash (2014-01-10) 1 commit
+ mv: let 'git mv file no-such-dir/' error out on Windows, too
Finishing touches to a topic that has already been merged to 'master'.
* ow/stash-with-ifs (2014-01-07) 1 commit
(merged to 'next' on 2014-01-10 at 4fc9bdb)
+ stash: handle specifying stashes with $IFS
The implementation of 'git stash $cmd stash@{...}' did not quote
the stash argument properly and left it split at IFS whitespace.
* rr/completion-format-coverletter (2014-01-07) 1 commit
(merged to 'next' on 2014-01-10 at d2dbc9d)
+ completion: complete format.coverLetter
The bash/zsh completion code did not know about format.coverLetter
among many format.* configuration variables.
--
[New Topics]
* ab/subtree-doc (2014-01-13) 1 commit
- subtree: fix argument validation in add/pull/push
Will merge to 'next'.
* jk/complete-merge-base (2014-01-13) 2 commits
- completion: handle --[no-]fork-point options to git-rebase
- completion: complete merge-base options
Will merge to 'next'.
* po/everyday-doc (2014-01-13) 1 commit
- Make 'git help everyday' work
* rb/merge-prepare-commit-msg-hook (2014-01-10) 4 commits
- merge: drop unused arg from abort_commit method signature
- merge: make prepare_to_commit responsible for write_merge_state
- t7505: ensure cleanup after hook blocks merge
- t7505: add missing
Expose more merge states (e.g. $GIT_DIR/MERGE_MODE) to hooks that
run during git merge. The log stressed too much on one hook,
prepare-commit-msg, but it would equally apply to other hooks like
post-merge, I think.
Waiting to give a chance to reroll.
* bl/blame-full-history (2014-01-14) 1 commit
- blame: new option --prefer-first to better handle merged cherry-picks
* da/pull-ff-configuration (2014-01-15) 2 commits
- pull: add --ff-only to the help text
- pull: add pull.ff configuration
Will merge to 'next'.
* jc/maint-pull-docfix (2014-01-14) 2 commits
- Documentation: git pull does not have the -m option
- Merge branch 'jc/maint-pull-docfix-for-409b8d82' into jc/maint-pull-docfix
(this branch uses jc/maint-pull-docfix-for-409b8d82.)
Will merge to 'next'.
* jc/maint-pull-docfix-for-409b8d82 (2014-01-14) 1 commit
- Documentation: exclude irrelevant options from git pull
(this branch is used by jc/maint-pull-docfix.)
Will merge to 'next'.
* jc/revision-range-unpeel
Re: [PATCH v4 6/6] Documentation: Describe 'submodule update' modes in detail
W. Trevor King wk...@tremily.us writes: A naïve expectation from a casual reader of the above would be The superproject's tree ought to point at the same commit as the tip of the branch used in the submodule (modulo mirroring delays and somesuch), What is the branch used in the submodule? The remote subproject's current submodule.name.branch? The local submodule's submodule.name.branch (or localBranch) branch? The submodule's current HEAD? They are good questions that such casual readers would have, and giving answers to them in this part of the documentation would be a good way to give them a clear picture of how the command is designed to be used. -- To unsubscribe from this list: send the line 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] .gitignore: Ignore editor backup and swap files
Alexander Berntsen alexan...@plaimi.net writes: Signed-off-by: Alexander Berntsen alexan...@plaimi.net --- .gitignore | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.gitignore b/.gitignore index b5f9def..2905c21 100644 --- a/.gitignore +++ b/.gitignore @@ -240,3 +240,5 @@ *.pdb /Debug/ /Release/ +*~ +.*.swp I personally do not mind listing these common ones too much, but if I am not mistaken, our policy on this file so far has been that it lists build artifacts, and not personal preference (the *.swp entry is useless for those who never use vim, for example). These paths that depend on your choice of the editor and other tools can still be managed in your personal .git/info/exclude in the meantime. -- To unsubscribe from this list: send the line 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] send-email: If the ca path is not specified, use the defaults
Jonathan Nieder jrnie...@gmail.com writes:
FWIW this should help on Mac OS X, too. Folks using git on mac
at $DAYJOB have been using the workaround described at
http://mercurial.selenic.com/wiki/CACertificates#Mac_OS_X_10.6_and_higher
so I forgot to report it. :/
Hmph, is that the same issue, though? That page seems to suggest
using an empty ca file that does not have any useful information as
a workaround. The issue Fedora folks saw is that we see a directory
/etc/ssl/certs exist on the system, and blindly attempt to use it as
SSL_ca_path when the directory is not suitable to be used as such.
In any case, I tried to summarize the discussion in the updated log
message. I wanted to say does not but stopped at should not in
the last paragraph for now. Maybe Ram can say something before we
merge it to 'next'.
The patch in the meantime will be queued on 'pu'.
-- 8 --
From: Ruben Kerkhof ru...@rubenkerkhof.com
Date: Wed, 15 Jan 2014 21:31:11 +0400
Subject: [PATCH] send-email: /etc/ssl/certs/ directory may not be usable as
ca_path
When sending patches on Fedora rawhide with
git-1.8.5.2-1.fc21.x86_64 and perl-IO-Socket-SSL-1.962-1.fc21.noarch,
with the following
[sendemail]
smtpencryption = tls
smtpserver = smtp.gmail.com
smtpuser = ru...@rubenkerkhof.com
smtpserverport = 587
git-send-email fails with:
STARTTLS failed! SSL connect attempt failed with unknown error
error:14090086:SSL routines:SSL3_GET_SERVER_CERTIFICATE:certificate
verify failed at /usr/libexec/git-core/git-send-email line 1236.
The current code detects the presence of /etc/ssl/certs directory
(it actually is a symlink to another directory, but that does not
matter) and uses SSL_ca_path to point at it when initializing the
connection with IO::Socket::SSL or Net::SMTP::SSL. However, on the
said platform, it seems that this directory is not designed to be
used as SSL_ca_path. Using a single file inside that directory
(cert.pem, which is a Mozilla CA bundle) with SSL_ca_file does work,
and also not specifying any SSL_ca_file/SSL_ca_path (and letting the
library use its own default) and asking for peer verification does
work.
By removing the code that blindly defaults $smtp_ssl_cert_path to
/etc/ssl/certs, we can prevent the codepath that treats any
directory specified with that variable as usable for SSL_ca_path
from incorrectly triggering.
This change could introduce a regression for people on a platform
whose certificate directory is /etc/ssl/certs but its IO::Socket:SSL
somehow fails to use it as SSL_ca_path without being told. Using
/etc/ssl/certs directory as SSL_ca_path by default like the current
code does would have been hiding such a broken installation without
its user needing to do anything. These users can still work around
such a platform bug by setting the configuration variable explicitly
to point at /etc/ssl/certs.
This change should not negate what 35035bbf (send-email: be explicit
with SSL certificate verification, 2013-07-18), which was the
original change that introduced the defaulting to /etc/ssl/certs/,
attempted to do, which is to make sure we do not communicate over
insecure connection by default, triggering warning from the library.
Cf. https://bugzilla.redhat.com/show_bug.cgi?id=1043194
Tested-by: Igor Gnatenko i.gnatenko.br...@gmail.com
Signed-off-by: Ruben Kerkhof ru...@rubenkerkhof.com
Signed-off-by: Junio C Hamano gits...@pobox.com
---
git-send-email.perl | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/git-send-email.perl b/git-send-email.perl
index 3782c3b..689944f 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -1095,7 +1095,8 @@ sub ssl_verify_params {
}
if (!defined $smtp_ssl_cert_path) {
- $smtp_ssl_cert_path = /etc/ssl/certs;
+ # use the OpenSSL defaults
+ return (SSL_verify_mode = SSL_VERIFY_PEER());
}
if ($smtp_ssl_cert_path eq ) {
--
1.8.5.3-493-gb139ac2
--
To unsubscribe from this list: send the line 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] send-email: If the ca path is not specified, use the defaults
Kyle J. McKay mack...@gmail.com writes: On my OS X platform depending on which version of OpenSSL I'm using, the OPENSSLDIR path would be one of these: /System/Library/OpenSSL /opt/local/etc/openssl And neither of those uses a certs directory, they both use a cert.pem bundle instead: $ ls -l /System/Library/OpenSSL total 32 lrwxrwxrwx 1 root wheel42 cert.pem - ../../../usr/share/curl/ curl-ca-bundle.crt drwxr-xr-x 2 root wheel68 certs drwxr-xr-x 8 root wheel 272 misc -rw-r--r-- 1 root wheel 9381 openssl.cnf drwxr-xr-x 2 root wheel68 private # the certs directory is empty $ ls -l /opt/local/etc/openssl total 32 lrwxrwxrwx 1 root admin35 cert.pem@ - ../../share/curl/curl- ca-bundle.crt drwxr-xr-x 9 root admin 306 misc/ -rw-r--r-- 1 root admin 10835 openssl.cnf Notice neither of those refers to /etc/ssl/certs at all. So the short answer is, yes, hard-coding /etc/ssl/certs as the path on OS X is incorrect and if setting /etc/ssl/certs as the path has the effect of replacing the default locations the verification will fail. The current code says if nothing is specified, let's pretend /etc/ssl/certs was specified. Then if it is a directory, use it with SSL_ca_path, if it is a file, use it with SSL_ca_file, if it does not exist, do not even attempt verification. And that let's pretend breaks Fedora, where /etc/ssl/certs is a directory but is not meant to be used with SSL_ca_path---we try to use /etc/ssl/certs with SSL_ca_path and verification fails miserably. If I am reading the code correctly, if /etc/ssl/certs does not exist on the filesystem at all, it wouldn't even attempt verification, so I take your the verification will fail to mean that you forgot to also mention And on OS X, /etc/ssl/certs directory still exists, even though OpenSSL does not use it. If that is the case, then our current code indeed is broken in exactly the same way for OS X as for Fedora. The proposed change in this thread would stop the defaulting altogether, and still ask verification to the library using its own default, so I can see how that would make the setting you described used on OS X work properly. In short, I agree with you on both counts (the current code is wrong for OS X, and the proposed change will fix it). I just want to make sure that my understanding of the current breakage is in line with the reality ;-) 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 1/5] diff_filespec: reorder dirty_submodule macro definitions
Jeff King p...@peff.net writes:
diff_filespec has a 2-bit dirty_submodule field and
defines two flags as macros. Originally these were right
next to each other, but a new field was accidentally added
in between in commit 4682d85.
Interesting.
- 4682d852 (diff-index.c: git diff has no need to read blob from
the standard input, 2012-06-27) wants to use this rule: all the
bitfield definitions first, and then whatever macro constants
next.
- 25e5e2bf (combine-diff: support format_callback, 2011-08-19),
wants to use a different rule: a run of (one bitfield definition
and zero-or-more macro constants to be used in that bitfield).
When they were merged together at d7afe648 (Merge branch
'jc/refactor-diff-stdin', 2012-07-13), these two conflicting
philosophies crashed.
That is the commit to be blamed for this mess ;-)
I am of course fine with the end result this patch gives us.
Thanks.
This patch puts the field and
its flags back together.
Using an enum like:
enum {
DIRTY_SUBMODULE_UNTRACKED = 1,
DIRTY_SUBMODULE_MODIFIED = 2
} dirty_submodule;
would be more obvious, but it bloats the structure. Limiting
the enum size like:
} dirty_submodule : 2;
might work, but it is not portable.
Signed-off-by: Jeff King p...@peff.net
---
diffcore.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/diffcore.h b/diffcore.h
index 1c16c85..f822f9e 100644
--- a/diffcore.h
+++ b/diffcore.h
@@ -43,9 +43,9 @@ struct diff_filespec {
unsigned should_free : 1; /* data should be free()'ed */
unsigned should_munmap : 1; /* data should be munmap()'ed */
unsigned dirty_submodule : 2; /* For submodules: its work tree is
dirty */
- unsigned is_stdin : 1;
#define DIRTY_SUBMODULE_UNTRACKED 1
#define DIRTY_SUBMODULE_MODIFIED 2
+ unsigned is_stdin : 1;
unsigned has_more_entries : 1; /* only appear in combined diff */
struct userdiff_driver *driver;
/* data should be considered binary; -1 means don't know yet */
--
To unsubscribe from this list: send the line 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 0/5] diff_filespec cleanups and optimizations
Jeff King p...@peff.net writes: But while looking at it, I noticed a bunch of cleanups for diff_filespec. With the patches below, sizeof(struct diff_filespec) on my 64-bit machine goes from 96 bytes down to 80. Compiling with -m32 goes from 64 bytes down to 52. The first few patches have cleanup value aside from the struct size improvement. The last two are pure optimization. I doubt the optimization is noticeable for any real-life cases, so I don't mind if they get dropped. But they're quite trivial and obvious. Thanks for a pleasant read. -- To unsubscribe from this list: send the line 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: with git-next, git pull --rebase can print out fatal: No such ref: '' for branches without an upstream
Jeff King p...@peff.net writes:
On Thu, Jan 16, 2014 at 05:08:14PM -0800, Siddharth Agarwal wrote:
With git-next, where git pull --rebase can print out fatal: No such
ref: '' if git pull --rebase is run on branches without an upstream.
This is already fixed in bb3f458 (rebase: fix fork-point with zero
arguments, 2014-01-09), I think.
Doesn't the call to get_remote_merge_branch in this part
test -n $curr_branch
. git-parse-remote
remoteref=$(get_remote_merge_branch $@ 2/dev/null)
oldremoteref=$(git merge-base --fork-point $remoteref $curr_branch)
yield an empty string, feeding it to merge-base --fork-point as
its first parameter?
Perhaps something like this is needed?
git-pull.sh | 1 +
1 file changed, 1 insertion(+)
diff --git a/git-pull.sh b/git-pull.sh
index 605e957..467c66c 100755
--- a/git-pull.sh
+++ b/git-pull.sh
@@ -229,6 +229,7 @@ test true = $rebase {
test -n $curr_branch
. git-parse-remote
remoteref=$(get_remote_merge_branch $@ 2/dev/null)
+ test -n $remoteref
oldremoteref=$(git merge-base --fork-point $remoteref $curr_branch)
}
orig_head=$(git rev-parse -q --verify HEAD)
--
To unsubscribe from this list: send the line 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] prefer xwrite instead of write
Jonathan Nieder jrnie...@gmail.com writes:
Hi,
Erik Faye-Lund wrote:
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -367,7 +367,7 @@ static void squash_message(struct commit *commit, struct
commit_list *remotehead
sha1_to_hex(commit-object.sha1));
pretty_print_commit(ctx, commit, out);
}
-if (write(fd, out.buf, out.len) 0)
+if (xwrite(fd, out.buf, out.len) 0)
die_errno(_(Writing SQUASH_MSG));
Shouldn't this use write_in_full() to avoid a silently truncated result? (*)
Meaning this? If so, I think it makes sense.
builtin/merge.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/builtin/merge.c b/builtin/merge.c
index 6e108d2..a6a38ee 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -367,7 +367,7 @@ static void squash_message(struct commit *commit, struct
commit_list *remotehead
sha1_to_hex(commit-object.sha1));
pretty_print_commit(ctx, commit, out);
}
- if (xwrite(fd, out.buf, out.len) 0)
+ if (write_in_full(fd, out.buf, out.len) != out.len)
die_errno(_(Writing SQUASH_MSG));
if (close(fd))
die_errno(_(Finishing SQUASH_MSG));
[...]
--- a/streaming.c
+++ b/streaming.c
@@ -538,7 +538,7 @@ int stream_blob_to_fd(int fd, unsigned const char *sha1,
struct stream_filter *f
goto close_and_exit;
}
if (kept (lseek(fd, kept - 1, SEEK_CUR) == (off_t) -1 ||
- write(fd, , 1) != 1))
+ xwrite(fd, , 1) != 1))
Yeah, if we get EINTR then it's worth retrying.
[...]
--- a/transport-helper.c
+++ b/transport-helper.c
@@ -1129,9 +1129,8 @@ static int udt_do_write(struct unidirectional_transfer
*t)
return 0; /* Nothing to write. */
transfer_debug(%s is writable, t-dest_name);
-bytes = write(t-dest, t-buf, t-bufuse);
-if (bytes 0 errno != EWOULDBLOCK errno != EAGAIN
-errno != EINTR) {
+bytes = xwrite(t-dest, t-buf, t-bufuse);
+if (bytes 0 errno != EWOULDBLOCK) {
Here the write is limited by BUFFERSIZE, and returning to the outer
loop to try another read when the write returns EAGAIN, like the
original code does, seems philosophically like the right thing to do.
Luckily we don't use O_NONBLOCK anywhere, so the change shouldn't
matter in practice. So although it doesn't do any good, using xwrite
here for consistency should be fine.
So my only worry is the (*) above. With that change,
Reviewed-by: Jonathan Nieder jrnie...@gmail.com
--
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/3] setup_pager: set MORE=R
Kyle J. McKay mack...@gmail.com writes: On Jan 16, 2014, at 20:21, Jeff King wrote: When we run the pager, we always set LESS=R to tell the pager to pass through ANSI colors. On modern versions of FreeBSD, the system more can do the same trick. [snip] diff --git a/pager.c b/pager.c index 90d237e..2303164 100644 --- a/pager.c +++ b/pager.c @@ -87,6 +87,10 @@ void setup_pager(void) argv_array_push(env, LESS=FRSX); if (!getenv(LV)) argv_array_push(env, LV=-c); +#ifdef PAGER_MORE_UNDERSTANDS_R +if (!getenv(MORE)) +argv_array_push(env, MORE=R); +#endif How about adding a leading - to both the LESS and MORE settings? Since you're in there patching... :) The discussion we had when LV=-c was added, namely $gmane/240124, agrees. I however am perfectly fine to see it done as a separate clean-up patch. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/3] setup_pager: set MORE=R
Jeff King p...@peff.net writes: diff --git a/pager.c b/pager.c index 90d237e..2303164 100644 --- a/pager.c +++ b/pager.c @@ -87,6 +87,10 @@ void setup_pager(void) argv_array_push(env, LESS=FRSX); if (!getenv(LV)) argv_array_push(env, LV=-c); +#ifdef PAGER_MORE_UNDERSTANDS_R + if (!getenv(MORE)) + argv_array_push(env, MORE=R); +#endif pager_process.env = argv_array_detach(env, NULL); if (start_command(pager_process)) Let me repeat from $gmane/240110: - Can we generalize this a bit so that a builder can pass a list of var=val pairs and demote the existing LESS=FRSX to just a canned setting of such a mechanism? As we need to maintain this set these environments when unset here and also in git-sh-setup.sh, I think it is about time to do that clean-up. Duplicating two settings was borderline OK, but seeing the third added fairly soon after the second was added tells me that the clean-up must come before adding the third. -- To unsubscribe from this list: send the line 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] git-svn: workaround for a bug in svn serf backend
Roman Kagan rka...@mail.ru writes: 2013/12/31 Roman Kagan rka...@mail.ru: 2013/12/30 Junio C Hamano gits...@pobox.com: Roman Kagan rka...@mail.ru writes: I'd like to note that it's IMO worth including in the 'maint' branch as it's a crasher. Especially so since the real fix has been merged in the subversion upstream and nominated for 1.8 branch, so the workaround may soon lose its relevance. I do not quite get this part, though. If they refused to fix it for real, it would make it likely that this workaround will stay relevant for a long time, in which case it would be worth cherry-picking to an older maintenance track. But if this workaround is expected to lose its relevance shortly, I see it as one less reason to cherry-pick it to an older maintenance track. Confused... I thought it was exactly the other way around. By the time the next feature release reaches users, chances are they'd already have subversion with the fix. OTOH the workaround would benefit those who get their maintenance release of git (e.g. through their Linux distro update) before they get their maintenance release of subversion. So this actually happened: 1.8.5.3 is out, and some distributions are shipping it (Arch, Debian), but the workaround didn't make it there. The way I read your message was that the fix on the subversion side is already there and this patch to work it around on our end is of no importance. But actually you wanted to say quite the opposite. They are slow and it is likely that we need to work their bug around for a while. If so, then I think it might make sense to cherry-pick it to the maint branch, even though we usually apply only fixes to our own bugs to the maintenance track. -- To unsubscribe from this list: send the line 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 quietly fails on https:// URL, https errors are never reported to user
Yuri y...@rawbw.com writes: I think that in a rare case of error this extra-printout wouldn't hurt. If the error is rare, extra verbiage does not hurt were a valid attitude, error is rare, non-zero exit is enough would be equally valid ;-) Also that statement contradicts with the rationale given by 266f1fdf (transport-helper: be quiet on read errors from helpers, 2013-06-21), no? However, this makes a much more common case worse: when a helper does die with a useful message, we print the extra Reading from 'git-remote-foo failed message. This can also end up confusing users, as they may not even know what remote helpers are (e.g., the fact that http support comes through git-remote-https is purely an implementation detail that most users do not know or care about). Your change is not an exact revert and rewords the message to read Failure in 'http' protocol reader. instead of Reading from helper 'git-remote-http' failed. which avoids the helper word and replacing it with protocol reader [*1*] in an attempt to make it less likely to end up confusing users, but I am not sure if protocol reader is good enough for those who get confused with helper in the first place. They will ask their resident guru or favourite search engine about the message and will be told that your http connection did not go well either way, but not many people have seen this new message. If we were to reinstate the extra final line in the error message, I think we would be off doing a straight revert without any rewording that introduces yet another word protocol reader that is not found anywhere in our glossary. I think I am OK with adding one more line after Reading from ... failed that explains a more detailed error message might be there above that line, but I am not sure what the good phrasing would be. [Footnote] *1* It may introduce a new confusion was it 'read' that failed, or 'write'?, though ;-) -- To unsubscribe from this list: send the line 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/3] setup_pager: set MORE=R
Junio C Hamano gits...@pobox.com writes: Jeff King p...@peff.net writes: diff --git a/pager.c b/pager.c index 90d237e..2303164 100644 --- a/pager.c +++ b/pager.c @@ -87,6 +87,10 @@ void setup_pager(void) argv_array_push(env, LESS=FRSX); if (!getenv(LV)) argv_array_push(env, LV=-c); +#ifdef PAGER_MORE_UNDERSTANDS_R +if (!getenv(MORE)) +argv_array_push(env, MORE=R); +#endif pager_process.env = argv_array_detach(env, NULL); if (start_command(pager_process)) Let me repeat from $gmane/240110: - Can we generalize this a bit so that a builder can pass a list of var=val pairs and demote the existing LESS=FRSX to just a canned setting of such a mechanism? As we need to maintain this set these environments when unset here and also in git-sh-setup.sh, I think it is about time to do that clean-up. Duplicating two settings was borderline OK, but seeing the third added fairly soon after the second was added tells me that the clean-up must come before adding the third. Perhaps we can start it like this. Then pager.c can iterate over the PAGER_ENV string, parse out LESS=, LV=, etc., and do its thing. We would also need to muck with git-sh-setup.sh but that file is already preprocessed in the Makefile, so we should be able to do something similar to # @@BROKEN_PATH_FIX@@ there. Makefile | 15 ++- config.mak.uname | 1 + 2 files changed, 15 insertions(+), 1 deletion(-) diff --git a/Makefile b/Makefile index b4af1e2..c9e7847 100644 --- a/Makefile +++ b/Makefile @@ -342,6 +342,14 @@ all:: # Define DEFAULT_HELP_FORMAT to man, info or html # (defaults to man) if you want to have a different default when # git help is called without a parameter specifying the format. +# +# Define PAGER_ENV to a SP separated VAR=VAL pairs to define +# default environment variables to be passed when a pager is spawned, e.g. +# +#PAGER_ENV = LESS=-FRSX LV=-c +# +# to say export LESS=-FRSX (and LV=-c) if the environment variable +# LESS (and LV) is not set, respectively. GIT-VERSION-FILE: FORCE @$(SHELL_PATH) ./GIT-VERSION-GEN @@ -1506,6 +1514,10 @@ ifeq ($(PYTHON_PATH),) NO_PYTHON = NoThanks endif +ifndef PAGER_ENV +PAGER_ENV = LESS=-FRSX LV=-c +endif + QUIET_SUBDIR0 = +$(MAKE) -C # space to separate -C and subdir QUIET_SUBDIR1 = @@ -1585,11 +1597,12 @@ PYTHON_PATH_SQ = $(subst ','\'',$(PYTHON_PATH)) TCLTK_PATH_SQ = $(subst ','\'',$(TCLTK_PATH)) DIFF_SQ = $(subst ','\'',$(DIFF)) PERLLIB_EXTRA_SQ = $(subst ','\'',$(PERLLIB_EXTRA)) +PAGER_ENV_SQ = $(subst ','\'',$(PAGER_ENV)) LIBS = $(GITLIBS) $(EXTLIBS) BASIC_CFLAGS += -DSHA1_HEADER='$(SHA1_HEADER_SQ)' \ - $(COMPAT_CFLAGS) + $(COMPAT_CFLAGS) -DPAGER_ENV='$(PAGER_ENV_SQ)' LIB_OBJS += $(COMPAT_OBJS) # Quote for C diff --git a/config.mak.uname b/config.mak.uname index 7d31fad..8aea8a6 100644 --- a/config.mak.uname +++ b/config.mak.uname @@ -188,6 +188,7 @@ ifeq ($(uname_S),FreeBSD) endif PYTHON_PATH = /usr/local/bin/python HAVE_PATHS_H = YesPlease + PAGER_ENV = LESS=-FRSX LV=-c MORE=-R endif ifeq ($(uname_S),OpenBSD) NO_STRCASESTR = YesPlease -- To unsubscribe from this list: send the line 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: with git-next, git pull --rebase can print out fatal: No such ref: '' for branches without an upstream
John Keeping j...@keeping.me.uk writes: Perhaps something like this is needed? ... Either that or 2/dev/null like in the original, yes. Ah, that makes sense. I see you already followed-up with a patch, so I'll pick it up. 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 1/2] prefer xwrite instead of write
Jonathan Nieder jrnie...@gmail.com writes: Junio C Hamano wrote: Jonathan Nieder jrnie...@gmail.com writes: Shouldn't this use write_in_full() to avoid a silently truncated result? (*) Meaning this? If so, I think it makes sense. [...] -if (xwrite(fd, out.buf, out.len) 0) +if (write_in_full(fd, out.buf, out.len) != out.len) Yes. Either ' 0' or '!= out.len' would work fine here, since write_in_full is defined to always either write the full 'count' bytes or return an error. An unrelated tangent but we may want to fix majority of callers that do not seem to know that ;-) -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/3] setup_pager: set MORE=R
Junio C Hamano gits...@pobox.com writes:
Perhaps we can start it like this. Then pager.c can iterate over
the PAGER_ENV string, parse out LESS=, LV=, etc., and do its thing.
We would also need to muck with git-sh-setup.sh but that file is
already preprocessed in the Makefile, so we should be able to do
something similar to # @@BROKEN_PATH_FIX@@ there.
And here is such an attempt. There may be some missing dependencies
that need to be covered with a mechanism like GIT-BUILD-OPTIONS,
though.
Makefile | 18 --
config.mak.uname | 1 +
git-sh-setup.sh | 9 +
pager.c | 44 +---
4 files changed, 55 insertions(+), 17 deletions(-)
diff --git a/Makefile b/Makefile
index b4af1e2..690f4c6 100644
--- a/Makefile
+++ b/Makefile
@@ -342,6 +342,14 @@ all::
# Define DEFAULT_HELP_FORMAT to man, info or html
# (defaults to man) if you want to have a different default when
# git help is called without a parameter specifying the format.
+#
+# Define PAGER_ENV to a SP separated VAR=VAL pairs to define
+# default environment variables to be passed when a pager is spawned, e.g.
+#
+#PAGER_ENV = LESS=-FRSX LV=-c
+#
+# to say export LESS=-FRSX (and LV=-c) if the environment variable
+# LESS (and LV) is not set, respectively.
GIT-VERSION-FILE: FORCE
@$(SHELL_PATH) ./GIT-VERSION-GEN
@@ -1506,6 +1514,10 @@ ifeq ($(PYTHON_PATH),)
NO_PYTHON = NoThanks
endif
+ifndef PAGER_ENV
+PAGER_ENV = LESS=-FRSX LV=-c
+endif
+
QUIET_SUBDIR0 = +$(MAKE) -C # space to separate -C and subdir
QUIET_SUBDIR1 =
@@ -1585,11 +1597,12 @@ PYTHON_PATH_SQ = $(subst ','\'',$(PYTHON_PATH))
TCLTK_PATH_SQ = $(subst ','\'',$(TCLTK_PATH))
DIFF_SQ = $(subst ','\'',$(DIFF))
PERLLIB_EXTRA_SQ = $(subst ','\'',$(PERLLIB_EXTRA))
+PAGER_ENV_SQ = $(subst ','\'',$(PAGER_ENV))
LIBS = $(GITLIBS) $(EXTLIBS)
BASIC_CFLAGS += -DSHA1_HEADER='$(SHA1_HEADER_SQ)' \
- $(COMPAT_CFLAGS)
+ $(COMPAT_CFLAGS) -DPAGER_ENV='$(PAGER_ENV)'
LIB_OBJS += $(COMPAT_OBJS)
# Quote for C
@@ -1739,7 +1752,7 @@ common-cmds.h: $(wildcard Documentation/git-*.txt)
SCRIPT_DEFINES = $(SHELL_PATH_SQ):$(DIFF_SQ):$(GIT_VERSION):\
$(localedir_SQ):$(NO_CURL):$(USE_GETTEXT_SCHEME):$(SANE_TOOL_PATH_SQ):\
- $(gitwebdir_SQ):$(PERL_PATH_SQ)
+ $(gitwebdir_SQ):$(PERL_PATH_SQ):$(PAGER_ENV)
define cmd_munge_script
$(RM) $@ $@+ \
sed -e '1s|#!.*/sh|#!$(SHELL_PATH_SQ)|' \
@@ -1751,6 +1764,7 @@ sed -e '1s|#!.*/sh|#!$(SHELL_PATH_SQ)|' \
-e $(BROKEN_PATH_FIX) \
-e 's|@@GITWEBDIR@@|$(gitwebdir_SQ)|g' \
-e 's|@@PERL@@|$(PERL_PATH_SQ)|g' \
+-e 's|@@PAGER_ENV@@|$(PAGER_ENV_SQ)|g' \
$@.sh $@+
endef
diff --git a/config.mak.uname b/config.mak.uname
index 7d31fad..8aea8a6 100644
--- a/config.mak.uname
+++ b/config.mak.uname
@@ -188,6 +188,7 @@ ifeq ($(uname_S),FreeBSD)
endif
PYTHON_PATH = /usr/local/bin/python
HAVE_PATHS_H = YesPlease
+ PAGER_ENV = LESS=-FRSX LV=-c MORE=-R
endif
ifeq ($(uname_S),OpenBSD)
NO_STRCASESTR = YesPlease
diff --git a/git-sh-setup.sh b/git-sh-setup.sh
index fffa3c7..8fc1bbd 100644
--- a/git-sh-setup.sh
+++ b/git-sh-setup.sh
@@ -158,10 +158,11 @@ git_pager() {
else
GIT_PAGER=cat
fi
- : ${LESS=-FRSX}
- : ${LV=-c}
- export LESS LV
-
+ for vardef in @@PAGER_ENV@@
+ do
+ var=${vardef%%=*}
+ eval : \${$vardef} export $var
+ done
eval $GIT_PAGER '$@'
}
diff --git a/pager.c b/pager.c
index 0cc75a8..81a8af7 100644
--- a/pager.c
+++ b/pager.c
@@ -1,6 +1,7 @@
#include cache.h
#include run-command.h
#include sigchain.h
+#include argv-array.h
#ifndef DEFAULT_PAGER
#define DEFAULT_PAGER less
@@ -60,9 +61,37 @@ const char *git_pager(int stdout_is_tty)
return pager;
}
+#define stringify_(x) #x
+#define stringify(x) stringify_(x)
+
+static void setup_pager_env(struct argv_array *env)
+{
+ const char *pager_env = stringify(PAGER_ENV);
+
+ while (*pager_env) {
+ struct strbuf buf = STRBUF_INIT;
+ const char *cp = strchrnul(pager_env, '=');
+
+ if (!*cp)
+ die(malformed build-time PAGER_ENV);
+ strbuf_add(buf, pager_env, cp - pager_env);
+ cp = strchrnul(pager_env, ' ');
+ if (!getenv(buf.buf)) {
+ strbuf_reset(buf);
+ strbuf_add(buf, pager_env, cp - pager_env);
+ argv_array_push(env, strbuf_detach(buf, NULL));
+ }
+ strbuf_reset(buf);
+ while (*cp *cp == ' ')
+ cp++;
+ pager_env = cp;
+ }
+}
+
void setup_pager(void)
{
const char *pager = git_pager(isatty(1));
+ struct argv_array env = ARGV_ARRAY_INIT;
if (!pager || pager_in_use())
return;
@@ -80,17 +109,10 @@ void
Re: [PATCH 1/5] diff_filespec: reorder dirty_submodule macro definitions
Jeff King p...@peff.net writes: I'm happy with it either way. I almost just pulled the macro definitions, including DIFF_FILE_VALID, out of the struct definition completely. I see the value in having the flags near their bitfield, but it makes the definition a bit harder to read. Yeah, my thoughts exactly when I did those two conflicting changes. I have a slight preference Constants go with the fields they are used in over fields and macros mixed together is harder to read, so let's use your patch as-is. -- To unsubscribe from this list: send the line 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] safe_create_leading_directories(): on Windows, \ can separate path components
Sebastian Schuberth sschube...@gmail.com writes: On Sun, Jan 19, 2014 at 12:40 AM, Michael Haggerty mhag...@alum.mit.edu wrote: This patch applies on top of v3 of mh/safe-create-leading-directories. The only logical change from Sebastian's patch is that this version restores the original slash character rather than always restoring it to '/' (as suggested by Junio). Thanks Michael. This is very similar to Junio's current merge conflict resultion in pu between your original series and my patch (c2fec83). I like this patch of yours slightly better, however, as it uses a while instead of a for loop and the more conclusive slash_character than the was_slash variable name. Yeah, I agree this patch that uses more descriptive names is a much more desirable end-result than the tentative merge in 'pu'. -- To unsubscribe from this list: send the line 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/3] setup_pager: set MORE=R
Jeff King p...@peff.net writes: ... does complicate the point of my series, which was to add more intimate logic about how we handle LESS. ... return !x || strchr(x, 'R'); } [...] } but we are still hard-coding a lot of intelligence about less here. I am not sure if it is even a good idea for us to have so intimate logic for various pagers in the first place. I'd seriously wonder if it is better to take this position: A platform packager who sets the default pager and/or the default environment for the pager at the build time, or an individual user who tells your Git what pager you want to use and/or with what setting that pager should be run under with environment variables. These people ought to know far better than Git what their specific choices do. Do not try to second-guess them. The potential breakage caused by setting MORE=R unconditionally is a good example. A careless intimate logic may think that any pager that is called 'more' would behave like traditional 'more', breaking half the 'more' user population while catering to the other half. I wonder if the abstraction provided by the Makefile variable is really worthwhile. Anybody adding a new line to it would also want to tweak pager_can_handle_color to add similar logic. And that is why I am not enthused by the idea of adding such logic in the first place. I view the Makefile customization as a way for the packager to offer a sensible default for their platform without touching the code, which is slightly different from your 1. below. Taking a step back for a moment, we are getting two things out of such a Makefile variable: 1. An easy way for packager to add intelligence about common pagers on their system. 2. DRY between git-sh-setup and the C code. I do like (1), but I do not know if we want to try to encode the can handle color logic into a Makefile variable. What would it look like? For (2), an alternate method would be to simply provide git pager as C code, which spawns the appropriate pager as the C code would. Then git-sh-setup can easily build around that. And as to 2., if the answer to the other issue do we want our code to be intimately aware of pager-specific quirks, or do we just want to give packagers a knob to express their choice of the default? resolves to the former, I would think that git pager would be not just a workable alternative, but would be the only viable one. -- To unsubscribe from this list: send the line 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: Verifiable git archives?
Michael Haggerty mhag...@alum.mit.edu writes: On 01/09/2014 09:11 PM, Junio C Hamano wrote: Andy Lutomirski l...@amacapital.net writes: It's possible, in principle, to shove enough metadata into the output of 'git archive' to allow anyone to verify (without cloning the repo) to verify that the archive is a correct copy of a given commit. Would this be considered a useful feature? Presumably there would be a 'git untar' command that would report failure if it fails to verify the archive contents. This could be as simple as including copies of the commit object and all relevant tree objects and checking all of the hashes when untarring. You only need the object name of the top-level tree. After untar the archive into an empty directory, make it a new repository and git add . git write-tree---the result should match the top-level tree the archive was supposed to contain. [...] This wouldn't work if any files were excluded from the archive using gitattribute export-ignore (or export-subst, which you already mentioned in a follow-up email). Correct. By and such below, I meant any and all futzing that makes the resulting working tree different from the tree object being archived ;-) That includes the line-ending configuration and other things as well. Also, if you used keyword substitution and such when creating an archive, then the filesystem entities resulting from expanding it would not match the original. -- To unsubscribe from this list: send the line 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/3] setup_pager: set MORE=R
Jeff King p...@peff.net writes:
Perhaps instead of just dumping it into a macro, we could actually parse
it at compile time into C code, and store the result as a header file.
Then that header file becomes the proper dependency, and this run-time
error:
+while (*pager_env) {
+struct strbuf buf = STRBUF_INIT;
+const char *cp = strchrnul(pager_env, '=');
+
+if (!*cp)
+die(malformed build-time PAGER_ENV);
would become a compile-time error. We could do the same for the shell
code, too.
I'm thinking something like:
Heh, great minds think alike. I did start writing a toy-patch with
a new file called pager-env.h, before discarding it and sending a
simpler alternative which you are responding to.
I do not mind the approach at all; the primary reason I stopped
doing that was because the amount of code to get quotes right turned
out to be sufficiently big to obscure the real point of the how
about doing it this way illustration patch.
diff --git a/Makefile b/Makefile
index b4af1e2..3a8d15e 100644
--- a/Makefile
+++ b/Makefile
@@ -2182,6 +2182,16 @@ GIT-LDFLAGS: FORCE
echo $$FLAGS GIT-LDFLAGS; \
fi
+GIT-PAGER-ENV:
+ @PAGER_ENV='$(PAGER_ENV)'; \
+ if test x$$PAGER_ENV != x`cat GIT-PAGER-ENV 2/dev/null`; then \
+ echo $$PAGER_ENV GIT-PAGER-ENV; \
+ fi
+
+pager-env.h: GIT-PAGER-ENV mkcarray
+ $(SHELL_PATH) mkcarray pager_env $ $@+
+ mv $@+ $@
+
# We need to apply sq twice, once to protect from the shell
# that runs GIT-BUILD-OPTIONS, and then again to protect it
# and the first level quoting from the shell that runs echo.
diff --git a/mkcarray b/mkcarray
index e69de29..5962440 100644
--- a/mkcarray
+++ b/mkcarray
@@ -0,0 +1,21 @@
+name=$1; shift
+guard=$(echo $name | tr a-z A-Z)
+
+cat -EOF
+#ifndef ${guard}_H
+#define ${guard}_H
+
+static const char *${name} = {
+EOF
+
+for i in $(cat); do
+ # XXX c-quote the values?
+ printf '\t%s,\n' $i
+done
+
+cat EOF
+ NULL
+};
+
+#endif /* ${guard}_H */
+EOF
--
To unsubscribe from this list: send the line 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] Fix safe_create_leading_directories() for Windows
Sebastian Schuberth sschube...@gmail.com writes: On Thu, Jan 2, 2014 at 10:08 PM, Junio C Hamano gits...@pobox.com wrote: Seems like the path to clone to is taken as-is from argv in cmd_clone(). So maybe another solution would be to replace all backslashes with forward slashes already there? That sounds like a workable alternative, and it might even be a preferable solution but if and only if clone's use of the function to create paths that lead to a new working tree location is the only (ab)use of the function. That was what I meant when I said it may be that it is a bug in the specific caller. AFAIK, the function I think Dscho made valid points in his other mail that the better solution still is to make safe_create_leading_directories() actually safe, also regarding its arguments. Sorry, but I think I misremebered the reason why we have that function. There are two reasons we may need to do a rough equivalent of system(mkdir -p $(dirname a/b/c)) given an argument 'a/b/c' in our codebase. 1. We would want to be able to create 'c' such that we can later refer to a/b/c to retrieve what we wrote there, so we need to make sure that a/b/ refers to a writable directory. 2. We were told by the index (or a caller that will then later update the index in a consistent way) that 'a/b/c' must exist in the working tree, so we need to make sure that a/ and a/b/ are both directories (not a symlink to a directory elsewhere). Seeing hits git grep safe_create_leading_directories from apply and merge-recursive led me to incorrectly assume that this function was meant to do the latter [*1*]. But the original purpose of safe_create_leading_directories() in sha1_file.c was to mkdir -p inside .git/ directories when writing to refs e.g. refs/heads/foo/bar, and for this kind of use, we must honor existing symlinks. .git/refs may be a symbolic link in a working tree managed by new-workdir to the real repository, and we do not want to unlink mkdir it. We even have an offset-1st-component call in the function, which is a clear sign that we would want this as usable for the full path in the filesystem, which is an indication that this should not be used to create paths in the working tree. So I think it is the right thing to do to allow the function accept platform specific path here, and it is OK for clone to call it to create the path leading to the location of the new repository. A related tangent is that somebody needs to audit the callers to make sure this function is _not_ used to create leading paths in the working tree. If a merge tells us to create foo/bar/baz.test, we should not do an equivalent of mkdir -p foo/bar cat baz.test; we must make sure foo/ and foo/bar are real directories without any symbolic link in the leading paths components (the same thing can be said for apply). I have a suspicion that the two hits from apply and merge-recursive are showing an existing bug that is not platform specific. Thanks. [Footnote] *1* In such a context, getting a/b\c/d and failing to make sure a/ is a directory that has b\c/ as its immediate subdirectory is a bug---especially, splitting at the backslash between 'b' and 'c' and creating 'a/b/c/' is not acceptable. The platform code should instead say sorry, we cannot do that if it cannot create a directory entry b\c in the directory a/ (which would make sure such a non-portable path in projects will be detected). -- To unsubscribe from this list: send the line 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 0/2] Two janitorial patches for builtin/blame.c
Jonathan Nieder jrnie...@gmail.com writes: David Kastrup wrote: So my understanding is that when we are talking about _significant_ additions to builtin/blame.c (the current patches don't qualify as such really) that a) builtin/blame.c is licensed under GPLv2 b) significant contributions to it will not be relicensed under different licenses without the respective contributors' explicit consent. Yep, that's how it works. [...] The combination of the SubmittingPatches text with the file notices in builtin/blame.c is not really painting a full picture of the situation. Any idea how this could be made more clear? E.g., maybe we should bite the bullet and add a line to all source files that don't already state a license: /* * License: GPLv2. See COPYING for details. */ I vaguely recall that jgit folks at one point wanted to lift this implementation and were interested in seeing it to be dual licensed to BSD but that was a long time ago. http://git.661346.n2.nabble.com/JGIT-Blame-functionality-for-jgit-td2142726.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: GIT_WORK_TREE and GIT_DIR with git clone
Cosmin Apreutesei cosmin.apreute...@gmail.com writes: I would like to be able to tell my users that they can simply do: git clone --git-dir=_/git/module1/.git module1-url git clone --git-dir=_/git/module2/.git module2-url which will overlay the files from both modules into the current directory, which from git's perspective is the work-tree for both modules. Would there be a sensible semantics in the resulting working tree? Which repository would a new file in such a combined working tree belong to? -- To unsubscribe from this list: send the line 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/RFC] Makefile: Fix compilation of windows resource file
Ramsay Jones ram...@ramsay1.demon.co.uk writes: If the git version number consists of less than three period separated numbers, then the windows resource file compilation issues a syntax error: $ touch git.rc $ make V=1 git.res GIT_VERSION = 1.9.rc0 windres -O coff \ -DMAJOR=1 -DMINOR=9 -DPATCH=rc0 \ -DGIT_VERSION=\\\1.9.rc0\\\ git.rc -o git.res C:\msysgit\msysgit\mingw\bin\windres.exe: git.rc:2: syntax error make: *** [git.res] Error 1 $ [Note that -DPATCH=rc0] Thanks for a report. I've been wondering how many distros and packagers would have an issue like this when we go to 2-digit release naming. Of course we knew everybody can grok 3-or-4 ;-) In order to fix the syntax error, we replace any rcX with zero and include some additional 'zero' padding to the version number list. Signed-off-by: Ramsay Jones ram...@ramsay1.demon.co.uk --- Hi Junio, This patch is marked RFC because, as I was just about to send this email, I realized it wouldn't always work: Yeah, and I suspect that with the use of $(wordlist 1,3,...) it is not even working for maintenance releases. Does it differenciate between 1.8.5.1 and 1.8.5.2, for example?. Or does windres always assume that a package version is always 3-dewey-decimal (not 2, not 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: REQUEST-PULL: Please pull from git-gui.
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/RFC] Makefile: Fix compilation of windows resource file
Junio C Hamano gits...@pobox.com writes:
Ramsay Jones ram...@ramsay1.demon.co.uk writes:
If the git version number consists of less than three period
separated numbers, then the windows resource file compilation
issues a syntax error:
$ touch git.rc
$ make V=1 git.res
GIT_VERSION = 1.9.rc0
windres -O coff \
-DMAJOR=1 -DMINOR=9 -DPATCH=rc0 \
-DGIT_VERSION=\\\1.9.rc0\\\ git.rc -o git.res
C:\msysgit\msysgit\mingw\bin\windres.exe: git.rc:2: syntax error
make: *** [git.res] Error 1
$
[Note that -DPATCH=rc0]
Thanks for a report. I've been wondering how many distros and
packagers would have an issue like this when we go to 2-digit
release naming. Of course we knew everybody can grok 3-or-4 ;-)
In order to fix the syntax error, we replace any rcX with zero and
include some additional 'zero' padding to the version number list.
Signed-off-by: Ramsay Jones ram...@ramsay1.demon.co.uk
---
Hi Junio,
This patch is marked RFC because, as I was just about to send this
email, I realized it wouldn't always work:
Yeah, and I suspect that with the use of $(wordlist 1,3,...) it is
not even working for maintenance releases. Does it differenciate
between 1.8.5.1 and 1.8.5.2, for example?. Or does windres always
assume that a package version is always 3-dewey-decimal (not 2, not
4)?
Perhaps like this? Just grab digit-only segments that are separated
with either dot or dash (and stop when we see a non-digit like
'dirty' or 'rcX'), and make them separated with comma.
Note that I am merely guessing that short-digit version numbers
are acceptable by now after seeing
https://sourceware.org/ml/binutils/2012-07/msg00199.html
without knowing the current state of affairs. If that is not the
case you may have to count the iteration of the loop and append or
chop the resulting string as necessary.
Makefile | 2 +-
gen-version-string.sh | 13 +
git.rc| 4 ++--
3 files changed, 16 insertions(+), 3 deletions(-)
diff --git a/Makefile b/Makefile
index b4af1e2..329f942 100644
--- a/Makefile
+++ b/Makefile
@@ -1773,7 +1773,7 @@ $(SCRIPT_LIB) : % : %.sh GIT-SCRIPT-DEFINES
git.res: git.rc GIT-VERSION-FILE
$(QUIET_RC)$(RC) \
- $(join -DMAJOR= -DMINOR= -DPATCH=, $(wordlist 1,3,$(subst -, ,$(subst
., ,$(GIT_VERSION) \
+ -DVERSIONSTRING=$$(./gen-version-string.sh $(GIT_VERSION)) \
-DGIT_VERSION=\\\$(GIT_VERSION)\\\ $ -o $@
ifndef NO_PERL
diff --git a/gen-version-string.sh b/gen-version-string.sh
new file mode 100755
index 000..00af718
--- /dev/null
+++ b/gen-version-string.sh
@@ -0,0 +1,13 @@
+#!/bin/sh
+
+IFS=.- result=
+for v in $1
+do
+ if expr $v : '[0-9][0-9]*$' /dev/null
+ then
+ result=$result${result:+,}$v
+ else
+ break
+ fi
+done
+echo $result
diff --git a/git.rc b/git.rc
index bce6db9..6f2a8d2 100644
--- a/git.rc
+++ b/git.rc
@@ -1,6 +1,6 @@
1 VERSIONINFO
-FILEVERSION MAJOR,MINOR,PATCH,0
-PRODUCTVERSION MAJOR,MINOR,PATCH,0
+FILEVERSION VERSIONSTRING,0
+PRODUCTVERSION VERSIONSTRING,0
BEGIN
BLOCK StringFileInfo
BEGIN
--
To unsubscribe from this list: send the line 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] builtin/blame.c: struct blame_entry does not need a prev link
David Kastrup d...@gnu.org writes:
---
Thanks. At some point during its development I must have thought
that having it as a dual-linked list may make it easier when we have
to split a block into pieces, but it seems that split_overlap() does
not need to look at this information.
Needs sign-off.
builtin/blame.c | 13 ++---
1 file changed, 2 insertions(+), 11 deletions(-)
diff --git a/builtin/blame.c b/builtin/blame.c
index e44a6bb..2195595 100644
--- a/builtin/blame.c
+++ b/builtin/blame.c
@@ -197,7 +197,6 @@ static void drop_origin_blob(struct origin *o)
* scoreboard structure, sorted by the target line number.
*/
struct blame_entry {
- struct blame_entry *prev;
struct blame_entry *next;
/* the first line of this group in the final image;
@@ -282,8 +281,6 @@ static void coalesce(struct scoreboard *sb)
ent-s_lno + ent-num_lines == next-s_lno) {
ent-num_lines += next-num_lines;
ent-next = next-next;
- if (ent-next)
- ent-next-prev = ent;
origin_decref(next-suspect);
free(next);
ent-score = 0;
@@ -534,7 +531,7 @@ static void add_blame_entry(struct scoreboard *sb, struct
blame_entry *e)
prev = ent;
/* prev, if not NULL, is the last one that is below e */
- e-prev = prev;
+
if (prev) {
e-next = prev-next;
prev-next = e;
@@ -543,8 +540,6 @@ static void add_blame_entry(struct scoreboard *sb, struct
blame_entry *e)
e-next = sb-ent;
sb-ent = e;
}
- if (e-next)
- e-next-prev = e;
}
/*
@@ -555,14 +550,12 @@ static void add_blame_entry(struct scoreboard *sb,
struct blame_entry *e)
*/
static void dup_entry(struct blame_entry *dst, struct blame_entry *src)
{
- struct blame_entry *p, *n;
+ struct blame_entry *n;
- p = dst-prev;
n = dst-next;
origin_incref(src-suspect);
origin_decref(dst-suspect);
memcpy(dst, src, sizeof(*src));
- dst-prev = p;
dst-next = n;
dst-score = 0;
}
@@ -2502,8 +2495,6 @@ parse_done:
ent-suspect = o;
ent-s_lno = bottom;
ent-next = next;
- if (next)
- next-prev = ent;
origin_incref(o);
}
origin_decref(o);
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[ANNOUNCE] Git v1.9-rc0
for http auth tests t: set TEST_OUTPUT_DIRECTORY for sub-tests t: simplify HARNESS_ACTIVE hack t: drop known breakage test t5531: further matching fixups Jens Lehmann (4): submodule update: remove unnecessary orig_flags variable commit -v: strip diffs and submodule shortlogs from the commit message mv: better document side effects when moving a submodule rm: better document side effects when removing a submodule Johan Herland (1): sha1_file.c:create_tmpfile(): Fix race when creating loose object dirs Johannes Schindelin (1): Remove the line length limit for graft files Johannes Sixt (4): document --exclude option git_connect: remove artificial limit of a remote command git_connect: factor out discovery of the protocol and its parts mv: let 'git mv file no-such-dir/' error out on Windows, too John Keeping (8): repo-config: remove deprecated alias for git config tar-tree: remove deprecated command lost-found: remove deprecated command peek-remote: remove deprecated alias of ls-remote pull: use merge-base --fork-point when appropriate rebase: use reflog to find common base with upstream rebase: fix fork-point with zero arguments pull: suppress error when no remoteref is found John Murphy (1): git-gui: corrected setup of git worktree under cygwin. John Szakmeister (1): contrib/git-credential-gnome-keyring.c: small stylistic cleanups Jonathan Nieder (16): git-remote-mediawiki: do not remove installed files in clean target git-remote-mediawiki: honor DESTDIR in make install git-remote-mediawiki build: make 'install' command configurable git-remote-mediawiki build: handle DESTDIR/INSTLIBDIR with whitespace Makefile: rebuild perl scripts when perl paths change Makefile: add PERLLIB_EXTRA variable that adds to default perl path mark Windows build scripts executable mark perl test scripts executable mark contributed hooks executable contrib: remove git-p4import test: make FILEMODE a lazy prereq test: replace shebangs with descriptions in shell libraries remove #!interpreter line from shell libraries stop installing git-tar-tree link pager: set LV=-c alongside LESS=FRSX diff test: reading a directory as a file need not error out Junio C Hamano (28): revision: introduce --exclude=glob to tame wildcards merge-base: use OPT_CMDMODE and clarify the command line parsing merge-base: teach --fork-point mode rev-list --exclude: tests rev-list --exclude: export add/clear-ref-exclusion and ref-excluded API rev-parse: introduce --exclude=glob to tame wildcards t1005: reindent t1005: add test for read-tree --reset -u A B sha1_loose_object_info(): do not return success on missing object bundle: use argv-array submodule: do not copy unknown update mode from .gitmodules Git 1.8.4.5 Git 1.8.5.1 builtin/push.c: use strbuf instead of manual allocation push: use remote.$name.push as a refmap push: also use upstream mapping when pushing a single ref Start 1.9 cycle Update draft release notes to 1.9 prune-packed: use strbuf to avoid having to worry about PATH_MAX Git 1.8.5.2 Update draft release notes to 1.9 get_max_fd_limit(): fall back to OPEN_MAX upon getrlimit/sysconf failure merge-base: separate --independent codepath into its own helper merge-base --octopus: reduce the result from get_octopus_merge_bases() Update draft release notes to 1.9 Git 1.8.5.3 Update draft release notes to 1.9 Git 1.9-rc0 Karsten Blees (1): gitignore.txt: clarify recursive nature of excluded directories Krzesimir Nowak (4): gitweb: Move check-ref-format code into separate function gitweb: Return 1 on validation success instead of passed input gitweb: Add a feature for adding more branch refs gitweb: Denote non-heads, non-remotes branches Kyle J. McKay (1): gc: notice gc processes run by other users Mads Dørup (1): git-gui: Improve font rendering on retina macbooks Masanari Iida (4): typofixes: fix misspelt comments Documentation/technical/http-protocol.txt: typofixes contrib: typofixes git-gui: correct spelling errors in comments Matthieu Moy (1): mv: let 'git mv file no-such-dir/' error out Max Kirillov (2): git-gui: Add gui.displayuntracked option git-gui: right half window is paned Michael Haggerty (27): t5510: use the correct tag name in test t5510: prepare test refs more straightforwardly t5510: check that git fetch --prune --tags does not prune branches api-remote.txt: correct section struct refspec get_ref_map(): rename local variables builtin/fetch.c: reorder function definitions get_expanded_map
Re: [PATCH v2 04/16] trailer: process command line trailer arguments
Christian Couder chrisc...@tuxfamily.org writes:
This patch parses the trailer command line arguments
and put the result into an arg_tok doubly linked
list.
Signed-off-by: Christian Couder chrisc...@tuxfamily.org
---
trailer.c | 77
+++
1 file changed, 77 insertions(+)
diff --git a/trailer.c b/trailer.c
index e7d8244..bb1fcfb 100644
--- a/trailer.c
+++ b/trailer.c
@@ -363,3 +363,80 @@ static int git_trailer_config(const char *conf_key,
const char *value, void *cb)
}
return 0;
}
+
+static void parse_trailer(struct strbuf *tok, struct strbuf *val, const char
*trailer)
+{
+ char *end = strchr(trailer, '=');
+ if (!end)
+ end = strchr(trailer, ':');
+ if (end) {
+ strbuf_add(tok, trailer, end - trailer);
+ strbuf_trim(tok);
+ strbuf_addstr(val, end + 1);
+ strbuf_trim(val);
+ } else {
+ strbuf_addstr(tok, trailer);
+ strbuf_trim(tok);
+ }
+}
+
+static struct trailer_item *create_trailer_item(const char *string)
+{
+ struct strbuf tok = STRBUF_INIT;
+ struct strbuf val = STRBUF_INIT;
+ struct trailer_item *new;
+
+ parse_trailer(tok, val, string);
+
+ int tok_alnum_len = alnum_len(tok.buf, tok.len);
decl-after-stmt.
+
+ /* Lookup if the token matches something in the config */
+ struct trailer_item *item;
+ for (item = first_conf_item; item; item = item-next)
+ {
+ if (!strncasecmp(tok.buf, item-conf-key, tok_alnum_len) ||
+ !strncasecmp(tok.buf, item-conf-name, tok_alnum_len)) {
+ new = xcalloc(sizeof(struct trailer_item), 1);
+ new-conf = item-conf;
+ new-token = xstrdup(item-conf-key);
+ new-value = strbuf_detach(val, NULL);
+ strbuf_release(tok);
+ return new;
+ }
+ }
+
+ new = xcalloc(sizeof(struct trailer_item), 1);
+ new-conf = xcalloc(sizeof(struct conf_info), 1);
+ new-token = strbuf_detach(tok, NULL);
+ new-value = strbuf_detach(val, NULL);
+
+ return new;
+}
+
+static void add_trailer_item(struct trailer_item **first,
+ struct trailer_item **last,
+ struct trailer_item *new)
+{
+ if (!*last) {
+ *first = new;
+ *last = new;
+ } else {
+ (*last)-next = new;
+ new-previous = *last;
+ *last = new;
+ }
+}
+
+static struct trailer_item *process_command_line_args(int argc, const char
**argv)
+{
+ int i;
+ struct trailer_item *arg_tok_first = NULL;
+ struct trailer_item *arg_tok_last = NULL;
+
+ for (i = 0; i argc; i++) {
+ struct trailer_item *new = create_trailer_item(argv[i]);
+ add_trailer_item(arg_tok_first, arg_tok_last, new);
+ }
+
+ return arg_tok_first;
+}
--
To unsubscribe from this list: send the line 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/2] list-objects: only look at cmdline trees with edge_hint
Jeff King p...@peff.net writes:
This tradeoff probably makes sense in the context of
pack-objects, where we have set revs-edge_hint to have the
traversal feed us the set of boundary objects. For a
regular rev-list, though, it is probably not a good
tradeoff. It is true that it makes our list slightly closer
to a true set difference, but it is a rare case where this
is important. And because we do not have revs-edge_hint
set, we do nothing useful with the larger set of boundary
objects.
This patch therefore ties the extra tree examination to the
revs-edge_hint flag; it is the presence of that flag that
makes the tradeoff worthwhile.
Makes sense. Thanks. Will queue.
Here is output from the p0001-rev-list showing the
improvement in performance:
Test HEAD^ HEAD
-
0001.1: rev-list --all 0.69(0.65+0.02)
0.69(0.66+0.02) +0.0%
0001.2: rev-list --all --objects 3.22(3.19+0.03)
3.23(3.20+0.03) +0.3%
0001.4: rev-list $commit --not --all 0.04(0.04+0.00)
0.04(0.04+0.00) +0.0%
0001.5: rev-list --objects $commit --not --all 0.27(0.26+0.01)
0.04(0.04+0.00) -85.2%
Signed-off-by: Jeff King p...@peff.net
---
list-objects.c | 20 +++-
1 file changed, 11 insertions(+), 9 deletions(-)
diff --git a/list-objects.c b/list-objects.c
index 6cbedf0..206816f 100644
--- a/list-objects.c
+++ b/list-objects.c
@@ -162,15 +162,17 @@ void mark_edges_uninteresting(struct rev_info *revs,
show_edge_fn show_edge)
}
mark_edge_parents_uninteresting(commit, revs, show_edge);
}
- for (i = 0; i revs-cmdline.nr; i++) {
- struct object *obj = revs-cmdline.rev[i].item;
- struct commit *commit = (struct commit *)obj;
- if (obj-type != OBJ_COMMIT || !(obj-flags UNINTERESTING))
- continue;
- mark_tree_uninteresting(commit-tree);
- if (revs-edge_hint !(obj-flags SHOWN)) {
- obj-flags |= SHOWN;
- show_edge(commit);
+ if (revs-edge_hint) {
+ for (i = 0; i revs-cmdline.nr; i++) {
+ struct object *obj = revs-cmdline.rev[i].item;
+ struct commit *commit = (struct commit *)obj;
+ if (obj-type != OBJ_COMMIT || !(obj-flags
UNINTERESTING))
+ continue;
+ mark_tree_uninteresting(commit-tree);
+ if (!(obj-flags SHOWN)) {
+ obj-flags |= SHOWN;
+ show_edge(commit);
+ }
}
}
}
--
To unsubscribe from this list: send the line 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 04/16] trailer: process command line trailer arguments
Junio C Hamano gits...@pobox.com writes:
+static struct trailer_item *create_trailer_item(const char *string)
+{
+struct strbuf tok = STRBUF_INIT;
+struct strbuf val = STRBUF_INIT;
+struct trailer_item *new;
+
+parse_trailer(tok, val, string);
+
+int tok_alnum_len = alnum_len(tok.buf, tok.len);
decl-after-stmt.
+
+/* Lookup if the token matches something in the config */
+struct trailer_item *item;
ditto.
+for (item = first_conf_item; item; item = item-next)
+{
Style.
I wonder if Cc list is being a bit too wide for this series, by the
way.
--
To unsubscribe from this list: send the line 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 repack --max-pack-size broken in git-next
Siddharth Agarwal s...@fb.com writes: With git-next, the --max-pack-size option to git repack doesn't work. With git at b139ac2, `git repack --max-pack-size=1g` says error: option `max-pack-size' expects a numerical value Thanks, Siddharth. It seems that we have a hand-crafted parser outside parse-options framework in pack-objects, and the scripted git-repack used to pass this option without interpreting itself. We probably should lift the OPT_ULONG() implementation out of builtin/pack-objects.c and move it to parse-options.[ch] and use it in the reimplementation of repack. And probably use it in other places where these integers in human-friendly units may make sense, but that is a separate topic. -- To unsubscribe from this list: send the line 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 repack --max-pack-size broken in git-next
Junio C Hamano gits...@pobox.com writes:
Siddharth Agarwal s...@fb.com writes:
With git-next, the --max-pack-size option to git repack doesn't work.
With git at b139ac2, `git repack --max-pack-size=1g` says
error: option `max-pack-size' expects a numerical value
Thanks, Siddharth.
It seems that we have a hand-crafted parser outside parse-options
framework in pack-objects, and the scripted git-repack used to pass
this option without interpreting itself.
We probably should lift the OPT_ULONG() implementation out of
builtin/pack-objects.c and move it to parse-options.[ch] and use it
in the reimplementation of repack.
And probably use it in other places where these integers in
human-friendly units may make sense, but that is a separate topic.
The first step may look something like this (totally untested)..
builtin/pack-objects.c | 22 +++---
parse-options.c| 17 +
parse-options.h| 3 +++
3 files changed, 23 insertions(+), 19 deletions(-)
diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
index 541667f..b264f1f 100644
--- a/builtin/pack-objects.c
+++ b/builtin/pack-objects.c
@@ -2416,23 +2416,6 @@ static int option_parse_unpack_unreachable(const struct
option *opt,
return 0;
}
-static int option_parse_ulong(const struct option *opt,
- const char *arg, int unset)
-{
- if (unset)
- die(_(option %s does not accept negative form),
- opt-long_name);
-
- if (!git_parse_ulong(arg, opt-value))
- die(_(unable to parse value '%s' for option %s),
- arg, opt-long_name);
- return 0;
-}
-
-#define OPT_ULONG(s, l, v, h) \
- { OPTION_CALLBACK, (s), (l), (v), n, (h), \
- PARSE_OPT_NONEG, option_parse_ulong }
-
int cmd_pack_objects(int argc, const char **argv, const char *prefix)
{
int use_internal_rev_list = 0;
@@ -2462,8 +2445,9 @@ int cmd_pack_objects(int argc, const char **argv, const
char *prefix)
N_(ignore packed objects)),
OPT_INTEGER(0, window, window,
N_(limit pack window by objects)),
- OPT_ULONG(0, window-memory, window_memory_limit,
- N_(limit pack window by memory in addition to object
limit)),
+ { OPTION_ULONG, 0, window-memory, window_memory_limit,
N_(n),
+ N_(limit pack window by memory in addition to object limit),
+ PARSE_OPT_HUMAN_NUMBER },
OPT_INTEGER(0, depth, depth,
N_(maximum length of delta chain allowed in the
resulting pack)),
OPT_BOOL(0, reuse-delta, reuse_delta,
diff --git a/parse-options.c b/parse-options.c
index 7b8d3fa..3a47538 100644
--- a/parse-options.c
+++ b/parse-options.c
@@ -180,6 +180,23 @@ static int get_value(struct parse_opt_ctx_t *p,
return opterror(opt, expects a numerical value,
flags);
return 0;
+ case OPTION_ULONG:
+ if (unset) {
+ *(unsigned long *)opt-value = 0;
+ } else if (opt-flags PARSE_OPT_OPTARG !p-opt) {
+ *(unsigned long *)opt-value = opt-defval;
+ } else if (get_arg(p, opt, flags, arg)) {
+ return -1;
+ } else if (opt-flags PARSE_OPT_HUMAN_NUMBER) {
+ if (!git_parse_ulong(arg, (unsigned long *)opt-value))
+ return opterror(opt, expects a numerical
value, flags);
+ } else {
+ *(unsigned long *)opt-value = strtoul(arg, (char
**)s, 10);
+ if (*s)
+ return opterror(opt, expects a numerical
value, flags);
+ }
+ return 0;
+
default:
die(should not happen, someone must be hit on the forehead);
}
diff --git a/parse-options.h b/parse-options.h
index d670cb9..6a3cce0 100644
--- a/parse-options.h
+++ b/parse-options.h
@@ -17,6 +17,7 @@ enum parse_opt_type {
/* options with arguments (usually) */
OPTION_STRING,
OPTION_INTEGER,
+ OPTION_ULONG,
OPTION_CALLBACK,
OPTION_LOWLEVEL_CALLBACK,
OPTION_FILENAME
@@ -38,6 +39,7 @@ enum parse_opt_option_flags {
PARSE_OPT_LASTARG_DEFAULT = 16,
PARSE_OPT_NODASH = 32,
PARSE_OPT_LITERAL_ARGHELP = 64,
+ PARSE_OPT_HUMAN_NUMBER = 128,
PARSE_OPT_SHELL_EVAL = 256
};
@@ -133,6 +135,7 @@ struct option {
#define OPT_CMDMODE(s, l, v, h, i) { OPTION_CMDMODE, (s), (l), (v), NULL, \
(h), PARSE_OPT_NOARG|PARSE_OPT_NONEG,
NULL, (i) }
#define OPT_INTEGER(s, l, v, h) { OPTION_INTEGER, (s), (l), (v), N_(n),
(h) }
+#define OPT_ULONG(s, l, v, h) { OPTION_ULONG, (s), (l), (v), N_(n),
(h) }
#define OPT_STRING(s
Re: [PATCH 0/6] Make 'git help everyday' work - relnotes
Philip Oakley philipoak...@iee.org writes: Determining which is the current release note is possibly more problematic, which should be when making the documentation. Hmmm Why? You are already aware of the stale-notes section, no? Isn't the top one the latest? -- To unsubscribe from this list: send the line 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/RFC] Makefile: Fix compilation of windows resource file
Ramsay Jones ram...@ramsay1.demon.co.uk writes:
Note that I am merely guessing that short-digit version numbers
are acceptable by now after seeing
https://sourceware.org/ml/binutils/2012-07/msg00199.html
Ah, nice find!
I will test your patch (below) and let you know soon, but it looks
good to me. (I can't test it tonight, unfortunately.)
One thing to note is that I don't know why the existing code dropped
the fourth digit from the maintenance series. The updated one will
give you 1,8,5,3,0 (because I just have a hardcoded ,0 at the
end for no good reason there), and if the missing fourth digit in
the original was a deliberate workaround for this file having an
upper limit of the number of digits (like four), this change will
break it, so if that is the case, you may have to count and stop the
loop early, perhaps like...
diff --git a/gen-version-string.sh b/gen-version-string.sh
new file mode 100755
index 000..00af718
--- /dev/null
+++ b/gen-version-string.sh
@@ -0,0 +1,13 @@
+#!/bin/sh
+
+IFS=.- result=
Add
num_digits=0
here, and...
+for v in $1
+do
+if expr $v : '[0-9][0-9]*$' /dev/null
+then
+result=$result${result:+,}$v
... insert these here.
num_digits=$(( $num_digits + 1 ))
if test $num_digits = 4
then
break
fi
+else
+break
+fi
+done
+echo $result
diff --git a/git.rc b/git.rc
index bce6db9..6f2a8d2 100644
--- a/git.rc
+++ b/git.rc
@@ -1,6 +1,6 @@
1 VERSIONINFO
-FILEVERSION MAJOR,MINOR,PATCH,0
-PRODUCTVERSION MAJOR,MINOR,PATCH,0
+FILEVERSION VERSIONSTRING,0
+PRODUCTVERSION VERSIONSTRING,0
BEGIN
BLOCK StringFileInfo
BEGIN
.
--
To unsubscribe from this list: send the line 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 00/11] git p4 tests and a few bug fixes
Pete Wyckoff p...@padd.com writes: Most of this is work on tests for git p4. Patch 03 is a regression fix, found and narrowed down thanks to much work by Damien Gérard. But it is obscure enough that I'm not proposing it for a maintenance release. There are a couple other behavior fixes, but again, these are quite minor and can wait for the next release. Thanks. I am inclined to say that we should queue this on a fork from 'maint, merge the result to 'master' before 1.9-rc1 and ship the result as part of the upcoming release, and then possibly merging the topic to 1.8.5.x maintenance release after that. This is primarily because I personally do not have p4 expertise to test or properly judge this (iow, you are the area maintainer, the authority), and I somehow have this feeling that parking in 'next' for extended period of time would not give meaningfully larger exposure to the code. What do you think? If you feel uneasy about such a fast-track, I wouldn't push it, though. Pete Wyckoff (11): git p4 test: wildcards are supported git p4 test: ensure p4 symlink parsing works git p4: work around p4 bug that causes empty symlinks git p4 test: explicitly check p4 wildcard delete git p4 test: is_cli_file_writeable succeeds git p4 test: run as user author git p4 test: do not pollute /tmp git p4: handle files with wildcards when doing RCS scrubbing git p4: fix an error message when p4 where fails git p4 test: examine behavior with locked (+l) files git p4 doc: use two-line style for options with multiple spellings Documentation/git-p4.txt | 6 +- git-p4.py | 17 +++-- t/lib-git-p4.sh| 23 +- t/t9802-git-p4-filetype.sh | 83 + t/t9805-git-p4-skip-submit-edit.sh | 6 +- t/t9807-git-p4-submit.sh | 2 +- t/t9809-git-p4-client-view.sh | 16 ++-- t/t9812-git-p4-wildcards.sh| 50 + t/t9813-git-p4-preserve-users.sh | 38 -- t/t9816-git-p4-locked.sh | 145 + 10 files changed, 342 insertions(+), 44 deletions(-) create mode 100755 t/t9816-git-p4-locked.sh -- To unsubscribe from this list: send the line 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 0/6] Make 'git help everyday' work - relnotes
Philip Oakley philipoak...@iee.org writes: I already have a local patch that creates a stalenote.txt file, and includes that in a release-notes(7) man page, but it still leaves the actual release notes in a separate plain text file, linked from the man page, rather than being right at hand, which is what I think readers would expect. Sorry, but I still do not get it. If you have a script that reads git.txt and extracts its stale-notes section to generate the source to be processed into release-notes(7), why can't that script also include the contents of the latest release notes inline into its output? My release notes are _not_ written to be compatible with/processable by AsciiDoc (they are meant to be mere plain text)---perhaps you are wondering if that would make it harder to maintain your script that produces release-notes.txt? Confused... My other question would be to ask how you normally manage the up-issue of the stalenotes, and when you would normally create that section in git(1) as I didn't see any ifdef::stalenotes[] being defined anywhere else. I'm not sure if I am understanding the question right (up-issue?), but it used to be that the preformatted and web-reachable manual pages at k.org were processed with stalenotes defined (which by the way was disabled with adaa3caf Meta/dodoc.sh: adjust to the new layout, 2011-11-15 on the todo branch), and 26cfcfbf Add release notes to the distribution., 2007-02-13 used that facility to prepare something like this: docs/git.html /git-cat-file.html ... docs/vX.Y.Z/git.html docs/vX.Y.Z/git-cat-file.html ... where the latest one lived immediately underneath docs/*, while older ones were in versioned subdirectories. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [ANNOUNCE] Git v1.9-rc0
Javier Domingo Cansino javier...@gmail.com writes: Will there be any change on how tarballs are distributed, taking into account that Google will be shutting down Google Code Downloads section[1][2]? Aside from the obvious we won't be able to use something that is no longer offered? They are not the only download site even now, 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: Anomalous AUTHOR and DOCUMENTATION sections in manpages
Michael Haggerty mhag...@alum.mit.edu writes: I just noticed that there are exactly four Git manpages with an AUTHOR section and five with a DOCUMENTATION section: $ make doc $ grep -nIE -e '^\.SH DOCUMENTATION|AUTHOR' Documentation/*.[0-9] Documentation/git-column.1:80:.SH AUTHOR Documentation/git-for-each-ref.1:272:.SH AUTHOR Documentation/git-for-each-ref.1:275:.SH DOCUMENTATION Documentation/git-http-backend.1:404:.SH AUTHOR Documentation/git-http-backend.1:407:.SH DOCUMENTATION Documentation/git-notes.1:395:.SH AUTHOR Documentation/git-notes.1:398:.SH DOCUMENTATION Documentation/git-remote-ext.1:133:.SH DOCUMENTATION Documentation/git-remote-fd.1:71:.SH DOCUMENTATION These sections are inconsistent with the other manpages and seem superfluous in a project that has, on the one hand, a public history and, on the other hand, hundreds of contributors. We decided at 48bb914e (doc: drop author/documentation sections from most pages, 2011-03-11) to remove them for that exact reason. I'd be happy to see the one in for-each-ref under my name go. 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] Add cross-references between docs for for-each-ref and show-ref
Michael Haggerty mhag...@alum.mit.edu writes: Add cross-references between the manpages for git-for-each-ref(1) and git-show-ref(1). Signed-off-by: Michael Haggerty mhag...@alum.mit.edu --- There is a lot of overlap between the functionality of these two commands. Two differences I most often use (i.e. take advantage of) are: - The former is about showing 0 or more matching refs, and not matching anything is a perfectly normal condition, i.e. $ git for-each-ref refs/heads/no-such exits with status 0. The latter can be used with --verify to check if one ref exists, on the other hand. - The former takes the match-from-left-to-right pattern (similar to the usual pathspec match), while the latter matches from the tail. $ git for-each-ref refs/heads/* ;# only one-level names $ git for-each-ref refs/heads/** ;# catches both master and mh/path-max $ git show-ref master ;# refs/heads/master, refs/remotes/origin/master,... -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [ANNOUNCE] Git v1.9-rc0
Stefan Näwe stefan.na...@atlas-elektronik.com writes: Am 22.01.2014 13:53, schrieb Javier Domingo Cansino: Will there be any change on how tarballs are distributed, taking into account that Google will be shutting down Google Code Downloads section[1][2]? Am I missing something or what's wrong with this: https://github.com/gitster/git/archive/v1.9-rc0.tar.gz or any https://github.com/gitster/git/archive/$TAG.tar.gz ?? Do these consume CPU every time somebody asks for a tarball? That might be considered wrong depending on the view. -- To unsubscribe from this list: send the line 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/RFC] Makefile: Fix compilation of windows resource file
Johannes Sixt j.s...@viscovery.net writes: [Cc Pat, who added git.rc] Am 1/22/2014 0:48, schrieb Junio C Hamano: Ramsay Jones ram...@ramsay1.demon.co.uk writes: Note that I am merely guessing that short-digit version numbers are acceptable by now after seeing https://sourceware.org/ml/binutils/2012-07/msg00199.html Ah, nice find! I will test your patch (below) and let you know soon, but it looks good to me. (I can't test it tonight, unfortunately.) One thing to note is that I don't know why the existing code dropped the fourth digit from the maintenance series. I don't know either. But it does not really matter. When there are 4 digits in the FILEVERSION and PRODUCTVERSION statements, then the user does not see them as-are, but, for example, 1.8.1283 for FILEVERSION 1,8,5,3 (1283 = 5*256+3). Therefore, I think that there is no point in providing 4 numbers, and the patch below should be sufficient. Would that work well when we do 1.9.1, the first maintenance/bugfix release for 1.9? diff --git a/Makefile b/Makefile index b4af1e2..99b2b89 100644 --- a/Makefile +++ b/Makefile @@ -1773,7 +1773,7 @@ $(SCRIPT_LIB) : % : %.sh GIT-SCRIPT-DEFINES git.res: git.rc GIT-VERSION-FILE $(QUIET_RC)$(RC) \ - $(join -DMAJOR= -DMINOR= -DPATCH=, $(wordlist 1,3,$(subst -, ,$(subst ., ,$(GIT_VERSION) \ + $(join -DMAJOR= -DMINOR=, $(wordlist 1,2,$(subst -, ,$(subst ., ,$(GIT_VERSION) \ -DGIT_VERSION=\\\$(GIT_VERSION)\\\ $ -o $@ ifndef NO_PERL diff --git a/git.rc b/git.rc index bce6db9..33aafb7 100644 --- a/git.rc +++ b/git.rc @@ -1,6 +1,6 @@ 1 VERSIONINFO -FILEVERSION MAJOR,MINOR,PATCH,0 -PRODUCTVERSION MAJOR,MINOR,PATCH,0 +FILEVERSION MAJOR,MINOR,0,0 +PRODUCTVERSION MAJOR,MINOR,0,0 BEGIN BLOCK StringFileInfo BEGIN -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
