Upload your Invited Paper until March 20, 2015. Extended Versions of all the Invited papers will be promoted for direct publication in 36 Collaborating ISI/SCI Journals (with Impact Factor from Thomso
Dear Invited Author, Have you uploaded your invited paper for our conferences in Barcelona, Spain 7-9 April 2015? If not, kindly, upload now your INVITED paper via www. inase. org until March 20, 2015. We have collaboration with 36 Journals, all with Impact Factor from ISI (Thomson Reuters) and we can publish the extended version of your paper after the conference. INASE Staff works very hard every day to establish these collaborations by extending the list of our ISI/SCI/SCOPUS collaborating Journals. Write INVITED-by-RENI-DIMITROVA in the Field of Short CV of the Web Form to condider your paper as Invited Paper. If you have participated in our INASE. org conferences in 2013, 2014, 2015 and your paper has not been indexed in ISI and/or SCOPUS, contact me now to resolve the problem now. All the papers of our previous conferences (in the conference version OR in their extended version in Journals are now indexed in ISI/SCI and SCOPUS. Contact me.) The Proceedings will be distributed to you in CD-ROM and Hard-Copy (10 volumes) and will be indexed in SCOPUS , ISI, DBLP, Zentrablatt, ACM, ASM, IET, British library, Scholar Google and all the other major indices. Many Thanks Reni Dimitrova - If you do not want to receive other invitations from me, reply with Subject (with the email address, please): - BLOCK git@vger.kernel.org - -- To unsubscribe from this list: send the line "unsubscribe 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/14] numparse module: systematically tighten up integer parsing
Jeff King writes: > I wondered if we could do away with the radix entirely. Wouldn't we be > asking for base 10 most of the time? Of course, your first few patches > show octal parsing, but I wonder if we should actually have a separate > parse_mode() for that, since that seems to be the general reason for > doing octal parsing. 10644 does not overflow an int, but it is > hardly a reasonable mode. True. > I also wondered if we could get rid of NUM_SIGN in favor of just having > the type imply it (e.g., convert_l would always allow negative numbers, > whereas convert_ul would not). But I suppose there are times when we end > up using an "int" to store an unsigned value for a good reason (e.g., > "-1" is a sentinel value, but we expect only positive values from the > user). So that might be a bad idea. Yeah, there is a reason why ssize_t exists. > IMHO most of the tightening happening here is a good thing, and means we > are more likely to notice mistakes rather than silently doing something > stupid. I do like the general idea of making sure we avoid use of bare atoi() and unchecked use of strtol() and such, and have a set of well defined helper functions to perform the common checks, exactly for the reason you said. -- To unsubscribe from this list: send the line "unsubscribe 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 14/14] diff_opt_parse(): use convert_i() when handling --abbrev=
Michael Haggerty writes:
> die() with an error message if the argument is not a non-negative
> integer. This change tightens up parsing: '+' and '-', leading
> whitespace, and trailing junk are all disallowed now.
>
> Signed-off-by: Michael Haggerty
> ---
> diff.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/diff.c b/diff.c
> index be389ae..1cc5428 100644
> --- a/diff.c
> +++ b/diff.c
> @@ -3830,7 +3830,8 @@ int diff_opt_parse(struct diff_options *options, const
> char **av, int ac)
> else if (!strcmp(arg, "--abbrev"))
> options->abbrev = DEFAULT_ABBREV;
> else if (skip_prefix(arg, "--abbrev=", &arg)) {
> - options->abbrev = strtoul(arg, NULL, 10);
> + if (convert_i(arg, 10, &options->abbrev))
> + die("--abbrev requires an integer argument");
Everybody leading up to this step said "a non-negative integer", but
this one is different?
> if (options->abbrev < MINIMUM_ABBREV)
> options->abbrev = MINIMUM_ABBREV;
> else if (40 < options->abbrev)
--
To unsubscribe from this list: send the line "unsubscribe 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 05/14] handle_revision_opt(): use convert_i() when handling "-"
Michael Haggerty writes:
> This tightens up the parsing a bit:
>
> * Leading whitespace is no longer allowed
> * '+' and '-' are no longer allowed
>
> It also removes the need to check separately that max_count is
> non-negative.
Hmmm, on the surface this sound nice, but I am not sure if it is a
good idea. "git cmd-in-log-family -3 --max-count=-1" would have
been a nice way to flip a max-count given earlier on the command
line back to the default (unlimited) and we are removing it without
giving any escape hatch?
>
> Signed-off-by: Michael Haggerty
> ---
> revision.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/revision.c b/revision.c
> index 25838fc..4908e66 100644
> --- a/revision.c
> +++ b/revision.c
> @@ -1,4 +1,5 @@
> #include "cache.h"
> +#include "numparse.h"
> #include "tag.h"
> #include "blob.h"
> #include "tree.h"
> @@ -1709,8 +1710,7 @@ static int handle_revision_opt(struct rev_info *revs,
> int argc, const char **arg
> return argcount;
> } else if ((*arg == '-') && isdigit(arg[1])) {
> /* accept -, like traditional "head" */
> - if (strtol_i(arg + 1, 10, &revs->max_count) < 0 ||
> - revs->max_count < 0)
> + if (convert_i(arg + 1, 10, &revs->max_count))
> die("'%s': not a non-negative integer", arg + 1);
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 00/14] numparse module: systematically tighten up integer parsing
Michael Haggerty writes:
> Here were my thoughts:
>
> * I wanted to change the interface to look less like
> strtol()/strtoul(), so it seemed appropriate to make the names
> more dissimilar.
One reason I prefer the names in the compat-util is that it makes it
clear that what is converted into what (e.g. "str" to "ul") and what
type the result is stored ("i" or "ui"), and as an added bonus, with
a short-and-sweet "to" it is already so clear we are "converting"
that we do not have to use verbs like "parse_" or "convert_". From
parse_i() and convert_i(), I cannot tell which one lacks the endptr
and parses to the end and which one takes the endptr and tells the
caller where the conversion ended (and it also does not tell us that
they are converting from a string).
> * The functions seemed long enough that they shouldn't be inline,
> and I wanted to put them in their own module rather than put
> them in git-compat-util.h.
I actually agree on this, but that is not a justification why they
have to be named very differently.
Having said that, I would say that it will be futile to discuss this
further, as we seem to agree to disagree.
--
To unsubscribe from this list: send the line "unsubscribe 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/14] numparse module: systematically tighten up integer parsing
Michael Haggerty writes:
> * It allows leading whitespace.
This might be blessing in disguise. Our friends on MacOS may be
relying on that
git cmd --abbrev="$(wc -c * It allows arbitrary trailing characters.
Which is why we have introduced strtoul_ui() and such.
> * It allows a leading sign character ('+' or '-'), even though the
> result is unsigned.
I do not particularly see it a bad thing to accept "--abbrev=+7".
Using unsigned type to accept a width and parsing "--abbrev=-7" into
a large positive integer _is_ a problem, and we may want to fix it,
but I think that is still within the scope of the original "better
strtol/strtoul", and I do not see it as a justification to introduce
a set of functions with completely unfamiliar name.
> * If the string doesn't contain a number at all, it sets its "endptr"
> argument to point at the start of the string but doesn't set errno.
Why is that a problem? A caller that cares is supposed to check
endptr and the string it gave, no? Now, if strtoul_ui() and friends
do not do so, I again think that is still within the scope of the
original "better strtol/strtoul".
> * If the value is larger than fits in an unsigned long, it returns the
> value clamped to the range 0..ULONG_MAX (setting errno to ERANGE).
Ditto.
> * If the value is between -ULONG_MAX and 0, it returns the positive
> integer with the same bit pattern, without setting errno(!) (I can
> hardly think of a scenario where this would be useful.)
Ditto.
> * For base=0 (autodetect base), it allows a base specifier prefix "0x"
> or "0" and parses the number accordingly. For base=16 it also allows
> a "0x" prefix.
Why is it a problem to allow "git cmd --hexval=0x1234", even if "git
cmd --hexval=1234" would suffice?
> When I looked around, I found scores of sites that call atoi(),
> strtoul(), and strtol() carelessly. And it's understandable. Calling
> any of these functions safely is so much work as to be completely
> impractical in day-to-day code.
Yes, these burdens on the caller were exactly why strtoul_ui()
etc. wanted to reduce---and it will be a welcome change to do
necessary checks that are currently not done.
> Please see the docstrings in numparse.h in the first commit for
> detailed API docs. Briefly, there are eight new functions:
>
> parse_{l,ul,i,ui}(const char *s, unsigned int flags,
> T *result, char **endptr);
> convert_{l,ul,i,ui}(const char *s, unsigned int flags, T *result);
>
> The parse_*() functions are for parsing a number from the start of a
> string; the convert_*() functions are for parsing a string that
> consists of a single number.
I am not sure if I follow. Why not unify them into one 4-function
set, with optional endptr that could be NULL?
While we are on the topic of improving number parsing, one thing
that I found somewhat frustrating is that config.c layer knows the
scaling suffix but option parsing layer does not. These functions
should offer an option (via their "flags") to say "I allow scaled
numbers like 2k, 4M, etc.".
--
To unsubscribe from this list: send the line "unsubscribe 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] diff-lib.c: adjust position of i-t-a entries in diff
Junio C Hamano writes: > Ah, wait. > > I suspect that it all cancels out. > ... > Now, as you mentioned, there _may_ be codepaths that uses the same > definition of "what's in the index" as what diff-cache used to take > before your patches, and they may be broken by removing the > invalidation. If we find such codepaths, we should treat their old > behaviour as buggy and fix them, instead of reintroducing the > invalidation and keep their current behaviour, as the new world > order is "i-t-a entries in the index does not yet exist." One potential negative consequence of the new world order I can immediately think of is this. In many operations, we try to be lenient to changes in the working tree as long as the index is clean. "read-tree -m" is the most visible one, where we require that the index and HEAD matches while allowing changes to working tree paths as long as they do not interfere with the paths that are involved in the merge. We need to make sure that the path dir/bar added by "add -N dir/bar", which in the new world order does not count as "index is not clean and there is a difference from HEAD", (1) does not interfere with the mergy operation that wants to touch dir (which _could_ even be expected to be a file) or dir/bar, and (2) is not lost because the mergy operation wants to touch dir or dir/bar, for example. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 00/14] numparse module: systematically tighten up integer parsing
On Tue, Mar 17, 2015 at 05:00:02PM +0100, Michael Haggerty wrote: > My main questions: > > * Do people like the API? My main goal was to make these functions as > painless as possible to use correctly, because there are so many > call sites. > > * Is it too gimmicky to encode the base together with other options in > `flags`? (I think it is worth it to avoid the need for another > parameter, which callers could easily put in the wrong order.) I definitely like the overall direction of this. My first thought was that it does seem like there are a lot of possible options to the functions (and OR-ing the flags with the base does seem weird, though I can't think of a plausible case where it would actually cause errors). Many of those options don't seem used in the example conversions (I'm not clear who would want NUM_SATURATE, for example). I wondered if we could do away with the radix entirely. Wouldn't we be asking for base 10 most of the time? Of course, your first few patches show octal parsing, but I wonder if we should actually have a separate parse_mode() for that, since that seems to be the general reason for doing octal parsing. 10644 does not overflow an int, but it is hardly a reasonable mode. I also wondered if we could get rid of NUM_SIGN in favor of just having the type imply it (e.g., convert_l would always allow negative numbers, whereas convert_ul would not). But I suppose there are times when we end up using an "int" to store an unsigned value for a good reason (e.g., "-1" is a sentinel value, but we expect only positive values from the user). So that might be a bad idea. I notice that you go up to "unsigned long" here for sizes. If we want to use this everywhere, we'll need something larger for parsing off_t values on 32-bit machines (though I would not at all be surprised with the current code if 32-bit machines have a hard time configuring a pack.packSizeLimit above 4G). I wonder how much of the boilerplate in the parse_* functions could be factored out to use a uintmax_t, with the caller just providing the range. That would make it easier to add new types like off_t, and possibly even constrained types (e.g., an integer from 0 to 100). On the other hand, you mentioned to me elsewhere that there may be some bugs in the range-checks of config.c's integer parsing. I suspect they are related to exactly this kind of refactoring, so perhaps writing things out is best. > * Am I making callers too strict? In many cases where a positive > integer is expected (e.g., "--abbrev="), I have been replacing > code like > [...] IMHO most of the tightening happening here is a good thing, and means we are more likely to notice mistakes rather than silently doing something stupid. For sites that currently allow it, I could imagine people using hex notation for some values, though, depending on the context. It looks there aren't many of them ((it is just when the radix is "0", right?). Some of them look to be accidental (does anybody really ask for --threads=0x10?), but others might not be (the pack index-version contains an offset field that might be quite big). Feel free to ignore any or all of that. It is not so much criticism as a dump of thoughts I had while reading the patches. Perhaps you can pick something useful out of that, and perhaps not. :) -Peff -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: git pull & git gc
On Thu, Mar 19, 2015 at 11:29:57AM +0700, Duy Nguyen wrote: > > That omits the "N objects left over" information. Which I think may be > > useful, because otherwise the rule is basically "don't do another gc at > > all for X time units". That's OK for most use, but it has its own corner > > cases. > > True. But saving "N objects left over" in a file also has a corner > case. If the user "prune --expire=now" manually, the next 'gc --auto' > still thinks we have that many leftovers and keeps delaying gc for > some more time. Unless we make 'prune' (or any other commands that > delete leftovers) to also delete this file. Yeah maybe saving this > info in a file will work. I assumed that the user would not run prune manually, but would run "git gc --prune=now". And yeah, definitely any time gc runs, it should update the file (if there are fewer than `gc.auto` objects, I think it could just delete the file). We could also apply that rule any run of "git prune", but my mental model is that "git gc" is the magical porcelain that will do this stuff for you, and "git prune" is the plumbing that users shouldn't need to call themselves. I don't know if that model is shared by users, though. :) -Peff -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: git pull & git gc
On Thu, Mar 19, 2015 at 11:20 AM, Jeff King wrote: > On Thu, Mar 19, 2015 at 11:15:19AM +0700, Duy Nguyen wrote: > >> On Thu, Mar 19, 2015 at 8:27 AM, Jeff King wrote: >> > Keeping a file that says "I ran gc at time T, and there were still N >> > objects left over" is probably the best bet. When the next "gc --auto" >> > runs, if T is recent enough, subtract N from the estimated number of >> > objects. I'm not sure of the right value for "recent enough" there, >> > though. If it is too far back, you will not gc when you could. If it is >> > too close, then you will end up running gc repeatedly, waiting for those >> > objects to leave the expiration window. >> >> And would not be hard to implement either. git-gc is already prepared >> to deal with stale gc.pid, which would stop git-gc for a day or so >> before it deletes gc.pid and starts anyway. All we need to do is check >> at the end of git-gc, if we know for sure the next 'gc --auto' is a >> waste, then leave gc.pid behind. > > That omits the "N objects left over" information. Which I think may be > useful, because otherwise the rule is basically "don't do another gc at > all for X time units". That's OK for most use, but it has its own corner > cases. True. But saving "N objects left over" in a file also has a corner case. If the user "prune --expire=now" manually, the next 'gc --auto' still thinks we have that many leftovers and keeps delaying gc for some more time. Unless we make 'prune' (or any other commands that delete leftovers) to also delete this file. Yeah maybe saving this info in a file will work. > E.g., imagine you are doing an SVN import that does an auto-gc > check every 1000 commits. You have some unreferenced objects in your > repository. After the first 1000 commits, we do a gc, and then say "wow, > still a lot of cruft; let's block gc for a day". Five minutes later, > after another 1000 commits, we run "gc --auto" again. It doesn't run > because of the cruft-check, even though there are a _huge_ number of new > packable objects. > > If the blocker file tells us "7000 extra objects" and we see that there > are 17,000 in the repo, then we know it's still worth doing the gc > (i.e., we know we that we'll probably end up ignoring the 7000 cruft > that didn't get cleaned up last time, but we also know that there are > 10,000 new objects). -- Duy -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: git pull & git gc
On Thu, Mar 19, 2015 at 12:14:53AM -0400, Jeff King wrote: > On Thu, Mar 19, 2015 at 11:01:17AM +0900, Mike Hommey wrote: > > > > I don't think packing the unreachables is a good plan. They just end up > > > accumulating then, and they never expire, because we keep refreshing > > > their mtime at each pack (unless you pack them once and then leave them > > > to expire, but then you end up with a large number of packs). > > > > Note, sometimes I wish unreachables were packed. Recently, I ended up in > > a situation where running gc created something like 3GB of data as per > > du, because I suddenly had something like 600K unreachable objects, each > > of them, as a loose object, taking at least 4K on disk. This made my > > .git take 5GB instead of 2GB. That surely didn't feel like garbage > > collection. > > That's definitely a thing that happens, but it is a bit of a corner > case. It's unusual to have such a large number of unreferenced objects > all at once. > > I don't suppose you happen to remember the details, but would a lower > expiration time (e.g., 1 day or 1 hour) have made all of those objects > go away? Or were they really from some extremely recent event (of > course, "event" here might just have been "I did a full repack right > before rewriting history" which would freshen the mtimes on everything > in the pack). Unfortunately, I don't know the exact details. But yes, I guess a lower expiration time might have helped. > Certainly the "loosening" behavior for unreachable objects has corner > cases like this, and they suck when you hit one. Leaving the objects > packed would be better, but IMHO is not a viable alternative unless > somebody comes up with a plan for segregating the "old" objects in a way > that they actually expire eventually, and don't just keep getting > repacked and freshened over and over. It sure is a corner case, otoh, when it happens, every single git operation calls git gc --auto, which happily spends 5 minutes sucking CPU to end up doing nothing in practice. And add more salt on the injury if you are on battery 6700 loose objects seems easy to reach on a repo with 6M objects... Mike -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: git pull & git gc
On Thu, Mar 19, 2015 at 11:15:19AM +0700, Duy Nguyen wrote: > On Thu, Mar 19, 2015 at 8:27 AM, Jeff King wrote: > > Keeping a file that says "I ran gc at time T, and there were still N > > objects left over" is probably the best bet. When the next "gc --auto" > > runs, if T is recent enough, subtract N from the estimated number of > > objects. I'm not sure of the right value for "recent enough" there, > > though. If it is too far back, you will not gc when you could. If it is > > too close, then you will end up running gc repeatedly, waiting for those > > objects to leave the expiration window. > > And would not be hard to implement either. git-gc is already prepared > to deal with stale gc.pid, which would stop git-gc for a day or so > before it deletes gc.pid and starts anyway. All we need to do is check > at the end of git-gc, if we know for sure the next 'gc --auto' is a > waste, then leave gc.pid behind. That omits the "N objects left over" information. Which I think may be useful, because otherwise the rule is basically "don't do another gc at all for X time units". That's OK for most use, but it has its own corner cases. E.g., imagine you are doing an SVN import that does an auto-gc check every 1000 commits. You have some unreferenced objects in your repository. After the first 1000 commits, we do a gc, and then say "wow, still a lot of cruft; let's block gc for a day". Five minutes later, after another 1000 commits, we run "gc --auto" again. It doesn't run because of the cruft-check, even though there are a _huge_ number of new packable objects. If the blocker file tells us "7000 extra objects" and we see that there are 17,000 in the repo, then we know it's still worth doing the gc (i.e., we know we that we'll probably end up ignoring the 7000 cruft that didn't get cleaned up last time, but we also know that there are 10,000 new objects). -Peff -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: git pull & git gc
On Thu, Mar 19, 2015 at 8:27 AM, Jeff King wrote: > Keeping a file that says "I ran gc at time T, and there were still N > objects left over" is probably the best bet. When the next "gc --auto" > runs, if T is recent enough, subtract N from the estimated number of > objects. I'm not sure of the right value for "recent enough" there, > though. If it is too far back, you will not gc when you could. If it is > too close, then you will end up running gc repeatedly, waiting for those > objects to leave the expiration window. And would not be hard to implement either. git-gc is already prepared to deal with stale gc.pid, which would stop git-gc for a day or so before it deletes gc.pid and starts anyway. All we need to do is check at the end of git-gc, if we know for sure the next 'gc --auto' is a waste, then leave gc.pid behind. -- Duy -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: git pull & git gc
On Thu, Mar 19, 2015 at 11:01:17AM +0900, Mike Hommey wrote: > > I don't think packing the unreachables is a good plan. They just end up > > accumulating then, and they never expire, because we keep refreshing > > their mtime at each pack (unless you pack them once and then leave them > > to expire, but then you end up with a large number of packs). > > Note, sometimes I wish unreachables were packed. Recently, I ended up in > a situation where running gc created something like 3GB of data as per > du, because I suddenly had something like 600K unreachable objects, each > of them, as a loose object, taking at least 4K on disk. This made my > .git take 5GB instead of 2GB. That surely didn't feel like garbage > collection. That's definitely a thing that happens, but it is a bit of a corner case. It's unusual to have such a large number of unreferenced objects all at once. I don't suppose you happen to remember the details, but would a lower expiration time (e.g., 1 day or 1 hour) have made all of those objects go away? Or were they really from some extremely recent event (of course, "event" here might just have been "I did a full repack right before rewriting history" which would freshen the mtimes on everything in the pack). Certainly the "loosening" behavior for unreachable objects has corner cases like this, and they suck when you hit one. Leaving the objects packed would be better, but IMHO is not a viable alternative unless somebody comes up with a plan for segregating the "old" objects in a way that they actually expire eventually, and don't just keep getting repacked and freshened over and over. -Peff -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: git pull & git gc
On Wed, Mar 18, 2015 at 07:27:46PM -0700, Junio C Hamano wrote: > > I guess leaving a bunch of loose objects around longer than necessary > > isn't the end of the world. It wastes space, but it does not actively > > make the rest of git slower (whereas having a large number of packs does > > impact performance). So you could probably make "recent enough" be "T < > > now - gc.pruneExpire / 4" or something. At most we would try to gc 4 > > times before dropping unreachable objects, and for the default period, > > that's only once every couple days. > > We could simply prune unreachables more aggressively, and it would > solve this issue at the root cause, no? Yes, but not too aggressively. You mentioned object archaeology, but my main interest is avoiding corruption. The mtime check is the thing that prevents us from pruning objects being used for an operation-in-progress that has not yet updated a ref. For some long-running operations, like adding files to a commit, we take into account references like a blob being mentioned in the index. But I do not know offhand if there are other long-running operations that would run into problems if we shortened the expiration time drastically. Anything building a temporary index is potentially problematic. But if we assume that operations like that tend to create and reference their objects within a reasonable time period (say, seconds to minutes) then the current default of 2 weeks is absurd for this purpose. For raciness within a single operation, a few seconds is probably enough (e.g., we may write out a commit object and then update the ref a few milliseconds later). The potential for problems is exacerbated by the fact that object `X` may exist in the filesystem with an old mtime, and then a new operation wants to reference it. That's made somewhat better by 33d4221 (write_sha1_file: freshen existing objects, 2014-10-15), as before we could silently turn a file write into a noop. But it's still racy to do: git cat-file -e $commit git update-ref refs/heads/foo $commit as we do not update the mtime for a read-only operation like cat-file (and even if we did, it's still somewhat racy as prune does not atomically check the mtime and remove the file). So I think there's definitely some possible danger with dropping the default prune expiration time. For a long time GitHub ran with it as 1.hour.ago. We definitely saw some oddities and corruption over the years that were apparently caused by over-aggressive pruning and/or raciness. I've fixed a number of bugs, and things did get better as a result. But I could not say whether all such problems are gone. These days we do our regular repacks with "--keep-unreachable" and almost never prune anything. It's also not clear whether GitHub represents anything close to "normal" use. We have a much smaller array of operations that we perform (most objects are either from a push, or from a test-merge between a topic branch and HEAD). But we also have busy repos that are frequently doing gc in the background (especially because we share object storage, so activity on another fork can trigger a gc job that affects a whole repository network). On workstations, I'd guess most git-gc jobs run during a fairly quiescent period. All of which is to say that I don't really know the answer, and there may be dragons. I'd imagine that dropping the default expiration time from 2 weeks to 1 day would probably be fine. A good way to experiment would be for some brave souls to set gc.pruneexpire themselves, run with it for a few weeks or months, and see if anything goes wrong. -Peff -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: git pull & git gc
On Wed, Mar 18, 2015 at 6:27 PM, Jeff King wrote: > > Keeping a file that says "I ran gc at time T, and there were still N > objects left over" is probably the best bet. When the next "gc --auto" > runs, if T is recent enough, subtract N from the estimated number of > objects. I'm not sure of the right value for "recent enough" there, > though. If it is too far back, you will not gc when you could. If it is > too close, then you will end up running gc repeatedly, waiting for those > objects to leave the expiration window. > > I guess leaving a bunch of loose objects around longer than necessary > isn't the end of the world. It wastes space, but it does not actively > make the rest of git slower (whereas having a large number of packs does > impact performance). So you could probably make "recent enough" be "T < > now - gc.pruneExpire / 4" or something. At most we would try to gc 4 > times before dropping unreachable objects, and for the default period, > that's only once every couple days. We could simply prune unreachables more aggressively, and it would solve this issue at the root cause, no? We do keep things reachable from reflogs, so the only thing you are getting by leaving the unreachables around is for an expert to perform some forensic analysis---especially if there are so many loose objects that are all unreachable, nobody sane can go through them one by one and guess correctly if each of them is what they wished they kept if their ancient reflog entry extended a few weeks more. That is, unless there is some tool to analyse the unreachable loose objects, collect them into meaningful islands, and present them in some way that the end user can make sense of, which I do not think exists (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: git pull & git gc
On Wed, Mar 18, 2015 at 09:27:22PM -0400, Jeff King wrote: > On Thu, Mar 19, 2015 at 07:31:48AM +0700, Duy Nguyen wrote: > > > Or we could count/estimate the number of loose objects again after > > repack/prune. Then we can maybe have a way to prevent the next gc that > > we know will not improve the situation anyway. One option is pack > > unreachable objects in the second pack. This would stop the next gc, > > but that would screw prune up because st_mtime info is gone.. Maybe we > > just save a file to tell gc to ignore the number of loose objects > > until after a specific date. > > I don't think packing the unreachables is a good plan. They just end up > accumulating then, and they never expire, because we keep refreshing > their mtime at each pack (unless you pack them once and then leave them > to expire, but then you end up with a large number of packs). Note, sometimes I wish unreachables were packed. Recently, I ended up in a situation where running gc created something like 3GB of data as per du, because I suddenly had something like 600K unreachable objects, each of them, as a loose object, taking at least 4K on disk. This made my .git take 5GB instead of 2GB. That surely didn't feel like garbage collection. Mike -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: git pull & git gc
On Thu, Mar 19, 2015 at 07:31:48AM +0700, Duy Nguyen wrote: > Or we could count/estimate the number of loose objects again after > repack/prune. Then we can maybe have a way to prevent the next gc that > we know will not improve the situation anyway. One option is pack > unreachable objects in the second pack. This would stop the next gc, > but that would screw prune up because st_mtime info is gone.. Maybe we > just save a file to tell gc to ignore the number of loose objects > until after a specific date. I don't think packing the unreachables is a good plan. They just end up accumulating then, and they never expire, because we keep refreshing their mtime at each pack (unless you pack them once and then leave them to expire, but then you end up with a large number of packs). Keeping a file that says "I ran gc at time T, and there were still N objects left over" is probably the best bet. When the next "gc --auto" runs, if T is recent enough, subtract N from the estimated number of objects. I'm not sure of the right value for "recent enough" there, though. If it is too far back, you will not gc when you could. If it is too close, then you will end up running gc repeatedly, waiting for those objects to leave the expiration window. I guess leaving a bunch of loose objects around longer than necessary isn't the end of the world. It wastes space, but it does not actively make the rest of git slower (whereas having a large number of packs does impact performance). So you could probably make "recent enough" be "T < now - gc.pruneExpire / 4" or something. At most we would try to gc 4 times before dropping unreachable objects, and for the default period, that's only once every couple days. -Peff -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: git pull & git gc
On Thu, Mar 19, 2015 at 4:04 AM, Jeff King wrote: > On Wed, Mar 18, 2015 at 02:58:15PM +, John Keeping wrote: > >> On Wed, Mar 18, 2015 at 09:41:59PM +0700, Duy Nguyen wrote: >> > On Wed, Mar 18, 2015 at 9:33 PM, Duy Nguyen wrote: >> > > If not, I made some mistake in analyzing this and we'll start again. >> > >> > I did make one mistake, the first "gc" should have reduced the number >> > of loose objects to zero. Why didn't it.? I'll come back to this >> > tomorrow if nobody finds out first :) >> >> Most likely they are not referenced by anything but are younger than 2 >> weeks. >> >> I saw a similar issue with automatic gc triggering after every operation >> when I did something equivalent to: >> >> git add >> git commit >> git reset --hard HEAD^ >> >> which creates a log of unreachable objects which are not old enough to >> be pruned. > > Yes, this is almost certainly the problem. Though to be pedantic, the > command above will still have a reflog entry, so the objects will be > reachable (and packed). But there are other variants that don't leave > the objects reachable from even reflogs. > > I don't know if there is an easy way around this. Auto-gc's object count > is making the assumption that running the gc will reduce the number of > objects, but obviously it does not always do so. We could do a more > thorough check and find the number of actual packable and prune-able > objects. The "prune-able" part of that is easy; just omit objects from > the count that are newer than 2 weeks. But "packable" is expensive. You > would have to compute reachability by walking from the tips. That can > take tens of seconds on a large repo. Or we could count/estimate the number of loose objects again after repack/prune. Then we can maybe have a way to prevent the next gc that we know will not improve the situation anyway. One option is pack unreachable objects in the second pack. This would stop the next gc, but that would screw prune up because st_mtime info is gone.. Maybe we just save a file to tell gc to ignore the number of loose objects until after a specific date. -- Duy -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Need help deciding between subtree and submodule
On Wed, Mar 18, 2015 at 3:20 AM, Chris Packham wrote: > My $0.02 based on $dayjob > > (disclaimer I've never used subtree) > > On Wed, Mar 18, 2015 at 11:14 AM, Robert Dailey > wrote: >> At my workplace, the team is using Atlassian Stash + git >> >> We have a "Core" library that is our common code between various >> projects. To avoid a single monolithic repository and to allow our >> apps and tools to be modularized into their own repos, I have >> considered moving Core to a subtree or submodule. $DAYJOB has actually tried both... with varying levels of success. As you note, subtree looks wonderful from a user perspective, but behind the scenes, it does have issues. In our case, subtree support was modified into Gerrit, and this became cumbersome and difficult to maintain (which is the reason we eventually dropped support for subtree). Submodules have more of a labor-intensive aspect, but are far more obvious about what actions have been taken (IMHO). Either way, both our developers' needs were satisfied: the code was tracked cleanly, and there wasn't a configuration mismatch where a dependency was able to change versions without implicit direction. > > Our environment is slightly different. Our projects are made up > entirely of submodules, we don't embed submodules within a repo with > actual code (side note: I know syslog-ng does so it might be worth > having a look around there). > > Day to day development is done at the submodule level. A developer > working on a particular feature is generally only touching one repo > notwithstanding a little bit of to-and-fro as they work on the UI > aspects. When changes do touch multiple submodules the pushes can > generally be ordered in a sane manner. Things get a little complicated > when there are interdependent changes, then those pushes require > co-operation between submodule owners. We've done both (all of the above? a hybrid approach?)... We've gone so far to create 30 modules for every conceivable component, then tried to work that way with submodule, and our developers quickly revolted as it became too much of a maintenance burden. The other direction (with hugely monolithic code) is also problematic since the module boundaries become blurred. For us, usually cooperation between modules isn't so difficult, but the problem comes about when attempting to merge the changes. Sometimes, it can take significant effort to ensure conflict-free merges (going so far as to require "merge lock" emails to ask other developers to hold off on merging commits until the change lands completely and the project is stable). > > The key to making this work is our build system. It is the thing that > updates the project repo. After a successful build for all targets (we > hope to add unit/regression tests one day) the submodules sha1s are > updated and a new baseline (to borrow a clearcase term) is published. > Developers can do "git pull && git submodule update" to get the latest > stable baseline, but they can also run git pull in a submodule if they > want to be on the bleeding edge. > >> I tried subtree and this is definitely far more transparent and simple >> to the team (simplicity is very important), however I notice it has >> problems with unnecessary conflicts when you do not do `git subtree >> push` for each `git subtree pull`. This is unnecessary overhead and >> complicates the log graph which I don't like. >> >> Submodule functionally works but it is complicated. We make heavy use >> of pull requests for code reviews (they are required due to company >> policy). Instead of a pull request being atomic and containing any app >> changes + accompanying Core changes, we now need to create two pull >> requests and manage them in proper order. Things also become more >> difficult when branching. All around it just feels like submodule >> would interfere and add more administration overhead on a day to day >> basis, affecting productivity. > > We do have policies around review etc. With submodules it does > sometimes require engaging owners/reviewers from multiple > repositories. Tools like Gerrit can help, particularly where multiple > changes and reviewers are involved. Conflicts are definitely going to be a difficulty with either subtree or submodule (if multiple users could be changing the submodule), but if you have additional tools, such as Gerrit to look out for, submodule is the way to go since subtrees aren't supported within Gerrit. (Other tools may support it better: I'm honestly not sure?) That would be my one word of caution: I don't know how well Stash supports subtree. You are absolutely correct about the difficulty of integrating submodule pull requests taking two steps. This was an issue we worked hard to mitigate here, but at the end of the day, the work is necessary. Basically, we could also use a feature within Gerrit to automatically bring up a specific branch of the "superproject" when the submodule project on a certain branch chang
Re: [PATCH v2] diff-lib.c: adjust position of i-t-a entries in diff
Duy Nguyen writes:
> Right. I missed this but I think this is a less important test
> because I added a new test to make sure "diff --cached" ("git
> status" to be exact) outputs the right thing when i-t-a entries
> are present.
OK.
>> If on the other hand the tests show the same result from these two
>> "diff --cached" and the result is different from what the test
>> expects, that means your patch changed the world order, i.e. an
>> i-t-a entry used to be treated as if it were adding an empty blob to
>> the index but it is now treated as non-existent, then that is a good
>> thing and the only thing we need to update is what the test expects.
>> I am guessing that instead of expecting dir/bar to be shown, it now
>> should expect no output?
>
> Yes, no output.
Good, as that means your changes did not break things, right?
> But still, if I revert the change in cache-tree.c and force write-tree
> to produce valid cache-tree when i-t-a entries are present, this test
> still passes incorrectly.
This is worrysome.
Doesn't that suggest that the diff-cache optimization is disabled
and cache-tree that is marked as valid (because you "revert" the
fix) is not looked at? That is the only explanation that makes
sense to me---you write out a tree omitting i-t-a entries in the
index and update the index without your earlier fix eec3e7e4
(cache-tree: invalidate i-t-a paths after generating trees,
2012-12-16), i.e. marking the cache-tree entries valid. A later
invocation of diff-cache *should* trust that validity and just say
"the tree and the index match" without even digging into the
individual entries, at which point you should see a bogus result.
If that is the case, it would be a performance regression, which may
be already there before your patches or something your patches may
have introduced.
Ah, wait.
I suspect that it all cancels out.
* You "add -N dir/bar". You write-tree. The tree is written as if
dir/bar is not there. cache-tree is updated and it is marked as
valid (because you deliberately broke the earlier fix you made at
eec3e7e4).
* You run "diff-cache". It walks the HEAD and the cache-tree and
finds that HEAD:dir and the cache-tree entry for "dir" records
the same tree object name, and the cache-tree entry is marked
"valid". Hence you declare "no differences".
But for the updated definition of "diff-cache", there indeed is no
difference. The HEAD and the index matches, because dir/bar does
not yet exist.
OK, so I think I can see how the result could work without
invalidating the cache-tree entry that contains i-t-a entries.
It might be even the right thing to do in general. We can view that
invalidation a workaround to achieve the old behaviour of diff-cache,
which used to report that dir/ are different between HEAD and index.
We can even argue that it is the right thing to mark the cache-tree
entry for dir/ as valid, no matter how many i-t-a entries are in
there, as long as writing out the tree for dir/ out of index results
in the same tree as the tree that is stored as HEAD:dir/. And from
that viewpoint, keeping the earlier fix (invalidation code) when
these patches are applied _is_ introducing a performance bug.
Now, as you mentioned, there _may_ be codepaths that uses the same
definition of "what's in the index" as what diff-cache used to take
before your patches, and they may be broken by removing the
invalidation. If we find such codepaths, we should treat their old
behaviour as buggy and fix them, instead of reintroducing the
invalidation and keep their current behaviour, as the new world
order is "i-t-a entries in the index does not yet exist."
Thanks for straightening my confusion out.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH/RFC/GSOC] make git-pull a builtin
Paul Tan writes:
> +/* Global vars since they are used often */
> +static char *head_name;
> +static const char *head_name_short;
> +static unsigned char head_sha1[20];
> +static int head_flags;
> +
> +enum rebase_type {
> + REBASE_FALSE = 0,
> + REBASE_TRUE = 1,
> + REBASE_PRESERVE = 2
> +};
> +
> +/**
> + * Parse rebase config/option value and return corresponding int
> + */
> +static int parse_rebase(const char *arg)
> +{
> + if (!strcmp(arg, "true"))
> + return REBASE_TRUE;
> + else if (!strcmp(arg, "false"))
> + return REBASE_FALSE;
> + else if (!strcmp(arg, "preserve"))
> + return REBASE_PRESERVE;
> + else
> + return -1; /* Invalid value */
> +}
Even though the original does not use bool-or-string-config, we
would want to do the same by doing something like
case (config_maybe_bool()) {
case 0:
return REBASE_FALSE;
case 1:
return REBASE_TRUE;
default:
if (!strcmp(arg, "preserve"))
return REBASE_PRESERVE;
return -1;
}
and then use that in rebase_config_default().
> +
> +/**
> + * Returns default rebase option value
> + */
> +static int rebase_config_default(void)
> +{
> + struct strbuf name = STRBUF_INIT;
> + const char *value = NULL;
> + int boolval;
> +
> + strbuf_addf(&name, "branch.%s.rebase", head_name_short);
> + if (git_config_get_value(name.buf, &value))
> + git_config_get_value("pull.rebase", &value);
What happens when neither is defined?
> + strbuf_release(&name);
> + if (!value)
> + return REBASE_FALSE;
Hmph, are you sure about this? Isn't this "[pull] rebase" that does
not have "= value", in which case pull.rebase is "true"?
You cannot use NULL as the sentinel value to tell that you did not
find either branch.*.rebase nor pull.rebase (in which case you want
to default to 'false'). Either of them can be spelled as an
equal-less true, which you will see as value==NULL, and you want to
take that as 'true'.
const char *value = "false";
...
if (get_value(..., &value))
get_value(..., &value));
strbuf_release(&name);
if (!value)
return REBASE_TRUE;
return parse_rebase(value);
or something along that line, perhaps?
> + boolval = git_config_maybe_bool("pull.rebase", value);
> + if (boolval >= 0)
> + return boolval ? REBASE_TRUE : REBASE_FALSE;
> + else if (value && !strcmp(value, "preserve"))
> + return REBASE_PRESERVE;
Is value something you need to free before returning from this
function?
> +static int parse_opt_recurse_submodules(const struct option *opt, const char
> *arg, int unset)
> +{
> + if (!arg)
> + *(int *)opt->value = unset ? RS_NO : RS_YES;
> + else if (!strcmp(arg, "no"))
> + *(int *)opt->value = RS_NO;
> + else if (!strcmp(arg, "yes"))
> + *(int *)opt->value = RS_YES;
> + else if (!strcmp(arg, "on-demand"))
> + *(int *)opt->value = RS_ON_DEMAND;
> + else
> + return -1;
> + return 0;
I suspect that maybe-bool-or-string comment applies equally here for
the UI consistency.
I'll stop here for now. 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 01/14] numparse: new module for parsing integral numbers
On 03/18/2015 07:27 PM, Eric Sunshine wrote:
> On Tuesday, March 17, 2015, Michael Haggerty wrote:
>> Implement wrappers for strtol() and strtoul() that are safer and more
>> convenient to use.
>>
>> Signed-off-by: Michael Haggerty
>> ---
>> diff --git a/numparse.c b/numparse.c
>> new file mode 100644
>> index 000..90b44ce
>> --- /dev/null
>> +++ b/numparse.c
>> @@ -0,0 +1,180 @@
>> +int parse_l(const char *s, unsigned int flags, long *result, char **endptr)
>> +{
>> + long l;
>> + const char *end;
>> + int err = 0;
>> +
>> + err = parse_precheck(s, &flags);
>> + if (err)
>> + return err;
>> + /*
>> +* Now let strtol() do the heavy lifting:
>> +*/
>
> Perhaps use a /* one-line style comment */ to reduce vertical space
> consumption a bit, thus make it (very slightly) easier to run the eye
> over the code.
Yes, will change.
>> + errno = 0;
>> + l = strtol(s, (char **)&end, flags & NUM_BASE_MASK);
>> + if (errno) {
>> + if (errno == ERANGE) {
>> + if (!(flags & NUM_SATURATE))
>> + return -NUM_SATURATE;
>> + } else {
>> + return -NUM_OTHER_ERROR;
>> + }
>> + }
>
> Would it reduce cognitive load slightly (and reduce vertical space
> consumption) to rephrase the conditionals as:
>
> if (errno == ERANGE && !(flags & NUM_SATURATE))
> return -NUM_SATURATE;
>
> if (errno && errno != ERANGE)
> return -NUM_OTHER_ERROR;
>
> or something similar?
The most common case by far should be that errno is zero. The code as
written only needs one test for that case, whereas your code needs two
tests. I think it is worth compromising on code clarity a tiny bit to
avoid the extra test.
> More below.
>
>> + if (end == s)
>> + return -NUM_NO_DIGITS;
>> +
>> + if (*end && !(flags & NUM_TRAILING))
>> + return -NUM_TRAILING;
>> +
>> + /* Everything was OK */
>> + *result = l;
>> + if (endptr)
>> + *endptr = (char *)end;
>> + return 0;
>> +}
>> diff --git a/numparse.h b/numparse.h
>> new file mode 100644
>> index 000..4de5e10
>> --- /dev/null
>> +++ b/numparse.h
>> @@ -0,0 +1,207 @@
>> +#ifndef NUMPARSE_H
>> +#define NUMPARSE_H
>> +
>> +/*
>> + * Functions for parsing integral numbers.
>> + *
>> + * Examples:
>> + *
>> + * - Convert s into a signed int, interpreting prefix "0x" to mean
>> + * hexadecimal and "0" to mean octal. If the value doesn't fit in an
>> + * unsigned int, set result to INT_MIN or INT_MAX.
>
> Did you mean s/unsigned int/signed int/ ?
Thanks for catching this. I will fix it.
>> + *
>> + * if (convert_i(s, NUM_SLOPPY, &result))
>> + * die("...");
>> + */
>> +
>> +/*
>> + * The lowest 6 bits of flags hold the numerical base that should be
>> + * used to parse the number, 2 <= base <= 36. If base is set to 0,
>> + * then NUM_BASE_SPECIFIER must be set too; in this case, the base is
>> + * detected automatically from the string's prefix.
>
> Does this restriction go against the goal of making these functions
> convenient, even while remaining strict? Is there a strong reason for
> not merely inferring NUM_BASE_SPECIFIER when base is 0? Doing so would
> make it consistent with strto*l() without (I think) introducing any
> ambiguity.
I thought about doing this. If it were possible to eliminate
NUM_BASE_SPECIFIER altogether, then there is no doubt that it would be a
good change. But NUM_BASE_SPECIFIER also has an effect when base==16;
namely, that an "0x" prefix, if present, is consumed. So
parse_i("0xff", 16 | NUM_BASE_SPECIFIER, &result, &endptr)
gives result==255 and endptr==s+4, whereas
parse_i("0xff", 16, &result, &endptr)
gives result==0 and entptr==s+1 (it treats the "x" as the end of the
string).
We could forgo that feature, effectively allowing a base specifier if
and only if base==0. But I didn't want to rule out allowing an optional
base specifier for base==16, in which case NUM_BASE_SPECIFIER can't be
dispensed with entirely.
If you agree with that, then the remaining question is: which policy is
less error-prone? My thinking was that forcing the caller to specify
NUM_BASE_SPECIFIER explicitly when they select base==0 will serve as a
reminder that the two features are intertwined. Because another
imaginable policy (arguably more consistent with the policy for base!=0)
would be that
convert_i(s, 0, &result)
, because it *doesn't* specify NUM_BASE_SPECIFIER, doesn't allow a base
prefix, and therefore indirectly only allows base-10 numbers.
But I don't feel strongly about this.
>> + */
>> +/*
>> + * Number parsing functions:
>> + *
>> + * The following functions parse a number (long, unsigned long, int,
>> + * or unsigned int respectively) from the front of s, storing the
>> + * value to *result and storing a pointer to the first character afte
RE: Git with Lader logic
On March 18, 2015 6:29 PM Doug Kelly wrote: > On Wed, Mar 18, 2015 at 2:53 PM, Randall S. Becker > wrote: > > On March 17, 2015 7:34 PM, Bharat Suvarna wrote: > >> I am trying to find a way of using version control on PLC programmers > >> like > > Allen > >> Bradley PLC. I can't find a way of this. > >> > >> Could you please give me an idea if it will work with Plc programs. > >> Which > > are > >> basically Ladder logic. > > > > Many PLC programs either store their project code in XML, L5K or L5X > > (for example), TXT, CSV, or some other text format or can import and > > export to text forms. If you have a directory structure that > > represents your project, and the file formats have reasonable line > > separators so that diffs can be done easily, git very likely would > > work out for you. You do not have to have the local .git repository in > > the same directory as your working area if your tool has issues with > > that or .gitignore. You may want to use a GUI client to manage your > > local repository and handle the commit/push/pull/merge/rebase > > functions as I expect whatever PLC system you are using does not have git > built-in. > > > > To store binary PLC data natively, which some tools use, I expect that > > those who are better at git-conjuring than I, could provide guidance > > on how to automate binary diffs for your tool's particular file format. > > The one thing I find interesting about RSLogix in general (caveat: I only have > very limited experience with RSLogix 500 / 5000; if I do anything nowadays, > it's > in the micro series using RSLogix Micro Starter Lite)... they do have some > limited notion of version control inside the application itself, though it > seems > rudimentary to me. > This could prove to be helpful or extremely annoying, since even when I > connect to a PLC and go online, just to reset the RTC, it still prompts me to > save > again (even though nothing changed, other than the processor state). > > You may also find this link on stackexchange helpful: > http://programmers.stackexchange.com/questions/102487/are-there- > realistic-useful-solutions-for-source-control-for-ladder-logic-program > > As Randall noted, L5K is just text, and RSLogix 5000 uses it, according to > this > post. It may work okay. A really good reason to use git instead of some other systems is that new versions of files are determined by SHA signatures (real differences) rather than straight timestamps. So saving a file that has not changed will not force a new version - unlike some systems that shall remain nameless. -- To unsubscribe from this list: send the line "unsubscribe 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 with Lader logic
On Wed, Mar 18, 2015 at 2:53 PM, Randall S. Becker wrote: > On March 17, 2015 7:34 PM, Bharat Suvarna wrote: >> I am trying to find a way of using version control on PLC programmers like > Allen >> Bradley PLC. I can't find a way of this. >> >> Could you please give me an idea if it will work with Plc programs. Which > are >> basically Ladder logic. > > Many PLC programs either store their project code in XML, L5K or L5X (for > example), TXT, CSV, or some other text format or can import and export to > text forms. If you have a directory structure that represents your project, > and the file formats have reasonable line separators so that diffs can be > done easily, git very likely would work out for you. You do not have to have > the local .git repository in the same directory as your working area if your > tool has issues with that or .gitignore. You may want to use a GUI client to > manage your local repository and handle the commit/push/pull/merge/rebase > functions as I expect whatever PLC system you are using does not have git > built-in. > > To store binary PLC data natively, which some tools use, I expect that those > who are better at git-conjuring than I, could provide guidance on how to > automate binary diffs for your tool's particular file format. The one thing I find interesting about RSLogix in general (caveat: I only have very limited experience with RSLogix 500 / 5000; if I do anything nowadays, it's in the micro series using RSLogix Micro Starter Lite)... they do have some limited notion of version control inside the application itself, though it seems rudimentary to me. This could prove to be helpful or extremely annoying, since even when I connect to a PLC and go online, just to reset the RTC, it still prompts me to save again (even though nothing changed, other than the processor state). You may also find this link on stackexchange helpful: http://programmers.stackexchange.com/questions/102487/are-there-realistic-useful-solutions-for-source-control-for-ladder-logic-program As Randall noted, L5K is just text, and RSLogix 5000 uses it, according to this post. It may work okay. --Doug -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re:
On Wed, Mar 18, 2015 at 2:33 PM, Junio C Hamano wrote: > What does the Icon^M try to catch, exactly? Is it a file? Is it a directory? > Is it "anything that begins with Icon^M"? It seems to be a special hidden file on Macs for UI convenience. > On Apr 25, 2005, at 6:21 AM, Peter N. Lundblad wrote: > > The Icon^M file in a directory gives that directory a custom icon in > the Finder. They are a holdover from MacOS 9 but there are still a lot > of them out there. The "new" OS X format for icons are .icns files but > I'm not sure if you can do custom file directory icons with them (you > probably can, I just haven't found the docs 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:
On Wed, Mar 18, 2015 at 2:28 PM, Jeff King wrote: > On Wed, Mar 18, 2015 at 05:17:16PM -0400, Jeff King wrote: > >> [1] The double-CR fix works because we strip a single CR from the end of >> the line (as a convenience for CRLF systems), and then the remaining >> CR is syntactically significant. But I am surprised that quoting >> like: >> >> printf '"Icon\r"' >.gitignore >> >> does not seem to work. > > Answering myself: we don't do quoting like this in .gitignore. We allow > backslashing to escape particular characters, like trailing whitespace. > So in theory: > > Icon\\r > > (where "\r" is a literal CR) would work. But it doesn't, because the > CRLF chomping happens separately, and CR is therefore a special case. I > suspect you could not .gitignore a file with a literal LF in it at all > (and I equally suspect that nobody cares in practice). What does the Icon^M try to catch, exactly? Is it a file? Is it a directory? Is it "anything that begins with Icon^M"? I am wondering if we need an opposite of '/' prefix in the .gitignore file to say "the pattern does not match a directory, only a file". -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
per-repository and per-worktree config variables
On Sun, Feb 08, 2015 at 09:36:43AM -0800, Jens Lehmann wrote: > I wonder if it's worth all the hassle to invent new names. Wouldn't > it be much better to just keep a list of per-worktree configuration > value names and use that inside the config code to decide where to > find them for multiple work trees. That would also work easily for > stuff like EOL-config and would push the complexity in the config > machinery and not onto the user. I actually thought about the same, and now tend to think that most of config variables make sense to be per-worktree in some cases. Only few variable must always be per repository. I tried to summarize the variables which now (in current pu) should be common, also listed all the rest so somebody could scan through the list and spot anything I could miss. PS: some variables are with older case, they were mostly collected before I got aware of their rename. -- Max Strictly common only Distributiveness A repository should behave same way regartdless of which worktree is being accessed. core.gitProxy:: core.askpass:: credential.helper:: credential.useHttpPath:: credential.username:: credential..*:: fetch.fsckObjects:: fetch.prune:: gitcvs.*:: gitweb.*:: gui.pruneduringfetch:: http.*:: push.followTags:: receive.*:: remote..*:: remotes.:: transfer.fsckObjects:: transfer.hiderefs:: uploadarchive.allowUnreachable:: uploadpack.*:: url..insteadOf:: url..pushInsteadOf:: imap.*:: Repository storage options ^^ There should be same options of purging, compression etc. core.compression:: core.loosecompression:: core.packedGitWindowSize:: core.packedGitLimit:: core.deltaBaseCacheLimit:: core.bigFileThreshold:: core.fsyncobjectfiles:: core.createObject:: core.logAllRefUpdates:: core.protectHFS:: core.protectNTFS:: fetch.unpackLimit:: fsck.skipList:: fsck.error:: fsck.warn:: fsck.ignore:: gc.* pack.* repack.* Branch tracking settings Since branch are shared, they should be tracked same way regardless of where are they updated. branch..remote:: branch..pushremote:: branch..merge:: branch..mergeoptions:: branch..rebase:: Notes handling ^^ I don't know much about notes, and not sure about these ones, but notes looks like a part of common history, and the options affect all of them. notes.rewrite.:: notes.rewriteMode:: notes.rewriteRef:: Misc core.preferSymlinkRefs:: As description says, useful mostly for older scripts, which definitely not going to understand the new format. Probably incompatible with multiple worktrees at all. core.repositoryFormatVersion:: Looks like this is not used. init.templatedir:: This makes sense only as a global option. Prefer common ~ Technically can be used per-worktree, but does not seem to have much sense. Many of them are are actualy expected to be global. advice.*:: core.ignorecase:: core.precomposeunicode:: core.trustctime:: core.checkstat:: core.quotepath:: core.symlinks:: core.ignoreStat:: core.warnAmbiguousRefs:: core.attributesfile:: This should be common if it names tracked file. core.editor:: core.commentchar:: sequence.editor:: core.pager:: core.preloadindex:: alias.*:: browser..path:: color.branch:: color.diff:: color.diff.:: color.decorate.:: color.grep:: color.grep.:: color.interactive:: color.interactive.:: color.pager:: color.showbranch:: color.status:: color.status.:: color.ui:: color.branch.:: column.*:: commit.cleanup:: commit.gpgsign:: commit.status:: commit.template:: difftool.*:: fetch.recurseSubmodules:: Since submodules can differ, this also can. gui.commitmsgwidth:: gui.diffcontext:: gui.encoding:: While in theory possible to use per-workree value, probably this is intended to correspond to the actual data in repository. Not sure though. gui.trustmtime:: gui.spellingdictionary:: help.* i18n.commitEncoding:: i18n.logOutputEncoding:: index.version:: instaweb.*:: mailmap.*:: man.*:: mergetool.*:: pager.*:: sendemail.*:: tar.umask:: user.email:: user.name:: user.signingkey:: web.browser:: mailinfo.scissors:: Ok to be per-worktree ~ core.fileMode:: core.eol:: core.safecrlf:: core.autocrlf:: core.bare:: core.worktree:: core.excludesfile:: core.whitespace:: core.notesRef:: core.sparseCheckout:: core.abbrev:: add.ignore-errors:: add.ignoreErrors:: am.keepcr:: apply.ignorewhitespace:: apply.whitespace:: branch.autosetupmerge:: branch.autosetuprebase:: clean.requireForce:: format.*:: filter.*:: For some worktrees it may be desired to have them unsmudged. grep.*:: gpg.program:: gui.displayuntracked:: gui.matchtrackingbranch:: gui.newbranchtemplate:: gui.fastcopyblame:: gui.copyblamethreshold:: gui.blamehistoryctx:: guitool.* interactive.singlekey:: log.*:: notes.displayRef:: pretty.:: pull.ff:: pull.rebase:: pull.octopus:: pull.twohead:: push.default:: rebase.stat:: rebase.autosquash:: rebase.autostash:: remote.
Re:
On Wed, Mar 18, 2015 at 05:17:16PM -0400, Jeff King wrote: > [1] The double-CR fix works because we strip a single CR from the end of > the line (as a convenience for CRLF systems), and then the remaining > CR is syntactically significant. But I am surprised that quoting > like: > > printf '"Icon\r"' >.gitignore > > does not seem to work. Answering myself: we don't do quoting like this in .gitignore. We allow backslashing to escape particular characters, like trailing whitespace. So in theory: Icon\\r (where "\r" is a literal CR) would work. But it doesn't, because the CRLF chomping happens separately, and CR is therefore a special case. I suspect you could not .gitignore a file with a literal LF in it at all (and I equally suspect that nobody cares in practice). -Peff -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v4 0/2] path: implement common_dir handling in git_path_submodule()
After http://thread.gmane.org/gmane.comp.version-control.git/261990 the only thing which did not enter the serie is these 2 pathes. Should be applied over the patches by link, or ecf2ff6ace6a1cc3d55b6f917be5452b5fb0c21b in current pu. Max Kirillov (2): submodule refactor: use git_path_submodule() in add_submodule_odb() path: implement common_dir handling in git_path_submodule() cache.h | 1 + path.c | 24 setup.c | 17 - submodule.c | 28 ++-- t/t7410-submodule-checkout-to.sh | 10 ++ 5 files changed, 53 insertions(+), 27 deletions(-) -- 2.1.1.391.g7a54a76 -- To unsubscribe from this list: send the line "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 v4 1/2] submodule refactor: use git_path_submodule() in add_submodule_odb()
Signed-off-by: Max Kirillov
---
submodule.c | 28 ++--
1 file changed, 10 insertions(+), 18 deletions(-)
diff --git a/submodule.c b/submodule.c
index 34094f5..4aad3d4 100644
--- a/submodule.c
+++ b/submodule.c
@@ -122,43 +122,35 @@ void stage_updated_gitmodules(void)
static int add_submodule_odb(const char *path)
{
- struct strbuf objects_directory = STRBUF_INIT;
struct alternate_object_database *alt_odb;
+ const char* objects_directory;
int ret = 0;
- const char *git_dir;
- strbuf_addf(&objects_directory, "%s/.git", path);
- git_dir = read_gitfile(objects_directory.buf);
- if (git_dir) {
- strbuf_reset(&objects_directory);
- strbuf_addstr(&objects_directory, git_dir);
- }
- strbuf_addstr(&objects_directory, "/objects/");
- if (!is_directory(objects_directory.buf)) {
+ objects_directory = git_path_submodule(path, "objects/");
+ if (!is_directory(objects_directory)) {
ret = -1;
goto done;
}
+
/* avoid adding it twice */
for (alt_odb = alt_odb_list; alt_odb; alt_odb = alt_odb->next)
- if (alt_odb->name - alt_odb->base == objects_directory.len &&
- !strncmp(alt_odb->base, objects_directory.buf,
- objects_directory.len))
+ if (alt_odb->name - alt_odb->base == strlen(objects_directory)
&&
+ !strcmp(alt_odb->base, objects_directory))
goto done;
- alt_odb = xmalloc(objects_directory.len + 42 + sizeof(*alt_odb));
+ alt_odb = xmalloc(strlen(objects_directory) + 42 + sizeof(*alt_odb));
alt_odb->next = alt_odb_list;
- strcpy(alt_odb->base, objects_directory.buf);
- alt_odb->name = alt_odb->base + objects_directory.len;
+ strcpy(alt_odb->base, objects_directory);
+ alt_odb->name = alt_odb->base + strlen(objects_directory);
alt_odb->name[2] = '/';
alt_odb->name[40] = '\0';
alt_odb->name[41] = '\0';
alt_odb_list = alt_odb;
/* add possible alternates from the submodule */
- read_info_alternates(objects_directory.buf, 0);
+ read_info_alternates(objects_directory, 0);
prepare_alt_odb();
done:
- strbuf_release(&objects_directory);
return ret;
}
--
2.1.1.391.g7a54a76
--
To unsubscribe from this list: send the line "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 v4 2/2] path: implement common_dir handling in git_path_submodule()
This allows making submodules a linked workdirs.
Same as for .git, but ignores the GIT_COMMON_DIR environment variable,
because it would mean common directory for the parent repository and
does not make sense for submodule.
Also add test for functionality which uses this call.
Signed-off-by: Max Kirillov
---
cache.h | 1 +
path.c | 24
setup.c | 17 -
t/t7410-submodule-checkout-to.sh | 10 ++
4 files changed, 43 insertions(+), 9 deletions(-)
diff --git a/cache.h b/cache.h
index 670e861..4cd03f0 100644
--- a/cache.h
+++ b/cache.h
@@ -437,6 +437,7 @@ extern char *get_object_directory(void);
extern char *get_index_file(void);
extern char *get_graft_file(void);
extern int set_git_dir(const char *path);
+extern int get_common_dir_noenv(struct strbuf *sb, const char *gitdir);
extern int get_common_dir(struct strbuf *sb, const char *gitdir);
extern const char *get_git_namespace(void);
extern const char *strip_namespace(const char *namespaced_ref);
diff --git a/path.c b/path.c
index a5c51a3..78f718f 100644
--- a/path.c
+++ b/path.c
@@ -98,7 +98,7 @@ static const char *common_list[] = {
NULL
};
-static void update_common_dir(struct strbuf *buf, int git_dir_len)
+static void update_common_dir(struct strbuf *buf, int git_dir_len, const char*
common_dir)
{
char *base = buf->buf + git_dir_len;
const char **p;
@@ -115,12 +115,17 @@ static void update_common_dir(struct strbuf *buf, int
git_dir_len)
path++;
is_dir = 1;
}
+
+ if (!common_dir) {
+ common_dir = get_git_common_dir();
+ }
+
if (is_dir && dir_prefix(base, path)) {
- replace_dir(buf, git_dir_len, get_git_common_dir());
+ replace_dir(buf, git_dir_len, common_dir);
return;
}
if (!is_dir && !strcmp(base, path)) {
- replace_dir(buf, git_dir_len, get_git_common_dir());
+ replace_dir(buf, git_dir_len, common_dir);
return;
}
}
@@ -160,7 +165,7 @@ static void adjust_git_path(struct strbuf *buf, int
git_dir_len)
else if (git_db_env && dir_prefix(base, "objects"))
replace_dir(buf, git_dir_len + 7, get_object_directory());
else if (git_common_dir_env)
- update_common_dir(buf, git_dir_len);
+ update_common_dir(buf, git_dir_len, NULL);
}
static void do_git_path(struct strbuf *buf, const char *fmt, va_list args)
@@ -256,6 +261,8 @@ const char *git_path_submodule(const char *path, const char
*fmt, ...)
{
struct strbuf *buf = get_pathname();
const char *git_dir;
+ struct strbuf git_submodule_common_dir = STRBUF_INIT;
+ struct strbuf git_submodule_dir = STRBUF_INIT;
va_list args;
strbuf_addstr(buf, path);
@@ -269,11 +276,20 @@ const char *git_path_submodule(const char *path, const
char *fmt, ...)
strbuf_addstr(buf, git_dir);
}
strbuf_addch(buf, '/');
+ strbuf_addstr(&git_submodule_dir, buf->buf);
va_start(args, fmt);
strbuf_vaddf(buf, fmt, args);
va_end(args);
+
+ if (get_common_dir_noenv(&git_submodule_common_dir,
git_submodule_dir.buf)) {
+ update_common_dir(buf, git_submodule_dir.len,
git_submodule_common_dir.buf);
+ }
+
strbuf_cleanup_path(buf);
+
+ strbuf_release(&git_submodule_dir);
+ strbuf_release(&git_submodule_common_dir);
return buf->buf;
}
diff --git a/setup.c b/setup.c
index fb61860..ffda622 100644
--- a/setup.c
+++ b/setup.c
@@ -226,14 +226,21 @@ void verify_non_filename(const char *prefix, const char
*arg)
int get_common_dir(struct strbuf *sb, const char *gitdir)
{
+ const char *git_env_common_dir = getenv(GIT_COMMON_DIR_ENVIRONMENT);
+ if (git_env_common_dir) {
+ strbuf_addstr(sb, git_env_common_dir);
+ return 1;
+ } else {
+ return get_common_dir_noenv(sb, gitdir);
+ }
+}
+
+int get_common_dir_noenv(struct strbuf *sb, const char *gitdir)
+{
struct strbuf data = STRBUF_INIT;
struct strbuf path = STRBUF_INIT;
- const char *git_common_dir = getenv(GIT_COMMON_DIR_ENVIRONMENT);
int ret = 0;
- if (git_common_dir) {
- strbuf_addstr(sb, git_common_dir);
- return 1;
- }
+
strbuf_addf(&path, "%s/commondir", gitdir);
if (file_exists(path.buf)) {
if (strbuf_read_file(&data, path.buf, 0) <= 0)
diff --git a/t/t7410-submodule-checkout-to.sh b/t/t7410-submodule-checkout-to.sh
index 8f30aed..b43391a 100755
--- a/t/t7410-submodule-checkout-to.sh
+++ b/t/t7410-submodule-checkout-to.sh
@@ -47,4 +47,
Re:
On Wed, Mar 18, 2015 at 02:06:22PM -0700, Stefan Beller wrote: > > Where did you get that file from? We need to find whoever is > > responsible and notify them so that these users who are having > > the issue will be helped. > > Given that this is part of https://github.com/github/gitignore > which is the official collection of .gitignore files from Github, > the company, we could ask Jeff or Michael if it is urgent. > The actual fix being merged 3 years ago makes me belief > it is not urgent though. It looks like the fix they have in that repo does the right thing[1], but for reference, you are much more likely to get results by creating an issue or PR on that repository, rather than asking me. -Peff [1] The double-CR fix works because we strip a single CR from the end of the line (as a convenience for CRLF systems), and then the remaining CR is syntactically significant. But I am surprised that quoting like: printf '"Icon\r"' >.gitignore does not seem to work. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 4/4] t0302: test credential-store support for XDG_CONFIG_HOME
On Wed, Mar 18, 2015 at 12:41 PM, Matthieu Moy wrote: > Similarly to the "merge doc and code", I personally prefer seeing code > and tests in the same patch. In this case, the patch introducing the tests is already quite long and intricate, almost to the point of being a burden to review. Combining the patches would likely push it over the edge. I'd elect to keep them separate. > Actually, my preference goes for a first patch that introduces the tests > with test_expect_failure for things that are not yet implemented (and > you can check that tests do not pass yet before you code), and then the > patch introducing the feature doing > > -test_expect_failure > +test_expect_success > > which documents quite clearly and concisely that you just made the > feature work. I also tend to favor adding "failure" tests which are flipped to "success" when appropriate, however, as this is an entirely new feature, this approach may be unsuitable (and perhaps overkill). Generally speaking, these new tests don't really make sense without the feature; and, more specifically, due to their nature, several of them will pass even without the feature implemented. As such, the current patch organization seems reasonable. -- To unsubscribe from this list: send the line "unsubscribe 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 pull & git gc
On Wed, Mar 18, 2015 at 03:48:42PM +0100, Дилян Палаузов wrote: > #ls .git/objects/17/* | wc -l > 30 > > 30 * 256 = 7 680 > 6 700 > > And now? Do I have to run git gc --aggressive ? No, aggressive just controls the time we spend on repacking. If the guess is correct that the objects are kept because they are unreachable but "recent", then shortening the prune expiration time would get rid of them. E.g., "git gc --prune=1.hour.ago". That does not solve the underlying problem discussed elsewhere in the thread, but it would make this particular instance of it go away. :) -Peff -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re:
On Wed, Mar 18, 2015 at 1:45 PM, Junio C Hamano wrote: > On Wed, Mar 18, 2015 at 1:33 PM, Alessandro Zanardi > wrote: >> Here are other sources describing the issue: >> >> http://stackoverflow.com/questions/21109672/git-ignoring-icon-files-because-of-icon-rule >> >> http://blog.bitfluent.com/post/173740409/ignoring-icon-in-gitignore >> >> Sorry to bother the Git development team with such a minor issue, I just >> wanted to know if it's already been fixed. > > I do not ship your ~/.gitignore_global file as part of my software, > so the problem is not mine to fix in the first place ;-) Maybe this can be understood as a critique on the .gitignore format specifier for paths. (Maybe not, I dunno) So the `gitignore` script/executable which would generate your .gitignore file for you introduced a bug to also ignore files in "Icons/" when all you wanted to have is ignoring the file "Icon\r\r" (Mind that \r is an escape character to explain the meaning, gitignore cannot understand it. Sometimes it also shows up as ^M^M depending on operating system/editor used.) But as you can see, there have been several attempts at fixing it right and https://github.com/github/gitignore/pull/334 eventually got the right fix. (it was merged 2012, which has been a while now), maybe you'd want to use a new version of this gitignore script to regenerate your gitignore? > > Where did you get that file from? We need to find whoever is > responsible and notify them so that these users who are having > the issue will be helped. Given that this is part of https://github.com/github/gitignore which is the official collection of .gitignore files from Github, the company, we could ask Jeff or Michael if it is urgent. The actual fix being merged 3 years ago makes me belief it is not urgent though. Thanks, Stefan -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: git pull & git gc
On Wed, Mar 18, 2015 at 02:58:15PM +, John Keeping wrote: > On Wed, Mar 18, 2015 at 09:41:59PM +0700, Duy Nguyen wrote: > > On Wed, Mar 18, 2015 at 9:33 PM, Duy Nguyen wrote: > > > If not, I made some mistake in analyzing this and we'll start again. > > > > I did make one mistake, the first "gc" should have reduced the number > > of loose objects to zero. Why didn't it.? I'll come back to this > > tomorrow if nobody finds out first :) > > Most likely they are not referenced by anything but are younger than 2 > weeks. > > I saw a similar issue with automatic gc triggering after every operation > when I did something equivalent to: > > git add > git commit > git reset --hard HEAD^ > > which creates a log of unreachable objects which are not old enough to > be pruned. Yes, this is almost certainly the problem. Though to be pedantic, the command above will still have a reflog entry, so the objects will be reachable (and packed). But there are other variants that don't leave the objects reachable from even reflogs. I don't know if there is an easy way around this. Auto-gc's object count is making the assumption that running the gc will reduce the number of objects, but obviously it does not always do so. We could do a more thorough check and find the number of actual packable and prune-able objects. The "prune-able" part of that is easy; just omit objects from the count that are newer than 2 weeks. But "packable" is expensive. You would have to compute reachability by walking from the tips. That can take tens of seconds on a large repo. You could perhaps cut off the walk early when you hit a packed commit (this does not strictly imply that all of the related objects are packed, but it would be good enough for a heuristic). But even that is probably too expensive for "gc --auto". -Peff PS Note that in git v2.2.0 and up, prune will leave not only "recent" unreachable objects, but older objects which are reachable from those recent ones (so that we keep or prune whole chunks of history, rather than dropping part and leaving the rest broken). Technically this exacerbates the problem (we keep more objects), though I doubt it makes much difference in practice (most chunks of history were created at similar times, so the mtimes of the whole chunk will be close together). -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re:
On Wed, Mar 18, 2015 at 1:33 PM, Alessandro Zanardi wrote: > Here are other sources describing the issue: > > http://stackoverflow.com/questions/21109672/git-ignoring-icon-files-because-of-icon-rule > > http://blog.bitfluent.com/post/173740409/ignoring-icon-in-gitignore > > Sorry to bother the Git development team with such a minor issue, I just > wanted to know if it's already been fixed. I do not ship your ~/.gitignore_global file as part of my software, so the problem is not mine to fix in the first place ;-) Where did you get that file from? We need to find whoever is responsible and notify them so that these users who are having the issue will be helped. 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] t5312: test object deletion code paths in a corrupted repository
Am 17.03.2015 um 19:55 schrieb Jeff King: + echo $bogus >.git/refs/heads/bogus..name && ... I assumed the final "." in your example wasn't significant (it is not to git), but let me know if I've run afoul of another weird restriction. :) It was actually deliberate (with intents too complicated to explain), but it turns out not to be required. Your updated test case is good. -- Hannes -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: Git with Lader logic
On March 17, 2015 7:34 PM, Bharat Suvarna wrote: > I am trying to find a way of using version control on PLC programmers like Allen > Bradley PLC. I can't find a way of this. > > Could you please give me an idea if it will work with Plc programs. Which are > basically Ladder logic. Many PLC programs either store their project code in XML, L5K or L5X (for example), TXT, CSV, or some other text format or can import and export to text forms. If you have a directory structure that represents your project, and the file formats have reasonable line separators so that diffs can be done easily, git very likely would work out for you. You do not have to have the local .git repository in the same directory as your working area if your tool has issues with that or .gitignore. You may want to use a GUI client to manage your local repository and handle the commit/push/pull/merge/rebase functions as I expect whatever PLC system you are using does not have git built-in. To store binary PLC data natively, which some tools use, I expect that those who are better at git-conjuring than I, could provide guidance on how to automate binary diffs for your tool's particular file format. Cheers, Randall -- Brief whoami: NonStop&UNIX developer since approximately UNIX(421664400)/NonStop(2112884442) -- In my real life, I talk too much. -- To unsubscribe from this list: send the line "unsubscribe 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 2/2] sha1_file: refactor sha1_file.c to support 'cat-file --literally'
On 03/18/2015 01:40 AM, Junio C Hamano wrote:
Karthik Nayak writes:
Subject: Re: [PATCH v4 2/2] sha1_file: refactor sha1_file.c to support
'cat-file --literally'
Modify sha1_loose_object_info() to support 'cat-file --literally'
by accepting flags and also make changes to copy the type to
object_info::typename.
It would be more descriptive to mention the central point on the
title regarding what it takes to "support cat-file --literally".
For example:
sha1_file.c: support reading from a loose object of a bogus type
Update sha1_loose_object_info() to optionally allow it to read
from a loose object file of unknown/bogus type; as the function
usually returns the type of the object it read in the form of enum
for known types, add an optional "typename" field to receive the
name of the type in textual form.
By the way, I think your 1/2 would not even compile unless you have
this change; the patches in these two patch series must be swapped,
no?
Noted. Yes it wouldn't. I thought both would be applied together and
didn't give it much thought, but yeah, I should pay more attention to that.
diff --git a/sha1_file.c b/sha1_file.c
index 69a60ec..e31e9e2 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -1564,6 +1564,36 @@ int unpack_sha1_header(git_zstream *stream, unsigned
char *map, unsigned long ma
return git_inflate(stream, 0);
}
+static int unpack_sha1_header_literally(git_zstream *stream, unsigned char
*map,
+ unsigned long mapsize,
+ struct strbuf *header)
+{
+ unsigned char buffer[32], *cp;
+ unsigned long bufsiz = sizeof(buffer);
+ int status;
+
+ /* Get the data stream */
+ memset(stream, 0, sizeof(*stream));
+ stream->next_in = map;
+ stream->avail_in = mapsize;
+ stream->next_out = buffer;
+ stream->avail_out = bufsiz;
+
+ git_inflate_init(stream);
+
+ do {
+ status = git_inflate(stream, 0);
+ strbuf_add(header, buffer, stream->next_out - buffer);
+ for (cp = buffer; cp < stream->next_out; cp++)
+ if (!*cp)
+ /* Found the NUL at the end of the header */
+ return 0;
+ stream->next_out = buffer;
+ stream->avail_out = bufsiz;
+ } while (status == Z_OK);
+ return -1;
+}
+
There is nothing "literal" about this function.
The only difference between the original and this one is that this
uses a strbuf, instead of a buffer of a fixed size allocated by the
caller, to return the early part of the inflated data.
I wonder if it incurs too much overhead to the existing callers if
you reimplementated unpack_sha1_header() as a thin wrapper around
this function, something like
int unpack_sha1_header(git_zstream *stream,
unsigned char *map, unsigned long mapsize,
void *buffer, unsigned long bufsiz)
{
int status;
struct strbuf header = STRBUF_INIT;
status = unpack_sha1_header_to_strbuf(stream, map, mapsize,
&header);
if (bufsiz < header.len)
die("object header too long");
memcpy(buffer, header.buf, header.len);
return status;
}
Note that I am *not* suggesting to do this blindly. If there is no
downside from performance point of view, however, the above would be
a way to avoid code duplication.
Another way to avoid code duplication may be to implement
unpack_sha1_header_to_strbuf() in terms of unpack_sha1_header(),
perhaps like this:
static int unpack_sha1_header_to_strbuf(...)
{
unsigned char buffer[32], *cp;
unsigned long bufsiz = sizeof(buffer);
int status = unpack_sha1_header(stream, map, mapsize, buffer,
bufsiz);
if (status)
return status;
do {
strbuf_add(header, buffer, stream->next_out - buffer);
for (cp = buffer; cp < stream->next_out; cp++)
if (!*cp)
/* Found the NUL at the end of the
header */
return 0;
stream->next_out = buffer;
stream->avail_out = bufsiz;
} while (status == Z_OK);
return -1;
}
which may be more in line with how we read data from loose objects.
Agreed there is code duplication with unpack_sha1_header() and
unpack_sha1_header_extended(). But I thought there would be a
performance trade off, Any way I could test this?
Also the second suggestion you have given seems to be more practical, As
there is no performance overhead, if called by exi
Re: [PATCH v4 4/4] t0302: test credential-store support for XDG_CONFIG_HOME
On Wed, Mar 18, 2015 at 3:04 AM, Paul Tan wrote: > t0302 now tests git-credential-store's support for the XDG user-specific > configuration file $XDG_CONFIG_HOME/git/credentials. Specifically: > --- > > The previous version can be found at [1]. > > [1] http://thread.gmane.org/gmane.comp.version-control.git/265305/focus=265308 > > * Merge related, but previously separate, tests together in order to > make the test suite easier to understand. > > * Instead of setting/unsetting XDG_CONFIG_HOME in separate tests, set > it, and unset it immediately before and after "helper_test store" is > called in order to make it localized to only the command that it > should affect. > > * Add test, previously missing, to check that only the home credentials > file is written to if both the xdg and home files exist. > > * Correct mislabelling of "home-user"/"home-pass" to the proper > "xdg-user"/"xdg-pass". > > * Use "rm -f" instead of "test_might_fail rm". This round looks much better. Thanks. Most of the comments below are just nit-picky, with one or two genuine (minor) issues. > Thanks Eric for the code review. > > diff --git a/t/t0302-credential-store.sh b/t/t0302-credential-store.sh > index f61b40c..5b0a666 100755 > --- a/t/t0302-credential-store.sh > +++ b/t/t0302-credential-store.sh > @@ -6,4 +6,115 @@ test_description='credential-store tests' > > helper_test store > > +test_expect_success 'when xdg credentials file does not exist, only write to > ~/.git-credentials; do not create xdg file' ' These test descriptions are quite long, often mirroring in prose what the test body already says more concisely. I generally try to keep descriptions short while still being descriptive enough to give a general idea about what is being tested. I've rewritten a few test descriptions (below) to be very short, in fact probably too terse; but perhaps better descriptions would lie somewhere between the two extremes. First example, for this test: "!xdg: >.git-credentials !xdg" Key: Space-separated items. Items before ":" are the pre-conditions, and items after are the post-state. "!" file does not exist; ">" output goes here. > + test -s "$HOME/.git-credentials" && > + test_path_is_missing "$HOME/.config/git/credentials" > +' > + > +test_expect_success 'create $XDG_CONFIG_HOME/git/credentials file' ' It's customary to call this "setup" rather than "create". Terse version: "setup: -.git-redentials +xdg" Key: "-" file removed; "+" file created. > + rm -f "$HOME/.git-credentials" && > + mkdir -p "$HOME/.config/git" && > + >"$HOME/.config/git/credentials" > +' > + > +helper_test store > + > +test_expect_success 'when xdg credentials file exists, only write to xdg > file; do not create ~/.git-credentials' ' Terse version: "!.git-credentials xdg: !.git-credentials >xdg" > + test -s "$HOME/.config/git/credentials" && > + test_path_is_missing "$HOME/.git-credentials" > +' > + > +test_expect_success 'set up custom XDG_CONFIG_HOME credential file at > ~/xdg/git/credentials' ' s/set up/setup/ Terse: "setup custom-xdg" > + mkdir -p "$HOME/xdg/git" && > + rm -f "$HOME/.config/git/credentials" && > + >"$HOME/xdg/git/credentials" It would be easier to read this if you placed the two lines together which refer to the custom xdg file. Also, for completeness and to be self-contained, don't you want to remove ~/.git-credentials? rm -f "$HOME/.git-credentials" && rm -f "$HOME/.config/git/credentials" && mkdir -p "$HOME/xdg/git" && >"$HOME/xdg/git/credentials" > +' > + > +XDG_CONFIG_HOME="$HOME/xdg" && export XDG_CONFIG_HOME && helper_test store > +unset XDG_CONFIG_HOME It's hard to spot the "helper_test store" at the end of line. I'd place it on a line by itself so that it is easy to see that it is wrapped by the setting and unsetting of the environment variable. > +test_expect_success 'if custom XDG_CONFIG_HOME credentials file > ~/xdg/git/credentials exists, it will only be written to; > ~/.config/git/credentials and ~/.git-credentials will not be created' ' Terse: "!.git-credentials !xdg custom-xdg: !.git-credentials !xdg >custom-xdg" > + test_when_finished "rm -f $HOME/xdg/git/credentials" && > + test -s "$HOME/xdg/git/credentials" && > + test_path_is_missing "$HOME/.config/git/credentials" Matthieu already pointed out the broken &&-chain. > + test_path_is_missing "$HOME/.git-credentials" > +' > + > +test_expect_success 'get: return credentials from home file if matches are > found in both home and xdg files' ' Terse: ".git-credentials xdg: <.git-credentials" Key: "<" taken from here. > + mkdir -p "$HOME/.config/git" && > + echo "https://xdg-user:xdg-p...@example.com"; > >"$HOME/.config/git/credentials" && > + echo "https://home-user:home-p...@example.com"; > >"$HOME/.git-credentials" && > + check fill store <<-\EOF > + protocol=https > + host=example.com > +
Re: [v3 PATCH 1/2] reset: add '-' shorthand for '@{-1}'
On Wed, Mar 18 2015 at 04:29:44 AM, Sundararajan R wrote:
> Teaching reset the - shorthand involves checking if any file named '-' exists.
> check_filename() is used to perform this check.
>
> When the @{-1} branch does not exist then it can be safely assumed that the
> user is referring to the file '-',if any. If this file exists then it is
> reset or else
> a bad flag error is shown.
>
> But if the @{-1} branch exists then it becomes ambiguous without the explicit
> '--' disambiguation as to whether the user wants to reset the file '-' or if
> he wants to reset the working tree to the previous branch. Hence the program
> dies
> with a message about the ambiguous argument.
>
I might be wrong but I think any pathspec that begins with "-" needs to
be preceded by either a "--" marker or be specified as "./-filename",
else verify_filename just die. Therefore you would need to do something
like git reset ./- if you wanted to reset a file. I don't know if given
simply "-" as filename is desired since options starts with "-".
I don't know if you saw but Junio posted a while ago about about
allowing "-" as a stand-in everywhere a revision was allowed. He updated
a version on pu : "d40f108d"
> On Tue, Mar 17 2015 at 02:49:48 AM, Junio C Hamano wrote:
>> Junio C Hamano writes:
>>
>> if (try to see if it is a revision or regvision range) {
>> /* if failed ... */
>> if (starts with '-') {
>> do the option thing;
>> continue;
>> }
>> /* args must be pathspecs from here on */
>> check the '--' disambiguation;
>> add pathspec to prune-data;
>> } else {
>> got_rev_arg = 1;
>> }
>>
See $gmane/265672
--
To unsubscribe from this list: send the line "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] clone: drop period from end of die_errno message
We do not usually end our errors with a full stop, but it
looks especially bad when you use die_errno, which adds a
colon, like:
fatal: could not create work tree dir 'foo'.: No such file or directory
Signed-off-by: Jeff King
---
Not strictly related to the other patch, but I noticed it while playing
around.
builtin/clone.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/builtin/clone.c b/builtin/clone.c
index 9572467..aa01437 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -848,7 +848,7 @@ int cmd_clone(int argc, const char **argv, const char
*prefix)
die_errno(_("could not create leading directories of
'%s'"),
work_tree);
if (!dest_exists && mkdir(work_tree, 0777))
- die_errno(_("could not create work tree dir '%s'."),
+ die_errno(_("could not create work tree dir '%s'"),
work_tree);
set_git_work_tree(work_tree);
}
--
2.3.3.520.g3cfbb5d
--
To unsubscribe from this list: send the line "unsubscribe 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] grep: fix "--quiet" overwriting current output
On Wed, Mar 18, 2015 at 07:00:13PM +0100, Wilhelm Schuermann wrote: > When grep is called with the --quiet option, the pager is initialized > despite not being used. When the pager is "less", anything output by > previous commands and not ended with a newline is overwritten. > [...] > This patch prevents calling the pager in the first place, saving an > unnecessary fork() call. Thanks, I think this makes sense. We do a similar thing for "git diff --quiet". If you do not set "-F" in your $LESS variable, it is even more annoying. E.g., with: if git grep -q foo; then : do something fi which will pause, waiting for the user to hit 'q'. > diff --git a/builtin/grep.c b/builtin/grep.c > index e77f7cf..fe7b9fd 100644 > --- a/builtin/grep.c > +++ b/builtin/grep.c > @@ -885,7 +885,7 @@ int cmd_grep(int argc, const char **argv, const char > *prefix) > } > } > > - if (!show_in_pager) > + if (!show_in_pager && !opt.status_only) > setup_pager(); Patch looks obviously correct. -Peff -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] clone: initialize atexit cleanup handler earlier
On Wed, Mar 18, 2015 at 11:03:30AM -0700, Spencer Nelson wrote:
> If you’re in a shell in a directory which no longer exists (because,
> say, another terminal removed it), then getcwd() will fail, at least
> on OS X Yosemite 10.10.2. In this case, git clone will fail. That’s
> totally reasonable.
Yeah, we fail in set_git_work_tree, which calls real_path() on the new
directory, which fails because it cannot get the current working
directory. In the example you gave, we already have an absolute path to
the new directory, and it is outside the "disappeared" directory. So I
would think we could get by without having a cwd. It looks like we do so
because we have to chdir() away from the cwd as part of real_path, and
then need to be able to chdir back. So I'm not sure there's an easy way
to fix it.
Anyway, that's tangential to your actual problem...
> If you invoke git clone with the git clone syntax, then
> will be created, but it will be empty.
I think the original code just didn't expect set_git_work_tree to fail,
and doesn't install the cleanup code until after it is called. This
patch fixes it.
-- >8 --
Subject: clone: initialize atexit cleanup handler earlier
If clone fails, we generally try to clean up any directories
we've created. We do this by installing an atexit handler,
so that we don't have to manually trigger cleanup. However,
since we install this after touching the filesystem, any
errors between our initial mkdir() and our atexit() call
will result in us leaving a crufty directory around.
We can fix this by moving our atexit() call earlier. It's OK
to do it before the junk_work_tree variable is set, because
remove_junk makes sure the variable is initialized. This
means we "activate" the handler by assigning to the
junk_work_tree variable, which we now bump down to just
after we call mkdir(). We probably do not want to do it
before, because a plausible reason for mkdir() to fail is
EEXIST (i.e., we are racing with another "git init"), and we
would not want to remove their work.
OTOH, this is probably not that big a deal; we will allow
cloning into an empty directory (and skip the mkdir), which
is already racy (i.e., one clone may see the other's empty
dir and start writing into it). Still, it does not hurt to
err on the side of caution here.
Note that writing into junk_work_tree and junk_git_dir after
installing the handler is also technically racy, as we call
our handler on an async signal. Depending on the platform,
we could see a sheared write to the variables. Traditionally
we have not worried about this, and indeed we already do
this later in the function. If we want to address that, it
can come as a separate topic.
Signed-off-by: Jeff King
---
Sheesh, for such a little change, there are a lot of racy things to
think about. Note that even if we did want to make two racing clone
processes atomic in creating the working tree, the whole git_dir
initialization is still not (and explicitly ignores EEXIST). I think if
somebody wants atomicity here, they should do the mkdir themselves, and
then have git fill in the rest.
No test. I seem to recall that Windows is tricky with making the cwd go
away (can you even do it there while a process is still in it?), and I
don't think such a minor thing is worth the portability headcaches in
the test suite.
builtin/clone.c | 9 +
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/builtin/clone.c b/builtin/clone.c
index aa01437..53a2e5a 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -842,20 +842,21 @@ int cmd_clone(int argc, const char **argv, const char
*prefix)
git_dir = mkpathdup("%s/.git", dir);
}
+ atexit(remove_junk);
+ sigchain_push_common(remove_junk_on_signal);
+
if (!option_bare) {
- junk_work_tree = work_tree;
if (safe_create_leading_directories_const(work_tree) < 0)
die_errno(_("could not create leading directories of
'%s'"),
work_tree);
if (!dest_exists && mkdir(work_tree, 0777))
die_errno(_("could not create work tree dir '%s'"),
work_tree);
+ junk_work_tree = work_tree;
set_git_work_tree(work_tree);
}
- junk_git_dir = git_dir;
- atexit(remove_junk);
- sigchain_push_common(remove_junk_on_signal);
+ junk_git_dir = git_dir;
if (safe_create_leading_directories_const(git_dir) < 0)
die(_("could not create leading directories of '%s'"), git_dir);
--
2.3.3.520.g3cfbb5d
--
To unsubscribe from this list: send the line "unsubscribe 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 01/14] numparse: new module for parsing integral numbers
On Tuesday, March 17, 2015, Michael Haggerty wrote:
> Implement wrappers for strtol() and strtoul() that are safer and more
> convenient to use.
>
> Signed-off-by: Michael Haggerty
> ---
> diff --git a/numparse.c b/numparse.c
> new file mode 100644
> index 000..90b44ce
> --- /dev/null
> +++ b/numparse.c
> @@ -0,0 +1,180 @@
> +int parse_l(const char *s, unsigned int flags, long *result, char **endptr)
> +{
> + long l;
> + const char *end;
> + int err = 0;
> +
> + err = parse_precheck(s, &flags);
> + if (err)
> + return err;
> + /*
> +* Now let strtol() do the heavy lifting:
> +*/
Perhaps use a /* one-line style comment */ to reduce vertical space
consumption a bit, thus make it (very slightly) easier to run the eye
over the code.
> + errno = 0;
> + l = strtol(s, (char **)&end, flags & NUM_BASE_MASK);
> + if (errno) {
> + if (errno == ERANGE) {
> + if (!(flags & NUM_SATURATE))
> + return -NUM_SATURATE;
> + } else {
> + return -NUM_OTHER_ERROR;
> + }
> + }
Would it reduce cognitive load slightly (and reduce vertical space
consumption) to rephrase the conditionals as:
if (errno == ERANGE && !(flags & NUM_SATURATE))
return -NUM_SATURATE;
if (errno && errno != ERANGE)
return -NUM_OTHER_ERROR;
or something similar?
More below.
> + if (end == s)
> + return -NUM_NO_DIGITS;
> +
> + if (*end && !(flags & NUM_TRAILING))
> + return -NUM_TRAILING;
> +
> + /* Everything was OK */
> + *result = l;
> + if (endptr)
> + *endptr = (char *)end;
> + return 0;
> +}
> diff --git a/numparse.h b/numparse.h
> new file mode 100644
> index 000..4de5e10
> --- /dev/null
> +++ b/numparse.h
> @@ -0,0 +1,207 @@
> +#ifndef NUMPARSE_H
> +#define NUMPARSE_H
> +
> +/*
> + * Functions for parsing integral numbers.
> + *
> + * Examples:
> + *
> + * - Convert s into a signed int, interpreting prefix "0x" to mean
> + * hexadecimal and "0" to mean octal. If the value doesn't fit in an
> + * unsigned int, set result to INT_MIN or INT_MAX.
Did you mean s/unsigned int/signed int/ ?
> + *
> + * if (convert_i(s, NUM_SLOPPY, &result))
> + * die("...");
> + */
> +
> +/*
> + * The lowest 6 bits of flags hold the numerical base that should be
> + * used to parse the number, 2 <= base <= 36. If base is set to 0,
> + * then NUM_BASE_SPECIFIER must be set too; in this case, the base is
> + * detected automatically from the string's prefix.
Does this restriction go against the goal of making these functions
convenient, even while remaining strict? Is there a strong reason for
not merely inferring NUM_BASE_SPECIFIER when base is 0? Doing so would
make it consistent with strto*l() without (I think) introducing any
ambiguity.
> + */
> +/*
> + * Number parsing functions:
> + *
> + * The following functions parse a number (long, unsigned long, int,
> + * or unsigned int respectively) from the front of s, storing the
> + * value to *result and storing a pointer to the first character after
> + * the number to *endptr. flags specifies how the number should be
> + * parsed, including which base should be used. flags is a combination
> + * of the numerical base (2-36) and the NUM_* constants above (see).
"(see)" what?
> + * Return 0 on success or a negative value if there was an error. On
> + * failure, *result and *entptr are left unchanged.
> + *
> + * Please note that if NUM_TRAILING is not set, then it is
> + * nevertheless an error if there are any characters between the end
> + * of the number and the end of the string.
Again, on the subject of convenience, why this restriction? The stated
purpose of the parse_*() functions is to parse the number from the
front of the string and return a pointer to the first non-numeric
character following. As a reader of this API, I interpret that as
meaning that NUM_TRAILING is implied. Is there a strong reason for not
inferring NUM_TRAILING for the parse_*() functions at the API level?
(I realize that the convert_*() functions are built atop parse_*(),
but that's an implementation detail.)
> + */
> +
> +int parse_l(const char *s, unsigned int flags,
> + long *result, char **endptr);
Do we want to perpetuate the ugly (char **) convention for 'endptr'
from strto*l()? Considering that the incoming string is const, it
seems undesirable to return a non-const pointer to some place inside
that string.
> +/*
> + * Number conversion functions:
> + *
> + * The following functions parse a string into a number. They are
> + * identical to the parse_*() functions above, except that the endptr
> + * is not returned. These are most useful when parsing a whole string
> + * into a number; i.e., when (flags & NUM_TRAILING) is unset.
I can formulate arguments for allowing or disallowin
git clone doesn't cleanup on failure when getcwd fails
If you’re in a shell in a directory which no longer exists (because, say, another terminal removed it), then getcwd() will fail, at least on OS X Yosemite 10.10.2. In this case, git clone will fail. That’s totally reasonable. If you invoke git clone with the git clone syntax, then will be created, but it will be empty. This was unexpected - I would think the failure case shouldn’t leave this empty directory, but should instead clean up after itself. I’m not alone: golang’s go get command for installing third-party packages performs a `git clone ` only if does not already exist - if it does exist, then go believes that the package is already installed, and so does nothing. So, if you call go get from within a directory which no longer exists, git will create the empty directory, putting go get into a bad state. Concrete example: Make a dummy repo in /tmp/somerepo: tty1: $ cd /tmp $ git init somerepo In another tty, make a /tmp/poof and enter it tty2: $ mkdir /tmp/poof $ cd /tmp/poof In the first tty, delete /tmp/poof tty1: $ rmdir /tmp/poof Finally, in the second tty, clone: tty2: $ git clone /tmp/somerepo /tmp/newcopy fatal: Could not get current working directory: No such file or directory $ ls /tmp/newcopy && echo "yes, it exists" yes, it exists My version details: $ git version git version 2.3.2 $ uname -a Darwin mbp.local 14.1.0 Darwin Kernel Version 14.1.0: Thu Feb 26 19:26:47 PST 2015; root:xnu-2782.10.73~1/RELEASE_X86_64 x86_64 -- To unsubscribe from this list: send the line "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/RFC] grep: fix "--quiet" overwriting current output
When grep is called with the --quiet option, the pager is initialized despite not being used. When the pager is "less", anything output by previous commands and not ended with a newline is overwritten. $ echo -n aaa; echo bbb aaabbb $ echo -n aaa; git grep -q foo; echo bbb bbb This can be worked around, for example, by making sure STDOUT is not a TTY or more directly by setting git's pager to "cat": $ echo -n aaa; git grep -q foo > /dev/null; echo bbb aaabbb $ echo -n aaa; PAGER=cat git grep -q foo; echo bbb aaabbb This patch prevents calling the pager in the first place, saving an unnecessary fork() call. Signed-off-by: Wilhelm Schuermann --- builtin/grep.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/builtin/grep.c b/builtin/grep.c index e77f7cf..fe7b9fd 100644 --- a/builtin/grep.c +++ b/builtin/grep.c @@ -885,7 +885,7 @@ int cmd_grep(int argc, const char **argv, const char *prefix) } } - if (!show_in_pager) + if (!show_in_pager && !opt.status_only) setup_pager(); if (!use_index && (untracked || cached)) -- 2.3.3.dirty -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH/RFC/GSOC] make git-pull a builtin
Hi,
First of all, thanks a lot for working on this. I'm rather impressed to
see a working proof of concept so soon! And impressed by the quality for
a "first draft".
A few minor remaks below after a very quick look.
Paul Tan writes:
> Ideally, I think the solution is to
> improve the test suite and make it as comprehensive as possible, but
> writing a comprehensive test suite may be too time consuming.
time-consuming, but also very beneficial since the code would end up
being better tested. For sure, a rewrite is a good way to break stuff,
but anything untested can also be broken by mistake rather easily at any
time.
I'd suggest doing a bit of manual mutation testing: take your C code,
comment-out a few lines of code, see if the tests still pass, and if
they do, add a failing test that passes again once you uncomment the
code.
> diff --git a/Makefile b/Makefile
> index 44f1dd1..50a6a16 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -470,7 +470,6 @@ SCRIPT_SH += git-merge-octopus.sh
> SCRIPT_SH += git-merge-one-file.sh
> SCRIPT_SH += git-merge-resolve.sh
> SCRIPT_SH += git-mergetool.sh
> -SCRIPT_SH += git-pull.sh
When converting a script into a builtin, we usually move the old script
to contrib/examples/.
> +static const char * const builtin_pull_usage[] = {
> + N_("git pull [-n | --no-stat] [--[no-]commit] [--[no-]squash]
> [--[no-]ff|--ff-only] [--[no-]rebase|--rebase=preserve] [-s strategy]...
> [] ..."),
I know we have many instances of very long lines for usage string, but
it would be better IMHO to wrap it both in the code and in the output of
"git pull -h".
> +/* NOTE: git-pull.sh only supports --log and --no-log, as opposed to what
> + * man git-pull says. */
We usually write multi-line comments
/*
* like
* this
*/
> +/* Global vars since they are used often */
Being use often does not count as an excuse for being global IMHO.
Having global variables means you share the same instance in several
functions, and you have to be careful with things like
void g()
{
glob = bar;
}
void f()
{
glob = foo;
g();
bar = glob;
}
As a reader, I'd rather not have to be careful about this to keep my
neurons free for other things.
> +static char *head_name;
Actually, this one is used only in one function, so "often" is not even
true ;-).
> +static struct option builtin_pull_options[] = {
You may also declare this as local in cmd_pull().
> +/**
> + * Returns remote for branch
Here and elsewhere: use imperative (return, not return_s_). The comment
asks the function to return a value.
> + strbuf_addf(&key, "branch.%s.remote", branch);
> + git_config_get_value(key.buf, &remote);
> + strbuf_release(&key);
This config API is beautiful :-).
(Before last year's GSoC, you'd have needed ~10 lines of code to do the
same thing)
> + return error("Ambiguous refname: '%s'", ref);
Here and elsewhere: don't forget to mark strings for translation.
> +/**
> + * Appends FETCH_HEAD's merge heads into arr. Returns number of merge heads,
> + * or -1 on error.
> + */
> +static int sha1_array_append_fetch_merge_heads(struct sha1_array *arr)
> +{
> + int num_heads = 0;
> + char *filename = git_path("FETCH_HEAD");
> + FILE *fp = fopen(filename, "r");
I guess this is one instance where we could avoid writting (fetch) and
then parsing (here) if we had a better internal API.
But that can come after, of course.
> +}
> +
> +
> +static void argv_array_push_merge_args(struct argv_array *arr,
Bikeshed: you sometimes have two blank lines between functions,
sometimes one. Not sure it's intended.
--
Matthieu Moy
http://www-verimag.imag.fr/~moy/
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [v3 PATCH 2/2] reset: add tests for git reset -
Sundararajan R writes:
> Subject: [v3 PATCH 2/2] reset: add tests for git reset -
This should be [PATCH v3 2/2].
"git send-email -v2" can do this for you.
Sundararajan R writes:
> +test_expect_success 'reset - with no @{-1} branch and file named - should
> succeed' '
> + test_when_finished rm -rf new &&
> + >expected &&
> + git init new &&
> + (
> + cd new &&
> + echo "Hello" >- &&
> + git add - &&
> + git reset - >../actual
> + ) &&
> + test_cmp expected actual
> +'
test_must_be_empty actual would be easier to read than ">expected ...
test_cmp expected" IMHO.
> +test_expect_success 'reset - with @{-1} branch and no file named - should
> succeed' '
> + test_when_finished rm -rf new &&
> + git init new &&
> + (
> + cd new &&
> + echo "Hey" >new_file &&
> + git add new_file &&
> + git commit -m "first_commit" &&
> + git checkout -b new_branch &&
> + >new_file &&
> + git add new_file &&
> + git reset - &&
> + git status -uno >actual &&
> + git add new_file &&
> + git reset @{-1} &&
> + git status -uno >expected &&
> + test_cmp actual expected
> + )
> +'
Better use "git status --porcelain" here as its format is meant to be
stable and unambiguous. The non-porcelain should work two because you're
comparing the output on two identical states, but who knows.
With or without my suggested change, the series looks good to me.
--
Matthieu Moy
http://www-verimag.imag.fr/~moy/
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 4/4] t0302: test credential-store support for XDG_CONFIG_HOME
Similarly to the "merge doc and code", I personally prefer seeing code and tests in the same patch. Actually, my preference goes for a first patch that introduces the tests with test_expect_failure for things that are not yet implemented (and you can check that tests do not pass yet before you code), and then the patch introducing the feature doing -test_expect_failure +test_expect_success which documents quite clearly and concisely that you just made the feature work. Paul Tan writes: > +test_expect_success 'if custom XDG_CONFIG_HOME credentials file > + ~/xdg/git/credentials exists, it will only be written to; > + ~/.config/git/credentials and ~/.git-credentials will not be > + created' ' > + test_when_finished "rm -f $HOME/xdg/git/credentials" && > + test -s "$HOME/xdg/git/credentials" && > + test_path_is_missing "$HOME/.config/git/credentials" > + test_path_is_missing "$HOME/.git-credentials" > +' Broken &&-chain. That is a real issue that must be fixed. > +test_expect_success 'get: return credentials from home file if xdg files are > unreadable' ' "files are" -> "file is", no? > +' > + > + > +test_expect_success 'erase: erase matching credentials from both xdg and > home files' ' Double blank line. On overall, except the broken &&-chain above, the whole series looks good. Feel free to ignore my other nitpicks if you prefer. -- Matthieu Moy http://www-verimag.imag.fr/~moy/ -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 3/4] docs/git-credential-store: document XDG file and precedence
I would personnally prefer to see this squashed with PATCH 2/4: pushing the "bisectable history" principle a bit, the state between patches 2 and 3 could be considered broken because the code does not do what the documentation says. And as a reviewer, I like having pieces of docs linked to the patch they document. Paul Tan writes: > +Credential storage will per default Not a native, but "per default" sounds weird and "by default" seems far more common. -- Matthieu Moy http://www-verimag.imag.fr/~moy/ -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 1/4] git-credential-store: support multiple credential files
Paul Tan writes: > + /* Write credential to the filename specified by fns->items[0], thus > + * creating it */ Just to show I'm following: misformatted multi-line comment. Other than that, good job! -- Matthieu Moy http://www-verimag.imag.fr/~moy/ -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Submodule not working with specified work-tree
It seems like submodule isn't picking up on the work tree that I'm specifying. In the scenario that I'm working with, I'd prefer to not have to "cd" into a directory to update the submodules. All the other git subcommands that I'm executing work fine with specifying the git dir and work tree via envvars or parameters. I'm not sure if this is a bug or not. $ git --git-dir $HOME/testrepo/.git --work-tree $HOME/testrepo submodule update --init --recursive fatal: /usr/local/Cellar/git/2.3.0/libexec/git-core/git-submodule cannot be used without a working tree. $ GIT_WORK_TREE=$HOME/testrepo GIT_DIR=$HOME/testrepo/.git git submodule update --init --recursive fatal: /usr/local/Cellar/git/2.3.0/libexec/git-core/git-submodule cannot be used without a working tree. $ git version git version 2.3.0 $ uname -a Darwin hanazawa.local 14.1.0 Darwin Kernel Version 14.1.0: Mon Dec 22 23:10:38 PST 2014; root:xnu-2782.10.72~2/RELEASE_X86_64 x86_64 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Seems to be pushing more than necessary
Got there eventually! $ git verify-pack --verbose bar.pack e13e21a1f49704ed35ddc3b15b6111a5f9b34702 commit 220 152 12 03691863451ef9db6c69493da1fa556f9338a01d commit 334 227 164 ... snip ... chain length = 50: 2 objects bar.pack: ok Now what do I do with it :) On 18 March 2015 at 13:33, Duy Nguyen wrote: > On Wed, Mar 18, 2015 at 8:16 PM, Graham Hay wrote: >> I created a repo with over 1GB of images, but it works as expected >> (only pushed 3 objects). >> >> Sorry, I must have done something wrong. I put that script in >> ~/Applications, and checked it worked. Then I ran this: >> >> $ GIT_TRACE=2 PATH=~/Applications:$PATH git push --set-upstream origin >> git-wtf > > I think I encountered the same problem. Inserting > --exec-path=$HOME/Applications between "git" and "push" was probably > what made it work for me. Haven't investigated the reason yet. We > really should have an easier way to get this info without jumping > through hoops like this. > -- > Duy -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: git pull & git gc
On Wed, Mar 18, 2015 at 09:41:59PM +0700, Duy Nguyen wrote: > On Wed, Mar 18, 2015 at 9:33 PM, Duy Nguyen wrote: > > If not, I made some mistake in analyzing this and we'll start again. > > I did make one mistake, the first "gc" should have reduced the number > of loose objects to zero. Why didn't it.? I'll come back to this > tomorrow if nobody finds out first :) Most likely they are not referenced by anything but are younger than 2 weeks. I saw a similar issue with automatic gc triggering after every operation when I did something equivalent to: git add git commit git reset --hard HEAD^ which creates a log of unreachable objects which are not old enough to be pruned. -- To unsubscribe from this list: send the line "unsubscribe 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 pull & git gc
Hello Duy, #ls .git/objects/17/* | wc -l 30 30 * 256 = 7 680 > 6 700 And now? Do I have to run git gc --aggressive ? Kind regards Dilian On 18.03.2015 15:33, Duy Nguyen wrote: On Wed, Mar 18, 2015 at 9:23 PM, Дилян Палаузов wrote: Hello, # git gc --auto Auto packing the repository in background for optimum performance. See "git help gc" for manual housekeeping. and calls in the background: 25618 1 0 32451 884 1 14:20 ?00:00:00 git gc --auto 25639 25618 51 49076 49428 0 14:20 ?00:00:07 git prune --expire 2.weeks.ago # git count-objects -v count: 6039 loose number threshold is 6700, unless you tweaked something. But there's a tweak, we'll come back to this. size: 65464 in-pack: 185432 packs: 1 Pack threshold is 50, You only have one pack, good OK back to the "count 6039" above. You have that many loose objects. But 'git gc' is lazier than 'git count-objects'. It assume a flat distribution, and only counts the number of objects in .git/objects/17 directory only, then extrapolate for the total number. So can you see how many files you have in this directory .git/objects/17? That number, multiplied by 256, should be greater than 6700. If that's the case "git gc" laziness is the problem. If not, I made some mistake in analyzing this and we'll start again. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: git pull & git gc
On Wed, Mar 18, 2015 at 9:33 PM, Duy Nguyen wrote: > If not, I made some mistake in analyzing this and we'll start again. I did make one mistake, the first "gc" should have reduced the number of loose objects to zero. Why didn't it.? I'll come back to this tomorrow if nobody finds out first :) -- Duy -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: git pull & git gc
On Wed, Mar 18, 2015 at 9:23 PM, Дилян Палаузов wrote: > Hello, > > # git gc --auto > Auto packing the repository in background for optimum performance. > See "git help gc" for manual housekeeping. > > and calls in the background: > > 25618 1 0 32451 884 1 14:20 ?00:00:00 git gc --auto > 25639 25618 51 49076 49428 0 14:20 ?00:00:07 git prune --expire > 2.weeks.ago > > # git count-objects -v > count: 6039 loose number threshold is 6700, unless you tweaked something. But there's a tweak, we'll come back to this. > size: 65464 > in-pack: 185432 > packs: 1 Pack threshold is 50, You only have one pack, good OK back to the "count 6039" above. You have that many loose objects. But 'git gc' is lazier than 'git count-objects'. It assume a flat distribution, and only counts the number of objects in .git/objects/17 directory only, then extrapolate for the total number. So can you see how many files you have in this directory .git/objects/17? That number, multiplied by 256, should be greater than 6700. If that's the case "git gc" laziness is the problem. If not, I made some mistake in analyzing this and we'll start again. -- Duy -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: git pull & git gc
Hello, # git gc --auto Auto packing the repository in background for optimum performance. See "git help gc" for manual housekeeping. and calls in the background: 25618 1 0 32451 884 1 14:20 ?00:00:00 git gc --auto 25639 25618 51 49076 49428 0 14:20 ?00:00:07 git prune --expire 2.weeks.ago # git count-objects -v count: 6039 size: 65464 in-pack: 185432 packs: 1 size-pack: 46687 prune-packable: 0 garbage: 0 size-garbage: 0 Regards Dilian On 18.03.2015 15:16, Duy Nguyen wrote: On Wed, Mar 18, 2015 at 8:53 PM, Дилян Палаузов wrote: Hello, I have a local folder with the git-repository (so that its .git/config contains ([remote "origin"]\nurl = git://github.com/git/git.git\nfetch = +refs/heads/*:refs/remotes/origin/* ) I do there "git pull". Usually the output is Already up to date but since today it prints Auto packing the repository in background for optimum performance. See "git help gc" for manual housekeeping. Already up-to-date. and starts in the background a "git gc --auto" process. This is all fine, however, when the "git gc" process finishes, and I do again "git pull" I get the same message, as above (git gc is again started). So if you do "git gc --auto" now, does it exit immediately or go through the garbage collection process again (it'll print something)? What does "git count-objects -v" show? -- To unsubscribe from this list: send the line "unsubscribe 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 pull & git gc
On Wed, Mar 18, 2015 at 8:53 PM, Дилян Палаузов wrote: > Hello, > > I have a local folder with the git-repository (so that its .git/config > contains ([remote "origin"]\nurl = git://github.com/git/git.git\nfetch = > +refs/heads/*:refs/remotes/origin/* ) > > I do there "git pull". > > Usually the output is > Already up to date > > but since today it prints > Auto packing the repository in background for optimum performance. > See "git help gc" for manual housekeeping. > Already up-to-date. > > and starts in the background a "git gc --auto" process. This is all fine, > however, when the "git gc" process finishes, and I do again "git pull" I get > the same message, as above (git gc is again started). So if you do "git gc --auto" now, does it exit immediately or go through the garbage collection process again (it'll print something)? What does "git count-objects -v" show? -- Duy -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
git pull & git gc
Hello, I have a local folder with the git-repository (so that its .git/config contains ([remote "origin"]\n url = git://github.com/git/git.git\nfetch = +refs/heads/*:refs/remotes/origin/* ) I do there "git pull". Usually the output is Already up to date but since today it prints Auto packing the repository in background for optimum performance. See "git help gc" for manual housekeeping. Already up-to-date. and starts in the background a "git gc --auto" process. This is all fine, however, when the "git gc" process finishes, and I do again "git pull" I get the same message, as above (git gc is again started). My understanding is, that "git gc" has to be occasionally run and then the garbage collection is done for a while. In the concrete case, if "git pull" starts "git gc" in the background and prints a message on this, it is all fine, but running "git pull" after a while, when the garbage collection was recently done, where shall be neither message nor an action about "git gc". My system-wide gitconfig contains "[pack] threads = 1". I have "tar xJf"'ed my local git repository and have put it under http://mail.aegee.org/dpa/v/git-repository.tar.xz The question is: Why does "git pull" every time when it is invoked today print information about "git gc"? I have git 2.3.3 adjusted with "./configure --with-openssl --with-libpcre --with-curl --with-expat". Thanks in advance for your answer Dilian -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Seems to be pushing more than necessary
On Wed, Mar 18, 2015 at 8:16 PM, Graham Hay wrote: > I created a repo with over 1GB of images, but it works as expected > (only pushed 3 objects). > > Sorry, I must have done something wrong. I put that script in > ~/Applications, and checked it worked. Then I ran this: > > $ GIT_TRACE=2 PATH=~/Applications:$PATH git push --set-upstream origin git-wtf I think I encountered the same problem. Inserting --exec-path=$HOME/Applications between "git" and "push" was probably what made it work for me. Haven't investigated the reason yet. We really should have an easier way to get this info without jumping through hoops like this. -- Duy -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Seems to be pushing more than necessary
I created a repo with over 1GB of images, but it works as expected (only pushed 3 objects). Sorry, I must have done something wrong. I put that script in ~/Applications, and checked it worked. Then I ran this: $ GIT_TRACE=2 PATH=~/Applications:$PATH git push --set-upstream origin git-wtf 12:48:28.839026 git.c:349 trace: built-in: git 'push' '--set-upstream' 'origin' 'git-wtf' 12:48:28.907605 run-command.c:351 trace: run_command: 'ssh' 'g...@github.com' 'git-receive-pack '\''grahamrhay/bornlucky-ios.git'\''' 12:48:30.137410 run-command.c:351 trace: run_command: 'pack-objects' '--all-progress-implied' '--revs' '--stdout' '--thin' '--delta-base-offset' '--progress' 12:48:30.138246 exec_cmd.c:130 trace: exec: 'git' 'pack-objects' '--all-progress-implied' '--revs' '--stdout' '--thin' '--delta-base-offset' '--progress' 12:48:30.144783 git.c:349 trace: built-in: git 'pack-objects' '--all-progress-implied' '--revs' '--stdout' '--thin' '--delta-base-offset' '--progress' Counting objects: 10837, done. Delta compression using up to 4 threads. Compressing objects: 100% (9301/9301), done. Writing objects: 21% (2276/10837) but there was nothing in /tmp/stdin. Have I missed a step? I tried changing the tee to point to ~ in case it was permissions related. I fear this is some Mac nonsense. I added an echo in the script, but it only gets called for the first git incantation. On 18 March 2015 at 12:34, Duy Nguyen wrote: > On Wed, Mar 18, 2015 at 7:26 PM, Duy Nguyen wrote: >> It's quite a lot of work :) I created this script named "git" and put >> it in $PATH to capture input for pack-objects. You'll need to update >> "/path/to/real/git" to point to the real binary then you'll get >> /tmp/stdin > > Forgot one important sentence: You need to push again using this fake > "git" program to save data in /tmp/stdin. Also you can stop the push > when it goes to "compressing objects" phase. > -- > Duy -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] diff-lib.c: adjust position of i-t-a entries in diff
On Wed, Mar 18, 2015 at 12:57 AM, Junio C Hamano wrote:
> Duy Nguyen writes:
>
>> On Mon, Mar 16, 2015 at 09:05:45AM -0700, Junio C Hamano wrote:
>>> The offending one came from eec3e7e4 (cache-tree: invalidate i-t-a
>>> paths after generating trees, 2012-12-16), which was a fix to an
>>> earlier bug where a cache-tree written out of an index with i-t-a
>>> entries had incorrect information and still claimed it is fully
>>> valid after write-tree rebuilt it. The test probably should add
>>> another path without i-t-a bit, run the same "diff --cached" with
>>> updated expectation before write-tre, and run the "diff --cached"
>>> again to make sure it produces a result that match the updated
>>> expectation.
>>
>> Would adding another non-i-t-a entry help? Before this patch
>> "diff --cached" after write-tree shows the i-t-a entry only when
>> eec3e7e4 is applied. But with this patch we don't show i-t-a entry any
>> more, before or after write-tree, eec3e7e4 makes no visible difference.
>>
>> We could even revert eec3e7e4 and the outcome of "diff --cached" would
>> be the same because we just sort of move the "invalidation" part from
>> cache-tree to do_oneway_diff(). Not invalidating would speed up "diff
>> --cached" when i-t-a entries are present. Still it may be a good idea
>> to invalidate i-t-a paths to be on the safe side. Perhaps a patch like
>> this to resurrect the test?
>
> My unerstanding of what eec3e7e4 (cache-tree: invalidate i-t-a paths
> after generating trees, 2012-12-16) fixed was that in this sequence:
>
>...
>
> So reverting the fix obviously is not the right thing to do. If the
> tests show different results from two invocations of "diff --cached"
> with your patch applied, there is something that is broken by your
> patch, because the index and the HEAD does not change across
> write-tree in that test.
Right. I missed this but I think this is a less important test because
I added a new test to make sure "diff --cached" ("git status" to be
exact) outputs the right thing when i-t-a entries are present.
> If on the other hand the tests show the same result from these two
> "diff --cached" and the result is different from what the test
> expects, that means your patch changed the world order, i.e. an
> i-t-a entry used to be treated as if it were adding an empty blob to
> the index but it is now treated as non-existent, then that is a good
> thing and the only thing we need to update is what the test expects.
> I am guessing that instead of expecting dir/bar to be shown, it now
> should expect no output?
Yes, no output.
> Does adding an non-i-t-a entry help? It does not hurt, and it makes
> the test uses a non-empty output, making its effect more visible,
> which may or may not count as helping.
But still, if I revert the change in cache-tree.c and force write-tree
to produce valid cache-tree when i-t-a entries are present, this test
still passes incorrectly. This is why I changed the test.
--
Duy
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Seems to be pushing more than necessary
On Wed, Mar 18, 2015 at 7:26 PM, Duy Nguyen wrote: > It's quite a lot of work :) I created this script named "git" and put > it in $PATH to capture input for pack-objects. You'll need to update > "/path/to/real/git" to point to the real binary then you'll get > /tmp/stdin Forgot one important sentence: You need to push again using this fake "git" program to save data in /tmp/stdin. Also you can stop the push when it goes to "compressing objects" phase. -- Duy -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Bug in git-verify-pack or in its documentation?
On Mon, Mar 16, 2015 at 8:18 PM, Mike Hommey wrote: > On Mon, Mar 16, 2015 at 05:13:25PM +0700, Duy Nguyen wrote: >> On Mon, Mar 16, 2015 at 3:05 PM, Mike Hommey wrote: >> > Hi, >> > >> > git-verify-pack's man page says the following about --stat-only: >> > >> >Do not verify the pack contents; only show the histogram of delta >> >chain length. With --verbose, list of objects is also shown. >> > >> > However, there is no difference of output between --verbose and >> > --verbose --stat-only (and in fact, looking at the code, only one of >> > them has an effect, --stat-only having precedence). >> >> There is. very-pack --verbose would show a lot of " >> <"random" numbers>" lines before the histogram while --stat-only (with >> or without --verbose) would only show the histogram. > > Err, I meant between --stat-only and --verbose --stat-only. Yeah --verbose is always ignored when --stat-only is set. This command is funny. -- Duy -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH, RFC] checkout: Attempt to checkout submodules
If a user does git checkout HEAD -- path/to/submodule they'd expect the
submodule to be checked out to the commit that submodule is at in HEAD.
This is the most brute force possible way of try to do that, and so its
probably broken in some cases. However I'm not terribly familiar with
git's internals and I'm not sure if this is even wanted so I'm starting
simple. If people want this to work I can try and do something better.
Signed-off-by: Trevor Saunders
---
entry.c | 22 --
1 file changed, 20 insertions(+), 2 deletions(-)
diff --git a/entry.c b/entry.c
index 1eda8e9..2dbf5b9 100644
--- a/entry.c
+++ b/entry.c
@@ -1,6 +1,8 @@
#include "cache.h"
+#include "argv-array.h"
#include "blob.h"
#include "dir.h"
+#include "run-command.h"
#include "streaming.h"
static void create_directories(const char *path, int path_len,
@@ -277,9 +279,25 @@ int checkout_entry(struct cache_entry *ce,
* just do the right thing)
*/
if (S_ISDIR(st.st_mode)) {
- /* If it is a gitlink, leave it alone! */
- if (S_ISGITLINK(ce->ce_mode))
+ if (S_ISGITLINK(ce->ce_mode)) {
+ struct argv_array args = ARGV_ARRAY_INIT;
+ char sha1[41];
+
+ argv_array_push(&args, "checkout");
+
+ if (state->force)
+ argv_array_push(&args, "-f");
+
+ memcpy(sha1, sha1_to_hex(ce->sha1), 41);
+ argv_array_push(&args, sha1);
+
+ run_command_v_opt_cd_env(args.argv,
+RUN_GIT_CMD, ce->name,
+NULL);
+ argv_array_clear(&args);
+
return 0;
+ }
if (!state->force)
return error("%s is a directory", path.buf);
remove_subtree(&path);
--
2.1.4
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Seems to be pushing more than necessary
On Wed, Mar 18, 2015 at 7:03 PM, Graham Hay wrote: > Are there any commands that I can use to show exactly what it is trying to > push? It's a bit more than a command. If you push when GIT_TRACE is set to 2, you'll see it executes "git pack-objects" command with all its arguments. This command expects some input from stdin. If you can capture that, you can run it by yourself to create the exact pack that is transferred over network. Run that pack through "git index-pack --verify-stat" will show you SHA-1 of all sent objects. It's quite a lot of work :) I created this script named "git" and put it in $PATH to capture input for pack-objects. You'll need to update "/path/to/real/git" to point to the real binary then you'll get /tmp/stdin -- 8< -- #!/bin/sh if [ "$1" = pack-objects ]; then exec tee /tmp/stdin | /path/to/real/git "$@" else exec /path/to/real/git "$@" fi -- 8< -- The remaining steps may be this (may need tweaking) git pack-objects '--all-progress-implied' '--revs' '--stdout' '--thin' '--delta-base-offset' '--progress' < /tmp/stdin | git index-pack --fix-thin --stdin pack708538afeda8eb331858680e227f7713228ce782 <-- new pack git verify-pack --verbose .git/objects/pack/pack-708538afeda8eb331858680e227f7713228ce782.pack d75631bd83ebdf03d4b0d925ff6734380f801fc6 commit 567 377 12 dd44100a7cdad113b23d31876e469b74fbe21e1b tree 15069 10492 389 8f4bbccea759d7a47616e29bd55b3f205b3615c2 tree 3869 2831 10881 3db0460935bc843a2a70a0e087222eec61a0ff0d blob 12379 3529 13712 Here we can see this push of mine sends four objects, 1 commit, 2 trees and 1 blob. -- Duy -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Seems to be pushing more than necessary
Are there any commands that I can use to show exactly what it is trying to push? I'll see if I can create a (public) repo that has the same problem. Thanks for your help. > This 10804 looks wrong (i.e. sending that many compressed objects). > Also "80 MiB" sent at that point. If you modify just a couple files, > something is really wrong because the number of new objects may be > hundreds at most, not thousands. > > v2.2.2 supports "git fast-export --anonymize" [1] to create an > anonymized "clone" of your repo that you can share, which might help > us understand the problem. > > There's also the environment variable GIT_TRACE_PACKET that can help > see what's going on at the protocol level, but I think you're on your > own because without access to this repo, SHA-1s from that trace may > not make much sense. > > [1] https://github.com/git/git/commit/a8722750985a53cc502a66ae3d68a9e42c7fdb98 > -- > Duy -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Bug] git gc with alternates tries accessing non-existing directory
On 18.03.2015 10:42, Jeff King wrote: On Wed, Mar 18, 2015 at 09:11:48AM +0100, Ephrim Khong wrote: I have a non-bare repository /home/a set up with an alternate to the bare repository /b. Running git gc on /home/a produces below's error [...] git --version git version 2.3.0 Try v2.3.2. It has b0a4264 (sha1_file: fix iterating loose alternate objects, 2015-02-08), which is almost certainly the problem. Indeed, the message is gone in v2.3.3. Thanks! - Eph -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Seems to be pushing more than necessary
On Wed, Mar 18, 2015 at 6:26 PM, Graham Hay wrote: >> It would help if you pasted the push output. For example, does it stop >> at 20% at the "compressing objects" line or "writing objects". How >> many total objects does it say? > > It rattles through "compressing objects", and the first 20% of > "writing objects", then slows to a crawl. > > Writing objects: 33% (3647/10804), 80.00 MiB | 112.00 KiB/s This 10804 looks wrong (i.e. sending that many compressed objects). Also "80 MiB" sent at that point. If you modify just a couple files, something is really wrong because the number of new objects may be hundreds at most, not thousands. v2.2.2 supports "git fast-export --anonymize" [1] to create an anonymized "clone" of your repo that you can share, which might help us understand the problem. There's also the environment variable GIT_TRACE_PACKET that can help see what's going on at the protocol level, but I think you're on your own because without access to this repo, SHA-1s from that trace may not make much sense. [1] https://github.com/git/git/commit/a8722750985a53cc502a66ae3d68a9e42c7fdb98 -- Duy -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Git with Lader logic
On Tue, Mar 17, 2015 at 11:33:54PM +, Bharat Suvarna wrote: > Hi > > I am trying to find a way of using version control on PLC programmers like > Allen Bradley PLC. I can't find a way of this. > > Could you please give me an idea if it will work with Plc programs. Which are > basically Ladder logic. > I'm not familiar with these programs, so I can't give you specific advice about this. Although git is not very picky about the contents, it is optimized to track text files. Things like showing diffs and merging files only works on text files. Git can track binary files, but there are some disadvantages: - Diff / merge doesn't work well - Compression is often difficult, so the repository size may grow depending on the size of the things stored These disadvantages are not limited to only git, other SCM systems also have to deal with these problems. So if the Ladder logic is represented as text source, there is no problem with it. If it'sbinary, there might be other sollutions better suited. Kevin -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Seems to be pushing more than necessary
> It would help if you pasted the push output. For example, does it stop > at 20% at the "compressing objects" line or "writing objects". How > many total objects does it say? It rattles through "compressing objects", and the first 20% of "writing objects", then slows to a crawl. Writing objects: 33% (3647/10804), 80.00 MiB | 112.00 KiB/s > > Another question is how big are these binary files on average? Git > considers a file is "big" if its size is 512MB or more (see > core.bigFileThreshold). If your binary files are are mostly under this > limit, but still big enough, then git may still try to compare new > objects with these to find the smallest "diff" to send. If it's the > case, you could set core.bigFileThreshold to cover these binary files. None of the files are very big (KB rather than MB), but there's a lot of them. I'll try setting the threshold to something lower, 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: Seems to be pushing more than necessary
On Wed, Mar 18, 2015 at 5:55 PM, Graham Hay wrote: > We have a fairly large repo (~2.4GB), mainly due to binary resources > (for an ios app). I know this can generally be a problem, but I have a > specific question. > > If I cut a branch, and edit a few (non-binary) files, and push, what > should be uploaded? I assumed it was just the diff (I know whole > compressed files are used, I mean the differences between my branch > and where I cut it from). Is that correct? > > Because when I push, it grinds to a halt at the 20% mark, and feels > like it's trying to push the entire repo. If I run "git diff --stat > --cached origin/foo" I see the files I would expect (i.e. just those > that have changed). If I run "git format-patch origin/foo..foo" the > patch files total 1.7MB, which should upload in just a few seconds, > but I've had pushes take over an hour. I'm using git 2.2.2 on Mac OS X > (Mavericks), and ssh (g...@github.com). > > Am I "doing it wrong"? Is this the expected behaviour? If not, is > there anything I can do to debug it? It would help if you pasted the push output. For example, does it stop at 20% at the "compressing objects" line or "writing objects". How many total objects does it say? Another question is how big are these binary files on average? Git considers a file is "big" if its size is 512MB or more (see core.bigFileThreshold). If your binary files are are mostly under this limit, but still big enough, then git may still try to compare new objects with these to find the smallest "diff" to send. If it's the case, you could set core.bigFileThreshold to cover these binary files. -- Duy -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Fwd: Seems to be pushing more than necessary
We have a fairly large repo (~2.4GB), mainly due to binary resources (for an ios app). I know this can generally be a problem, but I have a specific question. If I cut a branch, and edit a few (non-binary) files, and push, what should be uploaded? I assumed it was just the diff (I know whole compressed files are used, I mean the differences between my branch and where I cut it from). Is that correct? Because when I push, it grinds to a halt at the 20% mark, and feels like it's trying to push the entire repo. If I run "git diff --stat --cached origin/foo" I see the files I would expect (i.e. just those that have changed). If I run "git format-patch origin/foo..foo" the patch files total 1.7MB, which should upload in just a few seconds, but I've had pushes take over an hour. I'm using git 2.2.2 on Mac OS X (Mavericks), and ssh (g...@github.com). Am I "doing it wrong"? Is this the expected behaviour? If not, is there anything I can do to debug it? Any help gratefully received. Thanks, Graham -- To unsubscribe from this list: send the line "unsubscribe 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] rev-list: refuse --first-parent combined with --bisect
On Tue, Mar 17, 2015 at 9:46 PM, Junio C Hamano wrote: > Christian Couder writes: > >> On Tue, Mar 17, 2015 at 7:33 PM, Junio C Hamano wrote: >>> Christian Couder writes: >>> Yes, but the user is supposed to not change the "bad" pointer for no good reason. >>> >>> That is irrelevant, no? Nobody is questioning that the user is >>> supposed to judge if a commit is "good" or "bad" correctly. >> >> So if there is already a bad commit and the user gives another >> bad commit, that means that the user knows that it will replace the >> existing bad commit with the new one and that it's done for this >> purpose. > > ECANNOTQUITEPARSE. The user may say "git bisect bad $that" and we > do not question $that is bad. Git does not know better than the > user. > > But that does not mean Git does not know better than the user how > the current bad commit and $that commit are related. The user is > not interested in "replacing" at all. The user is telling just one > single fact, that is, "$that is bad". The user may make mistakes and try to fix them, like for example: $ git checkout master $ git bisect bad $ git log --oneline --decorate --graph --all # Ooops I was not on the right branch $ git checkout dev $ git bisect bad $ git log --oneline --decorate --graph --all # Everything looks ok now; the "bad" commit is what I expect # I can properly continue bisecting using "git bisect good"... In this case the user, who knows how git bisect works, expected that the second "git bisect bad" would fix the previous mistake made using the first "git bisect bad". If we make "git bisect bad" behave in another way we may break an advanced user's expectation. >>> I am not quite sure if I am correctly getting what you meant to say, >>> but if you meant "only when --alternate is given, we should do the >>> merge-base thing; we should keep losing the current 'bad' and >>> replace it with the new one without the --alternate option", I would >>> see that as an exercise of a bad taste. >> >> What I wanted to say is that if we change "git bisect bad ", >> so that now it means "add a new bad commit" instead of the previous >> "replace the current bad commit, if any, with this one", then experienced >> users might see that change as a regression in the user interface and >> it might even break scripts. > > Huh? > > Step back a bit. The place you need to start from is to admit the > fact that what "git bisect bad " currently does is > broken. > > Try creating this history yourself > > a---b---c---d---e---f > > and start bisection this way: > > $ git bisect start f c > $ git bisect bad a > > Immediately after the second command, "git bisect" moans > > Some good revs are not ancestor of the bad rev. > git bisect cannot work properly in this case. > Maybe you mistake good and bad revs? > > when it notices that the good rev (i.e. 'c') is no longer an > ancestor of the 'bad', which now points at 'a'. > > But that is because "git bisect bad" _blindly_ moved 'bad' that used > to point at 'f' to 'a', making a good rev (i.e. 'c') an ancestor of > the bad rev, without even bothering to check. Yeah, "git bisect bad" currently does what it is asked and then complains when it looks like a mistake has been made. You might see that as a bug. I am seeing that more as "git bisect" expecting users to know what they are doing. For example an advanced user might have realized that the first "git bisect start f c" was completely rubish for some reason, and the "git bisect bad a" might be a first step to fix that. (The next step might then be deleting the "good" pointer...) > Now, if we fixed this bug and made the bisect_state function more > careful (namely, when accepting "bad", make sure it is not beyond > any existing "good", or barf like the above, _without_ moving the > bad pointer), the user interface and behaviour would be changed. Is > that a regression? No, it is a usability fix and a progress. Yeah, you might see that as a usability fix and a progress. > Simply put, bisect_state function can become more careful and > intelligent to help users. Yeah, we can try to help users more, but doing that we might annoy some advanced users, who are used to the current way "git bisect" works, and perhaps break some scripts. -- To unsubscribe from this list: send the line "unsubscribe 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] not making corruption worse
On Tue, Mar 17, 2015 at 03:54:02PM -0700, Junio C Hamano wrote: > Jeff King writes: > > > But it strikes me as weird that we consider the _tips_ of history to be > > special for ignoring breakage. If the tip of "bar" is broken, we omit > > it. But if the tip is fine, and there's breakage three commits down in > > the history, then doing a clone is going to fail horribly, as > > pack-objects realizes it can't generate the pack. So in practice, I'm > > not sure how much you're buying with the "don't mention broken refs" > > code. > > I think this is a trade-off between strictness and convenience. Is > it preferrable that every time you try to clone a repository you get > reminded that one of its refs point at a bogus object and you > instead have to do "git fetch $there" with a refspec that excludes > the broken one, or is it OK to allow clones and fetches silently > succeed as if nothing is broken? I think the real issue is that we do not know on the server side what the client wants. Is it "tell me the refs, so I can grab just the one I need, and I don't care about the broken ones"? Or is it "I want everything you have, and tell me if you can't serve it"? You want strictness in the latter case, but not in the former. But if we were to err on the side of strictness, you could not do the former at all (because upload-pack would barf before the client even has a chance to say anything). I'm not sure if anyone will actually find GIT_REF_PARANOIA useful for something like that or not. As an environment variable, it may impact a filesystem-local clone, but it would not travel across a TCP connection. And doing so is tough, because the ref advertisement happens before the client speaks. If we ever have a client-speaks-first protocol, one extension could allow the client to flip the paranoia switch on the server. But my main goal here was really just making "prune" safer, so I'm happy enough with what this series does, for now. > In some parts of the system there is a movement to make this trade > off tweakable (hint: what happened to the knobs to fsck that allow > certain kinds of broken objects in the object store? did the topic > go anywhere?). This one so far lacked such a knob to tweak, and I > view your paranoia bit as such a knob. I think I promised several times to review that topic and never got around to it. Which makes me a bad person. It is still on my todo list. -Peff -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 00/14] numparse module: systematically tighten up integer parsing
On 03/18/2015 11:03 AM, Jeff King wrote:
> On Wed, Mar 18, 2015 at 10:47:40AM +0100, Michael Haggerty wrote:
>
>> But in case you have some reason that you want upload-pack.c to be
>> converted right away, I just pushed that change (plus some related
>> cleanups) to my GitHub repo [1]. The branch depends only on the first
>> patch of the "numparse" patch series.
>>
>> By the way, some other packet line parsing code in that file doesn't
>> verify that there are no trailing characters on the lines that they
>> process. That might be another thing that should be tightened up.
>
> Do you mean that upload-pack gets a pkt-line of length N that contains a
> line of length M, and then doesn't check that M==N? We use the space
> between M and N for passing capabilities and other metadata around.
>
> Or do you mean that we see lines like:
>
> want [0-9a-f]{40} ...\n
>
> and do not bother looking at the "..." that comes after the data we
> expect? That I can believe, and I don't think it would hurt to tighten
> up (we shouldn't need it for extensibility, as anybody trying to stick
> extra data there should do so only after using a capability flag earlier
> in the protocol).
The latter. For example here [1], the "have" command and its SHA-1 are
read from the line, but I don't see a check that there are no characters
after the SHA-1. The same here [2].
Michael
[1]
https://github.com/gitster/git/blob/9ab698f4000a736864c41f57fbae1e021ac27799/upload-pack.c#L404-L429
[2]
https://github.com/gitster/git/blob/9ab698f4000a736864c41f57fbae1e021ac27799/upload-pack.c#L550-L565
--
Michael Haggerty
mhag...@alum.mit.edu
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 00/14] numparse module: systematically tighten up integer parsing
On Wed, Mar 18, 2015 at 10:47:40AM +0100, Michael Haggerty wrote:
> But in case you have some reason that you want upload-pack.c to be
> converted right away, I just pushed that change (plus some related
> cleanups) to my GitHub repo [1]. The branch depends only on the first
> patch of the "numparse" patch series.
>
> By the way, some other packet line parsing code in that file doesn't
> verify that there are no trailing characters on the lines that they
> process. That might be another thing that should be tightened up.
Do you mean that upload-pack gets a pkt-line of length N that contains a
line of length M, and then doesn't check that M==N? We use the space
between M and N for passing capabilities and other metadata around.
Or do you mean that we see lines like:
want [0-9a-f]{40} ...\n
and do not bother looking at the "..." that comes after the data we
expect? That I can believe, and I don't think it would hurt to tighten
up (we shouldn't need it for extensibility, as anybody trying to stick
extra data there should do so only after using a capability flag earlier
in the protocol).
-Peff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 00/14] numparse module: systematically tighten up integer parsing
On Wed, Mar 18, 2015 at 4:47 PM, Michael Haggerty wrote: >> Thank you for doing it. I was about to write another number parser and >> you did it :D Maybe you can add another patch to convert the only >> strtol in upload-pack.c to parse_ui. This place should accept positive >> number in base 10, plus sign is not accepted. > > If the general direction of this patch series is accepted, I'll > gradually try to go through the codebase, replacing *all* integer > parsing with these functions. So there's no need to request particular > callers of strtol()/strtoul() to be converted; I'll get to them all > sooner or later (I hope). Good to know. No there's no hurry in converting this particular place. It's just tightening things up, not bug fixing or anything. -- Duy -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH/WIP 5/8] rebase: turn rebase--interactive into a separate program
---
Makefile | 2 +-
git-rebase--interactive.sh (mode +x) | 36 +++-
git-rebase.sh| 9 ++---
3 files changed, 38 insertions(+), 9 deletions(-)
mode change 100644 => 100755 git-rebase--interactive.sh
diff --git a/Makefile b/Makefile
index 7ee8df7..83972a2 100644
--- a/Makefile
+++ b/Makefile
@@ -446,6 +446,7 @@ SCRIPT_SH += git-pull.sh
SCRIPT_SH += git-quiltimport.sh
SCRIPT_SH += git-rebase.sh
SCRIPT_SH += git-rebase--am.sh
+SCRIPT_SH += git-rebase--interactive.sh
SCRIPT_SH += git-rebase--merge.sh
SCRIPT_SH += git-repack.sh
SCRIPT_SH += git-request-pull.sh
@@ -455,7 +456,6 @@ SCRIPT_SH += git-web--browse.sh
SCRIPT_LIB += git-mergetool--lib
SCRIPT_LIB += git-parse-remote
-SCRIPT_LIB += git-rebase--interactive
SCRIPT_LIB += git-sh-setup
SCRIPT_LIB += git-sh-i18n
diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
old mode 100644
new mode 100755
index 44901d5..b1c71a9
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -9,7 +9,14 @@
#
# The original idea comes from Eric W. Biederman, in
# http://article.gmane.org/gmane.comp.version-control.git/22407
-#
+
+. git-sh-setup
+. git-sh-i18n
+set_reflog_action "rebase -i ($action)"
+require_work_tree_exists
+
+GIT_QUIET=$git_quiet
+
# The file containing rebase commands, comments, and empty lines.
# This file is created by "git rebase -i" then edited by the user. As
# the lines are processed, they are removed from the front of this
@@ -80,6 +87,33 @@ rewritten_pending="$state_dir"/rewritten-pending
GIT_CHERRY_PICK_HELP="$resolvemsg"
export GIT_CHERRY_PICK_HELP
+write_basic_state () {
+ echo "$head_name" > "$state_dir"/head-name &&
+ echo "$onto" > "$state_dir"/onto &&
+ echo "$orig_head" > "$state_dir"/orig-head &&
+ echo "$GIT_QUIET" > "$state_dir"/quiet &&
+ test t = "$verbose" && : > "$state_dir"/verbose
+ test -n "$strategy" && echo "$strategy" > "$state_dir"/strategy
+ test -n "$strategy_opts" && echo "$strategy_opts" > \
+ "$state_dir"/strategy_opts
+ test -n "$allow_rerere_autoupdate" && echo "$allow_rerere_autoupdate" >
\
+ "$state_dir"/allow_rerere_autoupdate
+}
+
+output () {
+ case "$verbose" in
+ '')
+ output=$("$@" 2>&1 )
+ status=$?
+ test $status != 0 && printf "%s\n" "$output"
+ return $status
+ ;;
+ *)
+ "$@"
+ ;;
+ esac
+}
+
warn () {
printf '%s\n' "$*" >&2
}
diff --git a/git-rebase.sh b/git-rebase.sh
index ff2c2ae..86119e7 100755
--- a/git-rebase.sh
+++ b/git-rebase.sh
@@ -154,13 +154,8 @@ run_specific_rebase () {
export action allow_rerere_autoupdate git_am_opt git_quiet head_name
keep_empty
export onto orig_head rebase_root resolvemsg revisions
export state_dir verbose strategy strategy_opts
- if [ "$type" = am ]; then
- exec git-rebase--am
- elif [ "$type" = merge ]; then
- exec git-rebase--merge
- else
- . git-rebase--$type
- fi
+ export autosquash cmd force_rebase preserve_merges squash_onto
switch_to upstream
+ exec git-rebase--$type
}
run_pre_rebase_hook () {
--
2.3.0.rc1.137.g477eb31
--
To unsubscribe from this list: send the line "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/WIP 2/8] Move reset_tree from builtin/checkout.c to unpack-trees.c
---
builtin/checkout.c | 40 +++-
builtin/merge.c| 29 +++--
unpack-trees.c | 33 +
unpack-trees.h | 4
4 files changed, 47 insertions(+), 59 deletions(-)
diff --git a/builtin/checkout.c b/builtin/checkout.c
index a9c1b5a..93b18ad 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -362,40 +362,6 @@ static void describe_detached_head(const char *msg, struct
commit *commit)
strbuf_release(&sb);
}
-static int reset_tree(struct tree *tree, const struct checkout_opts *o,
- int worktree, int *writeout_error)
-{
- struct unpack_trees_options opts;
- struct tree_desc tree_desc;
-
- memset(&opts, 0, sizeof(opts));
- opts.head_idx = -1;
- opts.update = worktree;
- opts.skip_unmerged = !worktree;
- opts.reset = 1;
- opts.merge = 1;
- opts.fn = oneway_merge;
- opts.verbose_update = !o->quiet && isatty(2);
- opts.src_index = &the_index;
- opts.dst_index = &the_index;
- parse_tree(tree);
- init_tree_desc(&tree_desc, tree->buffer, tree->size);
- switch (unpack_trees(1, &tree_desc, &opts)) {
- case -2:
- *writeout_error = 1;
- /*
-* We return 0 nevertheless, as the index is all right
-* and more importantly we have made best efforts to
-* update paths in the work tree, and we cannot revert
-* them.
-*/
- case 0:
- return 0;
- default:
- return 128;
- }
-}
-
struct branch_info {
const char *name; /* The short name used */
const char *path; /* The full name of a real branch */
@@ -427,7 +393,7 @@ static int merge_working_tree(const struct checkout_opts
*opts,
resolve_undo_clear();
if (opts->force) {
- ret = reset_tree(new->commit->tree, opts, 1, writeout_error);
+ ret = reset_tree(new->commit->tree, opts->quiet, 1,
writeout_error);
if (ret)
return ret;
} else {
@@ -513,7 +479,7 @@ static int merge_working_tree(const struct checkout_opts
*opts,
o.verbosity = 0;
work = write_tree_from_memory(&o);
- ret = reset_tree(new->commit->tree, opts, 1,
+ ret = reset_tree(new->commit->tree, opts->quiet, 1,
writeout_error);
if (ret)
return ret;
@@ -522,7 +488,7 @@ static int merge_working_tree(const struct checkout_opts
*opts,
o.branch2 = "local";
merge_trees(&o, new->commit->tree, work,
old->commit->tree, &result);
- ret = reset_tree(new->commit->tree, opts, 0,
+ ret = reset_tree(new->commit->tree, opts->quiet, 0,
writeout_error);
if (ret)
return ret;
diff --git a/builtin/merge.c b/builtin/merge.c
index 6babf39..c51e4f5 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -269,33 +269,18 @@ static void read_empty(unsigned const char *sha1, int
verbose)
die(_("read-tree failed"));
}
-static void reset_hard(unsigned const char *sha1, int verbose)
-{
- int i = 0;
- const char *args[6];
-
- args[i++] = "read-tree";
- if (verbose)
- args[i++] = "-v";
- args[i++] = "--reset";
- args[i++] = "-u";
- args[i++] = sha1_to_hex(sha1);
- args[i] = NULL;
-
- if (run_command_v_opt(args, RUN_GIT_CMD))
- die(_("read-tree failed"));
-}
-
-static void restore_state(const unsigned char *head,
+static void restore_state(struct commit *head_commit,
const unsigned char *stash)
{
struct strbuf sb = STRBUF_INIT;
const char *args[] = { "stash", "apply", NULL, NULL };
+ int error = 0;
if (is_null_sha1(stash))
return;
- reset_hard(head, 1);
+ if (reset_tree(head_commit->tree, 0, 1, &error) || error)
+ die(_("read-tree failed"));
args[2] = sha1_to_hex(stash);
@@ -1409,7 +1394,7 @@ int cmd_merge(int argc, const char **argv, const char
*prefix)
int ret;
if (i) {
printf(_("Rewinding the tree to pristine...\n"));
- restore_state(head_commit->object.sha1, stash);
+ restore_state(head_commit, stash);
}
if (use_strategies_nr != 1)
printf(_("Trying merge strategy %s...\n"),
@@ -1475,7 +1460,7 @@ int cmd_merge(int argc, const char **argv, const char
*prefix)
* it up.
*/
[PATCH/WIP 4/8] rebase: turn rebase--merge into a separate program
---
Makefile | 2 +-
git-rebase--merge.sh (mode +x) | 34 ++
git-rebase.sh | 2 ++
3 files changed, 37 insertions(+), 1 deletion(-)
mode change 100644 => 100755 git-rebase--merge.sh
diff --git a/Makefile b/Makefile
index 93151f4..7ee8df7 100644
--- a/Makefile
+++ b/Makefile
@@ -446,6 +446,7 @@ SCRIPT_SH += git-pull.sh
SCRIPT_SH += git-quiltimport.sh
SCRIPT_SH += git-rebase.sh
SCRIPT_SH += git-rebase--am.sh
+SCRIPT_SH += git-rebase--merge.sh
SCRIPT_SH += git-repack.sh
SCRIPT_SH += git-request-pull.sh
SCRIPT_SH += git-stash.sh
@@ -455,7 +456,6 @@ SCRIPT_SH += git-web--browse.sh
SCRIPT_LIB += git-mergetool--lib
SCRIPT_LIB += git-parse-remote
SCRIPT_LIB += git-rebase--interactive
-SCRIPT_LIB += git-rebase--merge
SCRIPT_LIB += git-sh-setup
SCRIPT_LIB += git-sh-i18n
diff --git a/git-rebase--merge.sh b/git-rebase--merge.sh
old mode 100644
new mode 100755
index b10f2cf..baaef41
--- a/git-rebase--merge.sh
+++ b/git-rebase--merge.sh
@@ -3,8 +3,42 @@
# Copyright (c) 2010 Junio C Hamano.
#
+. git-sh-setup
+. git-sh-i18n
+set_reflog_action rebase
+require_work_tree_exists
+
+GIT_QUIET=$git_quiet
+
prec=4
+write_basic_state () {
+ echo "$head_name" > "$state_dir"/head-name &&
+ echo "$onto" > "$state_dir"/onto &&
+ echo "$orig_head" > "$state_dir"/orig-head &&
+ echo "$GIT_QUIET" > "$state_dir"/quiet &&
+ test t = "$verbose" && : > "$state_dir"/verbose
+ test -n "$strategy" && echo "$strategy" > "$state_dir"/strategy
+ test -n "$strategy_opts" && echo "$strategy_opts" > \
+ "$state_dir"/strategy_opts
+ test -n "$allow_rerere_autoupdate" && echo "$allow_rerere_autoupdate" >
\
+ "$state_dir"/allow_rerere_autoupdate
+}
+
+move_to_original_branch () {
+ case "$head_name" in
+ refs/*)
+ message="rebase finished: $head_name onto $onto"
+ git update-ref -m "$message" \
+ $head_name $(git rev-parse HEAD) $orig_head &&
+ git symbolic-ref \
+ -m "rebase finished: returning to $head_name" \
+ HEAD $head_name ||
+ die "$(gettext "Could not move back to $head_name")"
+ ;;
+ esac
+}
+
read_state () {
onto_name=$(cat "$state_dir"/onto_name) &&
end=$(cat "$state_dir"/end) &&
diff --git a/git-rebase.sh b/git-rebase.sh
index 42e868d..ff2c2ae 100755
--- a/git-rebase.sh
+++ b/git-rebase.sh
@@ -156,6 +156,8 @@ run_specific_rebase () {
export state_dir verbose strategy strategy_opts
if [ "$type" = am ]; then
exec git-rebase--am
+ elif [ "$type" = merge ]; then
+ exec git-rebase--merge
else
. git-rebase--$type
fi
--
2.3.0.rc1.137.g477eb31
--
To unsubscribe from this list: send the line "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/WIP 6/8] rebase: remove unused function
git-rebase.sh is no longer a dependency for rebase subscripts. This
function is only used by subscripts only, which now becomes useless.
---
git-rebase.sh | 13 -
1 file changed, 13 deletions(-)
diff --git a/git-rebase.sh b/git-rebase.sh
index 86119e7..d941239 100755
--- a/git-rebase.sh
+++ b/git-rebase.sh
@@ -102,19 +102,6 @@ read_basic_state () {
allow_rerere_autoupdate="$(cat
"$state_dir"/allow_rerere_autoupdate)"
}
-write_basic_state () {
- echo "$head_name" > "$state_dir"/head-name &&
- echo "$onto" > "$state_dir"/onto &&
- echo "$orig_head" > "$state_dir"/orig-head &&
- echo "$GIT_QUIET" > "$state_dir"/quiet &&
- test t = "$verbose" && : > "$state_dir"/verbose
- test -n "$strategy" && echo "$strategy" > "$state_dir"/strategy
- test -n "$strategy_opts" && echo "$strategy_opts" > \
- "$state_dir"/strategy_opts
- test -n "$allow_rerere_autoupdate" && echo "$allow_rerere_autoupdate" >
\
- "$state_dir"/allow_rerere_autoupdate
-}
-
output () {
case "$verbose" in
'')
--
2.3.0.rc1.137.g477eb31
--
To unsubscribe from this list: send the line "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/WIP 8/8] Build in rebase
---
Makefile| 2 +-
builtin.h | 1 +
builtin/rebase.c (new) | 752
commit.c| 4 +-
commit.h| 4 +-
contrib/examples/git-rebase.sh (new +x) | 532 ++
git-rebase.sh (gone)| 532 --
git.c | 1 +
8 files changed, 1291 insertions(+), 537 deletions(-)
create mode 100644 builtin/rebase.c
create mode 100755 contrib/examples/git-rebase.sh
delete mode 100755 git-rebase.sh
diff --git a/Makefile b/Makefile
index 83972a2..223d50b 100644
--- a/Makefile
+++ b/Makefile
@@ -444,7 +444,6 @@ SCRIPT_SH += git-merge-resolve.sh
SCRIPT_SH += git-mergetool.sh
SCRIPT_SH += git-pull.sh
SCRIPT_SH += git-quiltimport.sh
-SCRIPT_SH += git-rebase.sh
SCRIPT_SH += git-rebase--am.sh
SCRIPT_SH += git-rebase--interactive.sh
SCRIPT_SH += git-rebase--merge.sh
@@ -903,6 +902,7 @@ BUILTIN_OBJS += builtin/prune-packed.o
BUILTIN_OBJS += builtin/prune.o
BUILTIN_OBJS += builtin/push.o
BUILTIN_OBJS += builtin/read-tree.o
+BUILTIN_OBJS += builtin/rebase.o
BUILTIN_OBJS += builtin/receive-pack.o
BUILTIN_OBJS += builtin/reflog.o
BUILTIN_OBJS += builtin/remote.o
diff --git a/builtin.h b/builtin.h
index 7e7bbd6..431bbbf 100644
--- a/builtin.h
+++ b/builtin.h
@@ -108,6 +108,7 @@ extern int cmd_prune(int argc, const char **argv, const
char *prefix);
extern int cmd_prune_packed(int argc, const char **argv, const char *prefix);
extern int cmd_push(int argc, const char **argv, const char *prefix);
extern int cmd_read_tree(int argc, const char **argv, const char *prefix);
+extern int cmd_rebase(int argc, const char **argv, const char *prefix);
extern int cmd_receive_pack(int argc, const char **argv, const char *prefix);
extern int cmd_reflog(int argc, const char **argv, const char *prefix);
extern int cmd_remote(int argc, const char **argv, const char *prefix);
diff --git a/builtin/rebase.c b/builtin/rebase.c
new file mode 100644
index 000..29eff0e
--- /dev/null
+++ b/builtin/rebase.c
@@ -0,0 +1,752 @@
+#include "cache.h"
+#include "parse-options.h"
+#include "argv-array.h"
+#include "run-command.h"
+#include "tree-walk.h"
+#include "unpack-trees.h"
+#include "diff.h"
+#include "commit.h"
+#include "revision.h"
+#include "submodule.h"
+#include "commit.h"
+#include "dir.h"
+
+#define REBASE_AM 1
+#define REBASE_MERGE 2
+#define REBASE_INTERACTIVE 3
+
+static const char * const builtin_rebase_usage[] = {
+ N_("git rebase [-i] [options] [--exec ] [--onto ]
[] []"),
+ N_("git rebase [-i] [options] [--exec ] [--onto ] --root
[]"),
+ N_("git rebase --continue | --abort | --skip | --edit-todo"),
+ NULL
+};
+
+/* These are exported as environment variables for git-rebase--*.sh */
+static int action_abort;
+static int action_continue;
+static int action_skip;
+static int autosquash;
+static int force_rebase;
+static int keep_empty;
+static int preserve_merges;
+static int quiet;
+static int rerere_autoupdate;
+static int root;
+static int verbose;
+static const char *exec_cmd;
+static const char *head_name;
+static const char *onto;
+static const char *orig_head;
+static struct strbuf revisions = STRBUF_INIT;
+static const char *state_basedir;
+static const char *strategy;
+static const char *strategy_opts;
+static const char *switch_to;
+static const char *squash_onto;
+
+static int do_merge;
+static int edit_todo;
+static int interactive_rebase;
+static int rebase_type;
+static int show_stat;
+
+static char *read_file(const char *name)
+{
+ struct strbuf sb = STRBUF_INIT;
+ if (strbuf_read_file(&sb,
+git_path("%s/%s", state_basedir, name),
+0) >= 0)
+ return strbuf_detach(&sb, NULL);
+ else
+ return NULL;
+}
+
+static char *read_file_or_die(const char *name)
+{
+ struct strbuf sb = STRBUF_INIT;
+ strbuf_read_file_or_die(&sb,
+ git_path("%s/%s", state_basedir, name),
+ 0);
+ return strbuf_detach(&sb, NULL);
+}
+
+static void read_bool(const char *name, int *var)
+{
+ char *buf = read_file(name);
+ if (buf) {
+ *var = buf[0] && !isspace(buf[0]);
+ free(buf);
+ }
+}
+
+static int read_bool_or_die(const char *name)
+{
+ char *buf = read_file_or_die(name);
+ int ret = buf[0] && !isspace(buf[0]);
+ free(buf);
+ return ret;
+}
+
+/*
+ * Note:
+ *
+ * After git-rebase--*.sh are integrated, we should probably adopt
+ * git-config format and store everything in one file instead of so
+ * many like this. state_dir will be something different to avoid
+ * misuse by old rebase versions. This code will stay for a few major
+ * releases before being phased out.
+ */
+static void read
[PATCH/WIP 7/8] rebase: move resolvemsg to rebase--* scripts
---
git-rebase--am.sh | 5 +
git-rebase--interactive.sh | 5 +
git-rebase--merge.sh | 5 +
git-rebase.sh | 7 +--
4 files changed, 16 insertions(+), 6 deletions(-)
diff --git a/git-rebase--am.sh b/git-rebase--am.sh
index ab84330..399956b 100755
--- a/git-rebase--am.sh
+++ b/git-rebase--am.sh
@@ -9,6 +9,11 @@ set_reflog_action rebase
require_work_tree_exists
GIT_QUIET=$git_quiet
+resolvemsg="
+$(gettext 'When you have resolved this problem, run "git rebase --continue".
+If you prefer to skip this patch, run "git rebase --skip" instead.
+To check out the original branch and stop rebasing, run "git rebase --abort".')
+"
write_basic_state () {
echo "$head_name" > "$state_dir"/head-name &&
diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index b1c71a9..337dec3 100755
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -16,6 +16,11 @@ set_reflog_action "rebase -i ($action)"
require_work_tree_exists
GIT_QUIET=$git_quiet
+resolvemsg="
+$(gettext 'When you have resolved this problem, run "git rebase --continue".
+If you prefer to skip this patch, run "git rebase --skip" instead.
+To check out the original branch and stop rebasing, run "git rebase --abort".')
+"
# The file containing rebase commands, comments, and empty lines.
# This file is created by "git rebase -i" then edited by the user. As
diff --git a/git-rebase--merge.sh b/git-rebase--merge.sh
index baaef41..7e240be 100755
--- a/git-rebase--merge.sh
+++ b/git-rebase--merge.sh
@@ -9,6 +9,11 @@ set_reflog_action rebase
require_work_tree_exists
GIT_QUIET=$git_quiet
+resolvemsg="
+$(gettext 'When you have resolved this problem, run "git rebase --continue".
+If you prefer to skip this patch, run "git rebase --skip" instead.
+To check out the original branch and stop rebasing, run "git rebase --abort".')
+"
prec=4
diff --git a/git-rebase.sh b/git-rebase.sh
index d941239..38530e8 100755
--- a/git-rebase.sh
+++ b/git-rebase.sh
@@ -49,11 +49,6 @@ cd_to_toplevel
LF='
'
ok_to_skip_pre_rebase=
-resolvemsg="
-$(gettext 'When you have resolved this problem, run "git rebase --continue".
-If you prefer to skip this patch, run "git rebase --skip" instead.
-To check out the original branch and stop rebasing, run "git rebase --abort".')
-"
unset onto
cmd=
strategy=
@@ -139,7 +134,7 @@ run_specific_rebase () {
git_quiet=$GIT_QUIET
export GIT_PAGER
export action allow_rerere_autoupdate git_am_opt git_quiet head_name
keep_empty
- export onto orig_head rebase_root resolvemsg revisions
+ export onto orig_head rebase_root revisions
export state_dir verbose strategy strategy_opts
export autosquash cmd force_rebase preserve_merges squash_onto
switch_to upstream
exec git-rebase--$type
--
2.3.0.rc1.137.g477eb31
--
To unsubscribe from this list: send the line "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/WIP 3/8] rebase: turn rebase--am into a separate program
---
Makefile| 2 +-
git-rebase--am.sh (mode +x) | 34 ++
git-rebase.sh | 11 ++-
3 files changed, 45 insertions(+), 2 deletions(-)
mode change 100644 => 100755 git-rebase--am.sh
diff --git a/Makefile b/Makefile
index 1b30d7b..93151f4 100644
--- a/Makefile
+++ b/Makefile
@@ -445,6 +445,7 @@ SCRIPT_SH += git-mergetool.sh
SCRIPT_SH += git-pull.sh
SCRIPT_SH += git-quiltimport.sh
SCRIPT_SH += git-rebase.sh
+SCRIPT_SH += git-rebase--am.sh
SCRIPT_SH += git-repack.sh
SCRIPT_SH += git-request-pull.sh
SCRIPT_SH += git-stash.sh
@@ -453,7 +454,6 @@ SCRIPT_SH += git-web--browse.sh
SCRIPT_LIB += git-mergetool--lib
SCRIPT_LIB += git-parse-remote
-SCRIPT_LIB += git-rebase--am
SCRIPT_LIB += git-rebase--interactive
SCRIPT_LIB += git-rebase--merge
SCRIPT_LIB += git-sh-setup
diff --git a/git-rebase--am.sh b/git-rebase--am.sh
old mode 100644
new mode 100755
index 97f31dc..ab84330
--- a/git-rebase--am.sh
+++ b/git-rebase--am.sh
@@ -3,6 +3,40 @@
# Copyright (c) 2010 Junio C Hamano.
#
+. git-sh-setup
+. git-sh-i18n
+set_reflog_action rebase
+require_work_tree_exists
+
+GIT_QUIET=$git_quiet
+
+write_basic_state () {
+ echo "$head_name" > "$state_dir"/head-name &&
+ echo "$onto" > "$state_dir"/onto &&
+ echo "$orig_head" > "$state_dir"/orig-head &&
+ echo "$GIT_QUIET" > "$state_dir"/quiet &&
+ test t = "$verbose" && : > "$state_dir"/verbose
+ test -n "$strategy" && echo "$strategy" > "$state_dir"/strategy
+ test -n "$strategy_opts" && echo "$strategy_opts" > \
+ "$state_dir"/strategy_opts
+ test -n "$allow_rerere_autoupdate" && echo "$allow_rerere_autoupdate" >
\
+ "$state_dir"/allow_rerere_autoupdate
+}
+
+move_to_original_branch () {
+ case "$head_name" in
+ refs/*)
+ message="rebase finished: $head_name onto $onto"
+ git update-ref -m "$message" \
+ $head_name $(git rev-parse HEAD) $orig_head &&
+ git symbolic-ref \
+ -m "rebase finished: returning to $head_name" \
+ HEAD $head_name ||
+ die "$(gettext "Could not move back to $head_name")"
+ ;;
+ esac
+}
+
case "$action" in
continue)
git am --resolved --resolvemsg="$resolvemsg" &&
diff --git a/git-rebase.sh b/git-rebase.sh
index b2f1c76..42e868d 100755
--- a/git-rebase.sh
+++ b/git-rebase.sh
@@ -149,7 +149,16 @@ run_specific_rebase () {
export GIT_EDITOR
autosquash=
fi
- . git-rebase--$type
+ git_quiet=$GIT_QUIET
+ export GIT_PAGER
+ export action allow_rerere_autoupdate git_am_opt git_quiet head_name
keep_empty
+ export onto orig_head rebase_root resolvemsg revisions
+ export state_dir verbose strategy strategy_opts
+ if [ "$type" = am ]; then
+ exec git-rebase--am
+ else
+ . git-rebase--$type
+ fi
}
run_pre_rebase_hook () {
--
2.3.0.rc1.137.g477eb31
--
To unsubscribe from this list: send the line "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/WIP 1/8] strbuf: add and use strbuf_read_file_or_die()
---
builtin/blame.c | 4 ++--
builtin/commit.c | 16 +---
builtin/merge.c | 3 +--
builtin/notes.c | 4 ++--
builtin/tag.c| 7 ++-
strbuf.c | 8
strbuf.h | 1 +
7 files changed, 21 insertions(+), 22 deletions(-)
diff --git a/builtin/blame.c b/builtin/blame.c
index bc6c899..503595c 100644
--- a/builtin/blame.c
+++ b/builtin/blame.c
@@ -2193,8 +2193,8 @@ static struct commit *fake_working_tree_commit(struct
diff_options *opt,
if (DIFF_OPT_TST(opt, ALLOW_TEXTCONV) &&
textconv_object(read_from, mode, null_sha1, 0,
&buf_ptr, &buf_len))
strbuf_attach(&buf, buf_ptr, buf_len, buf_len +
1);
- else if (strbuf_read_file(&buf, read_from, st.st_size)
!= st.st_size)
- die_errno("cannot open or read '%s'",
read_from);
+ else
+ strbuf_read_file_or_die(&buf, read_from,
st.st_size);
break;
case S_IFLNK:
if (strbuf_readlink(&buf, read_from, st.st_size) < 0)
diff --git a/builtin/commit.c b/builtin/commit.c
index d6dd3df..dad9acf 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -612,9 +612,7 @@ static int prepare_to_commit(const char *index_file, const
char *prefix,
die_errno(_("could not read log from standard input"));
hook_arg1 = "message";
} else if (logfile) {
- if (strbuf_read_file(&sb, logfile, 0) < 0)
- die_errno(_("could not read log file '%s'"),
- logfile);
+ strbuf_read_file_or_die(&sb, logfile, 0);
hook_arg1 = "message";
} else if (use_message) {
buffer = strstr(use_message_buffer, "\n\n");
@@ -634,16 +632,13 @@ static int prepare_to_commit(const char *index_file,
const char *prefix,
&sb, &ctx);
hook_arg1 = "message";
} else if (!stat(git_path("MERGE_MSG"), &statbuf)) {
- if (strbuf_read_file(&sb, git_path("MERGE_MSG"), 0) < 0)
- die_errno(_("could not read MERGE_MSG"));
+ strbuf_read_file_or_die(&sb, git_path("MERGE_MSG"), 0);
hook_arg1 = "merge";
} else if (!stat(git_path("SQUASH_MSG"), &statbuf)) {
- if (strbuf_read_file(&sb, git_path("SQUASH_MSG"), 0) < 0)
- die_errno(_("could not read SQUASH_MSG"));
+ strbuf_read_file_or_die(&sb, git_path("SQUASH_MSG"), 0);
hook_arg1 = "squash";
} else if (template_file) {
- if (strbuf_read_file(&sb, template_file, 0) < 0)
- die_errno(_("could not read '%s'"), template_file);
+ strbuf_read_file_or_die(&sb, template_file, 0);
hook_arg1 = "template";
clean_message_contents = 0;
}
@@ -1497,8 +1492,7 @@ int cmd_commit(int argc, const char **argv, const char
*prefix)
fclose(fp);
strbuf_release(&m);
if (!stat(git_path("MERGE_MODE"), &statbuf)) {
- if (strbuf_read_file(&sb, git_path("MERGE_MODE"), 0) <
0)
- die_errno(_("could not read MERGE_MODE"));
+ strbuf_read_file_or_die(&sb, git_path("MERGE_MODE"), 0);
if (!strcmp(sb.buf, "no-ff"))
allow_fast_forward = 0;
}
diff --git a/builtin/merge.c b/builtin/merge.c
index 9307e9c..6babf39 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -769,8 +769,7 @@ static void read_merge_msg(struct strbuf *msg)
{
const char *filename = git_path("MERGE_MSG");
strbuf_reset(msg);
- if (strbuf_read_file(msg, filename, 0) < 0)
- die_errno(_("Could not read from '%s'"), filename);
+ strbuf_read_file_or_die(msg, filename, 0);
}
static void write_merge_state(struct commit_list *);
diff --git a/builtin/notes.c b/builtin/notes.c
index 453457a..3210c7f 100644
--- a/builtin/notes.c
+++ b/builtin/notes.c
@@ -252,8 +252,8 @@ static int parse_file_arg(const struct option *opt, const
char *arg, int unset)
if (!strcmp(arg, "-")) {
if (strbuf_read(&(msg->buf), 0, 1024) < 0)
die_errno(_("cannot read '%s'"), arg);
- } else if (strbuf_read_file(&(msg->buf), arg, 1024) < 0)
- die_errno(_("could not open or read '%s'"), arg);
+ } else
+ strbuf_read_file_or_die(&(msg->buf), arg, 0);
stripspace(&(msg->buf), 0);
msg->given = 1;
diff --git a/builtin/tag.c b/builtin/tag.c
index 9c3e067..69f4ea3 100644
--- a/builtin/tag.c
+++ b/builtin/tag.c
@@ -540,11 +540,8 @@ int cmd_tag(int argc, const char **argv, const char
*prefix)
if (!strc
[PATCH/WIP 0/8] Convert git-rebase.sh to C
In the spirit of sharing code proactively [1], despite my embarrassment, this is what I got for converting git-rebase.sh to C. Note that this is only about git-rebase.sh, not git-rebase--*.sh. Some changes in git-rebase.sh are pushed back to git-rebase--*.sh. The idea is we convert git-rebase.sh first, then the rest later. Anybody who wants to base their work on this code, feel free to --reset-author (and take all the bugs with you, I'm sure there are lots of them). This work is from 2013. git-rebase.sh has received lots of updates since then, so there's still lots of work to do. [1] http://article.gmane.org/gmane.comp.version-control.git/265699 Nguyễn Thái Ngọc Duy (8): strbuf: add and use strbuf_read_file_or_die() Move reset_tree from builtin/checkout.c to unpack-trees.c rebase: turn rebase--am into a separate program rebase: turn rebase--merge into a separate program rebase: turn rebase--interactive into a separate program rebase: remove unused function rebase: move resolvemsg to rebase--* scripts Build in rebase Makefile| 8 +- builtin.h | 1 + builtin/blame.c | 4 +- builtin/checkout.c | 40 +- builtin/commit.c| 16 +- builtin/merge.c | 32 +- builtin/notes.c | 4 +- builtin/rebase.c (new) | 752 builtin/tag.c | 7 +- commit.c| 4 +- commit.h| 4 +- contrib/examples/git-rebase.sh (new +x) | 532 ++ git-rebase--am.sh (mode +x) | 39 ++ git-rebase--interactive.sh (mode +x)| 41 +- git-rebase--merge.sh (mode +x) | 39 ++ git-rebase.sh (gone)| 544 --- git.c | 1 + strbuf.c| 8 + strbuf.h| 1 + unpack-trees.c | 33 ++ unpack-trees.h | 4 + 21 files changed, 1480 insertions(+), 634 deletions(-) create mode 100644 builtin/rebase.c create mode 100755 contrib/examples/git-rebase.sh mode change 100644 => 100755 git-rebase--am.sh mode change 100644 => 100755 git-rebase--interactive.sh mode change 100644 => 100755 git-rebase--merge.sh delete mode 100755 git-rebase.sh -- 2.3.0.rc1.137.g477eb31 -- To unsubscribe from this list: send the line "unsubscribe 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/14] numparse module: systematically tighten up integer parsing
On 03/18/2015 12:05 AM, Duy Nguyen wrote: > On Tue, Mar 17, 2015 at 11:00 PM, Michael Haggerty > wrote: >> Michael Haggerty (14): >> numparse: new module for parsing integral numbers >> cacheinfo_callback(): use convert_ui() when handling "--cacheinfo" >> write_subdirectory(): use convert_ui() for parsing mode >> handle_revision_opt(): use skip_prefix() in many places >> handle_revision_opt(): use convert_i() when handling "-" >> strtoul_ui(), strtol_i(): remove functions >> handle_revision_opt(): use convert_ui() when handling "--abbrev=" >> builtin_diff(): detect errors when parsing --unified argument >> opt_arg(): val is always non-NULL >> opt_arg(): use convert_i() in implementation >> opt_arg(): report errors parsing option values >> opt_arg(): simplify pointer handling >> diff_opt_parse(): use convert_i() when handling "-l" >> diff_opt_parse(): use convert_i() when handling --abbrev= > > Thank you for doing it. I was about to write another number parser and > you did it :D Maybe you can add another patch to convert the only > strtol in upload-pack.c to parse_ui. This place should accept positive > number in base 10, plus sign is not accepted. If the general direction of this patch series is accepted, I'll gradually try to go through the codebase, replacing *all* integer parsing with these functions. So there's no need to request particular callers of strtol()/strtoul() to be converted; I'll get to them all sooner or later (I hope). But in case you have some reason that you want upload-pack.c to be converted right away, I just pushed that change (plus some related cleanups) to my GitHub repo [1]. The branch depends only on the first patch of the "numparse" patch series. By the way, some other packet line parsing code in that file doesn't verify that there are no trailing characters on the lines that they process. That might be another thing that should be tightened up. Michael [1] https://github.com/mhagger/git.git, branch "upload-pack-numparse" -- Michael Haggerty mhag...@alum.mit.edu -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Bug] git gc with alternates tries accessing non-existing directory
On Wed, Mar 18, 2015 at 09:11:48AM +0100, Ephrim Khong wrote: > I have a non-bare repository /home/a set up with an alternate to the bare > repository /b. Running git gc on /home/a produces below's error > [...] > > git --version > git version 2.3.0 Try v2.3.2. It has b0a4264 (sha1_file: fix iterating loose alternate objects, 2015-02-08), which is almost certainly the problem. -Peff -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH/RFC/GSOC] make git-pull a builtin
Hi Stephen, On 2015-03-18 09:38, Stephen Robin wrote: >> Paul Tan writes: >> I would like to share this very rough prototype with everyone. > ... >> I started this as a just-for-fun exercise to learn about the git internal > API > > I started to rewrite git-pull for similar reasons a couple of months ago, > but I haven't had time to complete it. It looks like my version has less > work remaining than yours, would you like me to share it? Hmm. It always results in an unnecessary round trip if you ask people to ask you to share it. At this point, because Paul already made his work public, I would say that we should continue with his version. Please understand this as an encouragement for the future to share your code pro-actively, just like Git's own source code has been shared with you without the need for you to request it explicitly. For the record, Duy also already shared his C code to implement `git pull` in C publicly. I think that Paul decided to start again in order to understand every detail of the process of converting a shell script into a builtin; If that was the rationale, I would agree that it is a wise one. Ciao, Johannes -- To unsubscribe from this list: send the line "unsubscribe 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/GSOC] make git-pull a builtin
Hi Paul,
thank you for this very detailed mail. It was a real pleasure to read this
well-researched document.
In the following, I will pick out only parts from the mail, in the interest of
both of our time. Please assume that I agree with everything that I do not
quote below (and even the with quoted parts I agree mostly ;-)).
On 2015-03-17 14:57, Paul Tan wrote:
> on Windows the runtime fell from 8m 25s to 1m 3s.
This is *exactly* the type of benefit that makes this project so important!
Nice one.
> A simpler, but less perfect strategy might be to just convert the shell
> scripts directly statement by statement to C, using the run_command*()
> functions as Duy Nguyen[2] suggested, before changing the code to use
> the internal API.
Yeah, the idea is to have a straight-forward strategy to convert the scripts in
as efficient manner as possible. It also makes reviewing easier if the first
step is an almost one-to-one translation to `run_command*()`-based builtins.
Plus, it is rewarding to have concise steps that can be completed in a timely
manner.
> +/* NOTE: git-pull.sh only supports --log and --no-log, as opposed to what
> + * man git-pull says. */
> +static int opt_shortlog_len;
This comment might want to live in the commit message instead... It is prone to
get stale in the code while it will always be correct (and easier to spot) in
the commit message.
> +/**
> + * Returns default rebase option value
> + */
> +static int rebase_config_default(void)
> +{
> + struct strbuf name = STRBUF_INIT;
> + const char *value = NULL;
> + int boolval;
> +
> + strbuf_addf(&name, "branch.%s.rebase", head_name_short);
> + if (git_config_get_value(name.buf, &value))
> + git_config_get_value("pull.rebase", &value);
> + strbuf_release(&name);
> + if (!value)
> + return REBASE_FALSE;
> + boolval = git_config_maybe_bool("pull.rebase", value);
> + if (boolval >= 0)
> + return boolval ? REBASE_TRUE : REBASE_FALSE;
> + else if (value && !strcmp(value, "preserve"))
> + return REBASE_PRESERVE;
> + die(_("invalid value for branch.%s.rebase \"%s\""), head_name_short,
> value);
> +}
Personally, I would use a couple of empty lines for enhanced structure.
Conceptually, there are four parts: the variable declarations, querying the
config, parsing the value, and `die()`ing in case of error. There is already an
empty line between the first two parts, and I would imagine that readability
would be improved further by having an empty line after the `strbuf_release()`
and the `return REBASE_PRESERVE` statement, respectively.
> +static int parse_opt_rebase(const struct option *opt, const char
> *arg, int unset)
> +{
> + if (arg)
> + *(int *)opt->value = parse_rebase(arg);
> + else
> + *(int *)opt->value = unset ? REBASE_FALSE : REBASE_TRUE;
> + return (*(int *)opt->value) >= 0 ? 0 : -1;
> +}
In this function (and also in other places below), there is this pattern that a
`struct option` pointer is passed to the function, but then only `*(int
*)opt->value` is written to. Therefore, I would suggest to change the signature
of the function and pass `(int *)opt->value` as function parameter.
> +static int has_unstaged_changes(void)
Yeah, this function, as well as the ones below it, look as if they are so
common that they *should* be already somewhere in libgit.a. But I did not find
them, either...
Of course it *would* be nice to identify places where essentially the same code
is needed, and refactor accordingly. But I think that is outside the scope of
this project.
The rest looks pretty good (and you realize that my comments above are really
more nit picks than anything else).
The FIXMEs should be relatively easy to address. It would be my pleasure to
work with you.
Ciao,
Johannes
--
To unsubscribe from this list: send the line "unsubscribe 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/GSOC] make git-pull a builtin
> Paul Tan writes: > I would like to share this very rough prototype with everyone. ... > I started this as a just-for-fun exercise to learn about the git internal API I started to rewrite git-pull for similar reasons a couple of months ago, but I haven't had time to complete it. It looks like my version has less work remaining than yours, would you like me to share it? > Finally, there is the possibility that in the process of conversion bugs or incompatible behavioral changes may be introduced which are not caught by the test suite. Ideally, I think the solution is to improve the test suite and make it as comprehensive as possible, but writing a comprehensive test suite may be too time consuming. This is why I haven't had time to finish and submit my patch. While the c code is pretty much complete, I felt the test suite needed some extension before I could be confident the new code is correct. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[v3 PATCH 2/2] reset: add tests for git reset -
The failure case which occurs on teaching git is taught the '-' shorthand
is when there exists no branch pointed to by '@{-1}'. But, if there is a file
named - in the working tree, the user can be unambiguously assumed to be
referring to it while issuing this command.
The ambiguous case occurs when the @{-1} branch exists and file named '-' also
exists in the working tree. This are also treated as a failure case but here
the user is given advice as to how he can proceed.
Another potentially tricky case is when the file '@{-1}' exists. In this case,
the command should succeed as the user doesn't mention the file '@{-1}' and can
be safely assumed to be referring to the @{-1} branch.
Add tests to check the handling of these cases.
Also add a test to verify that reset - behaves like reset @{-1} when none
of the above cases are true.
Helped-by: Junio C Hamano
Helped-by: Eric Sunshine
Helped-by: Matthieu Moy
Signed-off-by: Sundararajan R
---
Thank you for your feedback Torsten and Eric. I have made modifications
suggested
by you. I have also acted on Matthieu's suggestions on the archive.
Please let me know if there is something else I should add.
I have also removed one irrelevant test from which we come to know of nothing
new.
t/t7102-reset.sh | 75
1 file changed, 75 insertions(+)
diff --git a/t/t7102-reset.sh b/t/t7102-reset.sh
index 98bcfe2..f5a8e76 100755
--- a/t/t7102-reset.sh
+++ b/t/t7102-reset.sh
@@ -568,4 +568,79 @@ test_expect_success 'reset --mixed sets up work tree' '
test_cmp expect actual
'
+test_expect_success 'reset - with no @{-1} branch should fail' '
+ test_when_finished rm -rf new &&
+ git init new &&
+ (
+ cd new &&
+ test_must_fail git reset - 2>../actual
+ ) &&
+ test_i18ngrep "bad flag" actual
+'
+
+test_expect_success 'reset - with no @{-1} branch and file named - should
succeed' '
+ test_when_finished rm -rf new &&
+ >expected &&
+ git init new &&
+ (
+ cd new &&
+ echo "Hello" >- &&
+ git add - &&
+ git reset - >../actual
+ ) &&
+ test_cmp expected actual
+'
+
+test_expect_success 'reset - with @{-1} branch and file named - should fail' '
+ test_when_finished rm -rf new &&
+ git init new &&
+ (
+ cd new &&
+ echo "Hello" >- &&
+ git add - &&
+ git commit -m "first_commit" &&
+ git checkout -b new_branch &&
+ >- &&
+ git add - &&
+ test_must_fail git reset - 2>../actual
+ ) &&
+ test_i18ngrep "ambiguous argument" actual
+'
+
+test_expect_success 'reset - with @{-1} branch and file named @{-1} should
succeed' '
+ test_when_finished rm -rf new &&
+ git init new &&
+ (
+ cd new &&
+ echo "Hello" >@{-1} &&
+ git add @{-1} &&
+ git commit -m "first_commit" &&
+ git checkout -b new_branch &&
+ >@{-1} &&
+ git add @{-1} &&
+ git reset - >../actual
+ ) &&
+ test_i18ngrep "Unstaged" actual
+'
+
+test_expect_success 'reset - with @{-1} branch and no file named - should
succeed' '
+ test_when_finished rm -rf new &&
+ git init new &&
+ (
+ cd new &&
+ echo "Hey" >new_file &&
+ git add new_file &&
+ git commit -m "first_commit" &&
+ git checkout -b new_branch &&
+ >new_file &&
+ git add new_file &&
+ git reset - &&
+ git status -uno >actual &&
+ git add new_file &&
+ git reset @{-1} &&
+ git status -uno >expected &&
+ test_cmp actual expected
+ )
+'
+
test_done
--
2.1.0
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[v3 PATCH 1/2] reset: add '-' shorthand for '@{-1}'
Teaching reset the - shorthand involves checking if any file named '-' exists.
check_filename() is used to perform this check.
When the @{-1} branch does not exist then it can be safely assumed that the
user is referring to the file '-',if any. If this file exists then it is reset
or else
a bad flag error is shown.
But if the @{-1} branch exists then it becomes ambiguous without the explicit
'--' disambiguation as to whether the user wants to reset the file '-' or if
he wants to reset the working tree to the previous branch. Hence the program
dies
with a message about the ambiguous argument.
Helped-by: Junio C Hamano
Helped-by: Eric Sunshine
Signed-off-by: Sundararajan R
---
Thank you Eric and Junio for your patient feedback.
As verify_filename() and verify_non_filename() die and return,respectively when
passed the argument '-' without actually checking if such a file exists,
check_filename() has been used to perform this check. I hope it is okay.
builtin/reset.c | 21 +++--
1 file changed, 19 insertions(+), 2 deletions(-)
diff --git a/builtin/reset.c b/builtin/reset.c
index 4c08ddc..a126b38 100644
--- a/builtin/reset.c
+++ b/builtin/reset.c
@@ -192,6 +192,8 @@ static void parse_args(struct pathspec *pathspec,
{
const char *rev = "HEAD";
unsigned char unused[20];
+ int file_named_minus = 0;
+ int shorthand = 0;
/*
* Possible arguments are:
*
@@ -205,6 +207,12 @@ static void parse_args(struct pathspec *pathspec,
*/
if (argv[0]) {
+ if (!strcmp(argv[0], "-") && !argv[1]) {
+ argv[0] = "@{-1}";
+ shorthand = 1;
+ if(check_filename(prefix, "-"))
+ file_named_minus = 1;
+ }
if (!strcmp(argv[0], "--")) {
argv++; /* reset to HEAD, possibly with paths */
} else if (argv[1] && !strcmp(argv[1], "--")) {
@@ -222,11 +230,20 @@ static void parse_args(struct pathspec *pathspec,
* Ok, argv[0] looks like a commit/tree; it should not
* be a filename.
*/
- verify_non_filename(prefix, argv[0]);
+ if (file_named_minus) {
+ die(_("ambiguous argument '-': both revision
and filename\n"
+ "Use '--' to separate paths from revisions,
like this:\n"
+ "'git [...] --
[...]'"));
+ }
+ else if (!shorthand)
+ verify_non_filename(prefix, argv[0]);
rev = *argv++;
} else {
/* Otherwise we treat this as a filename */
- verify_filename(prefix, argv[0], 1);
+ if (shorthand)
+ argv[0] = "-";
+ if (!file_named_minus)
+ verify_filename(prefix, argv[0], 1);
}
}
*rev_ret = rev;
--
2.1.0
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/3] nd/multiple-work-trees updates
On Wed, Mar 18, 2015 at 3:04 PM, Ephrim Khong wrote: > Without having looked into this and nd/multiple-work-trees, but with "make > multiple checkouts aware of each other" in mind: Could this mechanism be > re-used to make alternates aware of each other, to mitigate the dangers of > having git gc on an alternate remove objects that are used by a > referencing repository? If we can turn on ref namespace and make $GIT_DIR/config and hooks per worktree, I think it may have a chance of replacing alternate object mechanism entirely: one object database, one ref database (but refs of each worktree is namespaced so no conflicts), multiple worktrees, multiple config files, multiple hooks. Because some config keys affect object database, having multiple/conflicting config keys imply that this worktree may change object database in a way trhat impacts performance (not correctness) of another worktree. Later on when we have multiple ref backends, if config keys can change ref backend behavior (or even choose the backend), we may run into other problems. This problem might go away if we define that those "global" config keys can't be per-worktree.. In short, I am good at confusing people. -- Duy -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Need help deciding between subtree and submodule
My $0.02 based on $dayjob (disclaimer I've never used subtree) On Wed, Mar 18, 2015 at 11:14 AM, Robert Dailey wrote: > At my workplace, the team is using Atlassian Stash + git > > We have a "Core" library that is our common code between various > projects. To avoid a single monolithic repository and to allow our > apps and tools to be modularized into their own repos, I have > considered moving Core to a subtree or submodule. Our environment is slightly different. Our projects are made up entirely of submodules, we don't embed submodules within a repo with actual code (side note: I know syslog-ng does so it might be worth having a look around there). Day to day development is done at the submodule level. A developer working on a particular feature is generally only touching one repo notwithstanding a little bit of to-and-fro as they work on the UI aspects. When changes do touch multiple submodules the pushes can generally be ordered in a sane manner. Things get a little complicated when there are interdependent changes, then those pushes require co-operation between submodule owners. The key to making this work is our build system. It is the thing that updates the project repo. After a successful build for all targets (we hope to add unit/regression tests one day) the submodules sha1s are updated and a new baseline (to borrow a clearcase term) is published. Developers can do "git pull && git submodule update" to get the latest stable baseline, but they can also run git pull in a submodule if they want to be on the bleeding edge. > I tried subtree and this is definitely far more transparent and simple > to the team (simplicity is very important), however I notice it has > problems with unnecessary conflicts when you do not do `git subtree > push` for each `git subtree pull`. This is unnecessary overhead and > complicates the log graph which I don't like. > > Submodule functionally works but it is complicated. We make heavy use > of pull requests for code reviews (they are required due to company > policy). Instead of a pull request being atomic and containing any app > changes + accompanying Core changes, we now need to create two pull > requests and manage them in proper order. Things also become more > difficult when branching. All around it just feels like submodule > would interfere and add more administration overhead on a day to day > basis, affecting productivity. We do have policies around review etc. With submodules it does sometimes require engaging owners/reviewers from multiple repositories. Tools like Gerrit can help, particularly where multiple changes and reviewers are involved. > Is there a third option here I'm missing? If only that issue with > subtree could be addressed (the conflicts), it would be perfect enough > for us I think. I have done all the stackoverflow reading and research > I can manage at this point. I would really love some feedback from the > actual git community on what would be a practical solution and > structure here from a company perspective. There's the thing google use for android, I think it's called "repo". There's a few googlers around here so mybe one of them will chime in. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
