Re: Corner case bug caused by shell dependent behavior
Jeff King p...@peff.net writes: Hmph. We ran into this before and fixed all of the sites (e.g., d1c3b10 and 938791c). This one appears to have been added a few months later (by 68d5d03). Maybe there are more places where it would be more robust to use printf instead of echo. FWIW, I just looked through the other uses of echo in git-rebase*.sh, and I think this is the only problematic case. -echo $sha1 $action $prefix $rest +printf %s %s %s %s\n $sha1 $action $prefix $rest Looks obviously correct. The echo just below here does not need the same treatment, as $rest is the problematic bit ($prefix is always fixup or squash). I'd not rationalize this away by deep analysis. Copypaste is a thing, so to just use printf whenever _any_ seriously variable strings (source not immediately the shell script itself, perhaps even _any_ nonconstant strings) are involved keeps people from introducing bugs by following apparent practice. -- David Kastrup -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH] diff: simplify cpp funcname regex
Am 3/14/2014 4:54, schrieb Jeff King:
On Fri, Mar 07, 2014 at 08:23:05AM +0100, Johannes Sixt wrote:
No, I meant lines like
static double var;
-static int old;
+static int new;
The motivation is to show hints where in a file the change is located:
Anything that could be of significance for the author should be picked up.
I see. Coupled with what you said below:
As I said, my motivation is not so much to find a container, but rather
a clue to help locate a change while reading the patch text. I can speak
for myself, but I have no idea what is more important for the majority.
your proposal makes a lot more sense to me, and I think that is really
at the center of our discussion. I do not have a gut feeling for which
is more right (i.e., container versus context).
But given that most of the cases we are discussing are ones where the
diff -p default regex gets it right (or at least better than) compared
to the C regex, I am tempted to say that we should be erring in the
direction of simplicity, and just finding interesting lines without
worrying about containers (i.e., it argues for your patch).
We are in agreement here. So, let's base a decision on the implications on
git grep.
... but I'm not sure if I understand all
of the implications for git grep. Can you give some concrete examples?
Consider this code:
void above()
{}
static int Y;
static int A;
int bar()
{
return X;
}
void below()
{}
When you 'git grep --function-context X', then you get this output with
the current pattern, you proposal, and my proposal (file name etc omitted
for brevity):
int bar()
{
return X;
}
because we start the context at the last hunk header pattern above *or
including the matching line* (and write it in the output), and we stop at
the next hunk header pattern below the match (and do not write it in the
output).
When you 'git grep --function-context Y', what do you want to see? With
the current pattern, and with your pattern that forbids semicolon we get:
void above()
{}
static int Y;
static int A;
and with my simple pattern, which allows semicolon, we get merely
static int Y;
because the line itself is a hunk header (and we do not look back any
further) and the next line is as well. That is not exactly function
context, and that is what I'm a bit worried about.
-- 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: Committing a change from one branch another branch
Hi Brandon McCaig, On Thu, Mar 13, 2014 at 9:06 PM, Brandon McCaig bamcc...@gmail.com wrote: Jagan: On Thu, Mar 13, 2014 at 4:56 AM, Jagan Teki jagannadh.t...@gmail.com wrote: Hi, Hello, I have two branches. - master-b1 - master-b2 Suppose I'm in master-b1 then did a change on master-b1 $ git add test/init.c $ git commit -s -m init.c Changed! $ git log Author: Jagan Teki jagannadh.t...@gmail.com Date: Thu Mar 13 00:48:44 2014 -0700 init.c Changed! $ git checkout master-b2 $ git log Author: Jagan Teki jagannadh.t...@gmail.com Date: Thu Mar 13 00:48:44 2014 -0700 init.c Changed! How can we do this, any idea? What you're asking is ambiguous and vague. The example output that you give doesn't even really make sense. You need to give more details about what you have and what you want to do to get proper help. Or join #git on irc.freenode.net for real-time help if you aren't sure how to explain it. I tried #git irc channel, looks like I'm facing some firewall issue on my end.! OK - Let me explain my query again. I've a git repository with 2 branches. - master-b1 - master-b2 I have to work both these branches since I need two different deliveries. master-b1 have a contents of joo_v1/test.txt joo_v2/test.txt master-b2 joo/test.txt Here, joo_v2 on master-b1 is same as joo on master-b2 means the latest updates on master-b1 with DIR_NAME_VERSION becomes DIR on master-b2 Suppose, if user is change the contents on joo_v2 on master-b1 will change the contents on joo on master-b2 (files contains particular directories are same name with same contents - joo_v2/test.txt and joo/text.txt) The moment user commit the change on master-b1 will create a commit on master-b2 Please let me know if you still need any more information. I will come to IRC soon, for more discussion. thanks! -- Jagan. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: No progress from push when using bitmaps
On 03/13/2014 11:07 PM, Jeff King wrote: On Thu, Mar 13, 2014 at 03:01:09PM -0700, Shawn Pearce wrote: It would definitely be good to have throughput measurements while writing out the pack. However, I'm not sure we have anything useful to count. We know the total number of objects we're reusing, but we're not actually parsing the data; we're just blitting it out as a stream. I think the progress code will need some refactoring to handle a throughput-only case. Yes. I think JGit suffers from this same bug, and again we never noticed it because usually only the servers are bitmapped, not the clients. pack-objects writes a throughput meter when its writing objects. Really just the bytes out/second would be enough to let the user know the client is working. Unfortunately I think that is still tied to the overall progress system having some other counter? Yes, I'm looking at it right now. The throughput meter is actually connected to the sha1fd output. So really we just need to call display_progress periodically as we loop through the data. It's a one-liner fix. _But_ it still looks ugly, because, as you mention, it's tied to the progress meter, which is counting up to N objects. So we basically sit there at 0, pumping data, and then after the pack is done, we can say we sent N. :) There are a few ways around this: 1. Add a new phase Writing packs which counts from 0 to 1. Even though it's more accurate, moving from 0 to 1 really isn't that useful (the throughput is, but the 0/1 just looks like noise). 2. Add a new phase Writing reused objects that counts from 0 bytes up to N bytes. This looks stupid, though, because we are repeating the current byte count both here and in the throughput. 3. Use the regular Writing objects progress, but fake the object count. We know we are writing M bytes with N objects. Bump the counter by 1 for every M/N bytes we write. Would it be practical to change it to a percentage of bytes written? Then we'd have progress info that is both convenient *and* truthful. Michael -- Michael Haggerty mhag...@alum.mit.edu http://softwareswirl.blogspot.com/ -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: No progress from push when using bitmaps
On Fri, Mar 14, 2014 at 4:43 PM, Michael Haggerty mhag...@alum.mit.edu wrote: Would it be practical to change it to a percentage of bytes written? Then we'd have progress info that is both convenient *and* truthful. I agreed for a second, then remembered that we don't know the final pack size until we finish writing it.. Not sure if we could estimate (cheaply) with a good accuracy though. If an object is reused, we already know its compressed size. If it's not reused and is a loose object, we could use on-disk size. It's a lot harder to estimate an not-reused, deltified object. All we have is the uncompressed size, and the size of each delta in the delta chain.. Neither gives a good hint of what the compressed size would be. -- 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 v3 0/8] Hiding refs
On Wed, Mar 12, 2014 at 3:36 AM, Jeff King p...@peff.net wrote: If the client is limited to setting a few flags, then something like http can get away with: GET foo.git/info/refs?service=git-upload-packadvertise-symrefsrefspec=refs/heads/* And it does not need to worry about upload-pack2 at all. Either the server recognizes and acts on them, or it ignores them. But given that we do not have such a magic out-of-band method for passing values over ssh and git, maybe it is not worth worrying about. git could go the same if we lift the restriction in 73bb33a (daemon: Strictly parse the extra arg part of the command - 2009-06-04). It's been five years. Old daemons hopefully have all died out by now. For ssh, I suppose upload-pack and receive-pack can take an extra argument like advertise-symrefsrefspec=refs/heads/* (daemon would use it too to pass the advertiment to upload-pack and receive-pack). That would make all three not need to change the underlying protocol for capability advertisement. Old git-daemon, upload-pack and receive-pack will fail hard on the new advertisement though, unlike http. But that's no worse than upload-pack2. Http can move to upload-pack2 along with the rest. Or maybe http may lead the rest to another way. -- 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: question about: Facebook makes Mercurial faster than Git
On Mon, Mar 10, 2014 at 6:28 PM, demerphq demer...@gmail.com wrote: I had the impression, and I would not be surprised if they had the impression that the git development community is relatively unconcerned about performance issues on larger repositories. There have been other reports, which are difficult to keep track of without a bug tracking system, but the ones I know of are: Poor performance of git status with large number of excluded files and large repositories. I thought this has been improved lately.. I think we could do better still, but my wip is nowhere ready for anybody's eyes. Poor performance, and breakage, on repositories with very large numbers of files in them. index v5 and sparse checkout should help a bit. The ultimate solution, though, is narrow clone that's nowhere near finishing. Well, if you need all files present in worktree, then narrow clone does not help either.. On the same line, poor performance on repos with a lot of very large files also. Junio's split-blob series was a start, but no one picked it up, so I guess your impression was right. (Rebase for instance will break if you rebase a commit that contains a *lot* of files.) Interesting. I guess it hits shell's limitations? Roughly how many files to break it? Poor performance in protocol layer (and other places) with repos with large numbers of refs. (Maybe this is fixed, not sure.) Ah.. no it's not. It's being stirred up again though, in both protocol and ref backend. -- 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 3/3] reset: Print a warning when user uses git reset during a merge
On 14-03-14 12:37 AM, Andrew Wong wrote: During a merge, --mixed is most likely not what the user wants. Using --mixed during a merge would leave the merged changes and new files mixed in with the local changes. The user would have to manually clean up the work tree, which is non-trivial. In future releases, we want to make git reset error out when used in the middle of a merge. For now, we simply print out a warning to the user. I know this approach was suggested earlier, but given these dangers it seems silly to give this big warning on a plain git reset but still go ahead and do the things the warning talks about. Is there any issue with changing git reset to error-out now but letting git reset --mixed proceed? Something like (note the reworded warning message): $ git reset Cowardly refusing to implicitly run 'git reset --mixed' during a merge. This would not clean up any merged changes and would not remove any new files that were created in the work tree. It would also make it impossible for git to automatically clean up the work tree later, so you would have to clean up the work tree manually. You probably meant to run 'git merge --abort' instead. $ git reset --mixed # Stoopid git! I know what I'm doing! $ This would mean that the 10% of git users who like to do git reset in the middle of a conflicted merge will have to teach their fingers some extra motions. But these users are all veterans, and they can more easily type in 8 extra characters (6 with completion) than new users can recover from accidentally misusing git-reset's power. M. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] general style: replaces memcmp() with proper starts_with()
2014-03-14 0:57 GMT-04:00 Jeff King p...@peff.net: This discussion ended up encompassing a lot of other related cleanups. I hope we didn't scare you away. :) I don't think you could; this community is much more accepting than other software communities around the web. The fact that I received constructive feedback rather than a lecture when formatting issues slipped my mind (i.e. forgetting [PATCH v2]) is reason enough to stick around! My understanding is that you were approaching this as a micro-project for GSoC. I'd love it if you want to pick up and run with some of the ideas discussed here. But as far as a microproject goes, I think it would make sense to identify one or two no-brainer improvement spots by hand, and submit a patch with just those (and I think Junio gave some good guidelines in his reply). I agree with trying to push a few uncontroversial changes through. I'd love to take a deeper look at these helper functions and related cleanups…perhaps it would be worth it to identify a few key areas to work on in addition to a main GSoC project? In fact, the project I'm looking to take on (rebase --interactive) also involves code cleanup and might not take all summer, so I could see how those could work well together in a proposal. I'll be re-reading this thread and working on this patch over the weekend to try to identify the more straightforward hunks I could submit in a patch. Thanks Peff and everyone else for your help. Quint -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: No progress from push when using bitmaps
On Fri, Mar 14, 2014 at 05:21:59PM +0700, Duy Nguyen wrote: On Fri, Mar 14, 2014 at 4:43 PM, Michael Haggerty mhag...@alum.mit.edu wrote: Would it be practical to change it to a percentage of bytes written? Then we'd have progress info that is both convenient *and* truthful. I agreed for a second, then remembered that we don't know the final pack size until we finish writing it.. Not sure if we could estimate (cheaply) with a good accuracy though. Right. I'm not sure what Michael meant by it. We can send a percentage of bytes written for the reused pack (my option 2), but we do not know the total bytes for the rest of the objects. So we'd end up with two progress meters (one for the reused pack, and one for everything else), both counting up to different endpoints. And it would require quite a few changes to the progress code. If an object is reused, we already know its compressed size. If it's not reused and is a loose object, we could use on-disk size. It's a lot harder to estimate an not-reused, deltified object. All we have is the uncompressed size, and the size of each delta in the delta chain.. Neither gives a good hint of what the compressed size would be. Hmm. I think we do have the compressed delta size after having run the compression phase (because that is ultimately what we compare to find the best delta). Loose objects are probably the hardest here, as we actually recompress them (IIRC, because packfiles encode the type/size info outside of the compressed bit, whereas it is inside for loose objects; the experimental loose format harmonized this, but it never caught on). Without doing that recompression, any value you came up with would be an estimate, though it would be pretty close (not off by more than a few bytes per object). However, you can't just run through the packing list and add up the object sizes; you'd need to do a real dry-run through the writing phase. There are probably more I'm missing, but you need at least to figure out: 1. The actual compressed size of a full loose object, as described above. 2. The variable-length headers for each object based on its type and size. 3. The final form that the object will take based on what has come before. For example, if there is a max pack size, we may split an object from its delta base, in which case we have to throw away the delta. We don't know where those breaks will be until we walk through the whole list. 4. If an object we attempt to reuse turns out to be corrupted, we fall back to the non-reuse code path, which will have a different size. So you'd need to actually check the reused object CRCs during the dry-run (and for local repacks, not transfers, we actually inflate and check the zlib, too, for safety). So I think it's _possible_. But it's definitely not trivial. For now, I think it makes sense to go with something like the patch I posted earlier (which I'll re-roll in a few minutes). That fixes what is IMHO a regression in the bitmaps case. And it does not make it any harder for somebody to later convert us to a true byte-counter (i.e., it is the easy half already). -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] general style: replaces memcmp() with starts_with()
2014-03-13 12:05 GMT-04:00 Michael Haggerty mhag...@alum.mit.edu: It is very, very unlikely that you inverted the sense of dozens of tests throughout the Git code base and the tests ran correctly. I rather think that you made a mistake when testing. You should double- and triple-check that you really ran the tests and ran them correctly. It looks like HEAD was in the wrong place when I ran the tests. They do not in fact pass. Apologies, Quint -- To unsubscribe from this list: send the line 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] microproject for GSOC
Apply for GSOC.The microprojects is rewriter diff-index.c
Signed-off-by: ubuntu733 ubuntu2...@126.com
---
diff-no-index.c | 11 ++-
dir.h |3 ++-
2 files changed, 8 insertions(+), 6 deletions(-)
diff --git a/diff-no-index.c b/diff-no-index.c
index 8e10bff..91ece64 100644
--- a/diff-no-index.c
+++ b/diff-no-index.c
@@ -3,7 +3,7 @@
* Copyright (c) 2007 by Johannes Schindelin
* Copyright (c) 2008 by Junio C Hamano
*/
-
+#define REMOVE 1
#include cache.h
#include color.h
#include commit.h
@@ -24,10 +24,11 @@ static int read_directory(const char *path, struct
string_list *list)
if (!(dir = opendir(path)))
return error(Could not open directory %s, path);
- while ((e = readdir(dir)))
- if (strcmp(., e-d_name) strcmp(.., e-d_name))
- string_list_insert(list, e-d_name);
-
+ while ((e = readdir(dir))) {
+ while(is_dot_or_dotdot(e-d_name))
+ break;
+ string_list_insert(list, e-d_name);
+ }
closedir(dir);
return 0;
}
diff --git a/dir.h b/dir.h
index 55e5345..1d68818 100644
--- a/dir.h
+++ b/dir.h
@@ -138,8 +138,9 @@ extern int match_pathspec(const struct pathspec *pathspec,
extern int within_depth(const char *name, int namelen, int depth, int
max_depth);
extern int fill_directory(struct dir_struct *dir, const struct pathspec
*pathspec);
+#ifndef REMOVE
extern int read_directory(struct dir_struct *, const char *path, int len,
const struct pathspec *pathspec);
-
+#endif
extern int is_excluded_from_list(const char *pathname, int pathlen, const char
*basename,
int *dtype, struct exclude_list *el);
struct dir_entry *dir_add_ignored(struct dir_struct *dir, const char
*pathname, int len);
--
1.7.9.5
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 0/8] Hiding refs
On Fri, Mar 14, 2014 at 5:37 AM, Duy Nguyen pclo...@gmail.com wrote: On Wed, Mar 12, 2014 at 3:36 AM, Jeff King p...@peff.net wrote: If the client is limited to setting a few flags, then something like http can get away with: GET foo.git/info/refs?service=git-upload-packadvertise-symrefsrefspec=refs/heads/* And it does not need to worry about upload-pack2 at all. Either the server recognizes and acts on them, or it ignores them. But given that we do not have such a magic out-of-band method for passing values over ssh and git, maybe it is not worth worrying about. git could go the same if we lift the restriction in 73bb33a (daemon: Strictly parse the extra arg part of the command - 2009-06-04). It's been five years. Old daemons hopefully have all died out by now. For ssh, I suppose upload-pack and receive-pack can take an extra argument like advertise-symrefsrefspec=refs/heads/* (daemon would use it too to pass the advertiment to upload-pack and receive-pack). Heh. IIRC you are talking about the DoS attack for git-daemon where you send an extra header and the process infinite loops forever? We really don't want a modern client attempting to upgrade the protocol with an ancient daemon to DoS attack that server. That would make all three not need to change the underlying protocol for capability advertisement. Old git-daemon, upload-pack and receive-pack will fail hard on the new advertisement though, unlike http. But that's no worse than upload-pack2. You missed the SSH case. It doesn't have this slot to hide the data into. Http can move to upload-pack2 along with the rest. Or maybe http may lead the rest to another way. -- 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] GSoC Change multiple if-else statements to be table-driven
Eric Sunshine sunsh...@sunshineco.com writes:
Thanks for the resubmission. Comments below.
Thanks, Eric, for helping so many micro exercises.
On Thu, Mar 13, 2014 at 4:20 PM, Yao Zhao zhaox...@umn.edu wrote:
Subject: [PATCH] GSoC Change multiple if-else statements to be table-driven
It's a good idea to let reviewers know that this is attempt 2. Do so
by saying [PATCH v2]. Your next one will be [PATCH v3]. The -v option
for git format-email can help.
Yao, I think Eric meant git format-patch.
When your patch is applied via git am, text inside [...] gets
stripped automatically. The GSoC tells email readers what this
submission is about, but isn't relevant to the actual commit message.
It should be placed inside [...]. For instance: [PATCH/GSoC v2].
So in short,
Subject: [PATCH/GSoC v2] branch.c: turn nested if-else logic to
table-driven
or something.
+ typedef struct PRINT_LIST {
...
+ int b_origin;
+ } PRINT_LIST;
We do not do ALL_CAPS names and tend not to introduce one-off
typedefs for struct. Instead we would just use struct print_list
throughout (if we were to indeed use such a new struct, that is).
+ PRINT_LIST print_list[] = {
+ {.print_str = _(Branch %s set up to track remote branch %s
from %s by rebasing.),
+ .arg2 = shortname, .arg3 = origin,
+.b_rebasing = 1,
.b_remote_is_branch = 1, .b_origin = 1},
I am confused here: I use struct initializer and I am not sure if it's ok
because it is only supported by ANSI
...
Indeed, you want to avoid named field initializers in this project and
instead use positional initializers.
Correct.
Translatable strings in an initializer should be wrapped with N_()
instead of _(). You will still need to use _() later on when you
reference the string from the table. See section 4.7 [2] of the GNU
gettext manual for details.
Correct.
An alternate approach might be to use a multi-dimensional array,
where the boolean values of rebasing, remote_is_branch, and origin
are keys into the array. This would allow you to pick out the
correct PRINT_LIST entry directly (no looping), thus eliminating
the need for those b_rebasing, b_remote_is_branch, and b_origin
members.
Correct.
After seeing so many table driven submissions, I however tend to
agree with your earlier comment on another thread on this same
micro, where you said an nested if-else cascade that was rewritten
in a clearer way (sorry, I do not remember whose submission it was
offhand) may be the best answer to the Would it make sense to make
the code table-driven? question, even though I tentatively queued
d7ea7894 (install_branch_config(): simplify verbose messages logic,
2014-03-13) from Paweł on 'pu'.
Thanks for a review.
--
To unsubscribe from this list: send the line unsubscribe 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] general style: replaces memcmp() with proper starts_with()
Quint Guvernator quintus.pub...@gmail.com writes: I'll be re-reading this thread and working on this patch over the weekend to try to identify the more straightforward hunks I could submit in a patch. Thanks. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/3] reset: Print a warning when user uses git reset during a merge
On Fri, Mar 14, 2014 at 10:33 AM, Marc Branchaud marcn...@xiplink.com wrote: I know this approach was suggested earlier, but given these dangers it seems silly to give this big warning on a plain git reset but still go ahead and do the things the warning talks about. Is there any issue with changing git reset to error-out now but letting git reset --mixed proceed? Something like (note the reworded warning message): Yeah, I would have preferred to have git reset error out right now, because the messed up work tree can be quite a pain to clean up. The main argument for issuing the warning is about maintaining compatibility. For the users that really did mean --merge, the warning is silly. It's basically saying We know that you're about to mess up your work tree, but we let you mess up anyway. Learn the correct way so that you don't mess up next time. It actually doesn't seem too bad if we did make git reset to error out (during a merge) right away. By erroring out, the command won't cause some irreversible damage, and users don't lose data. Yes, it breaks compatibility, but perhaps not in a bad way? I'm really fine with either. Junio? -- To unsubscribe from this list: send the line unsubscribe 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] microproject for GSOC
Hi,
Welcome to the Git community, and welcome to the GSOC program. Below are
some comments to give you a taste of what a review looks like on this
list. Do take the comments seriously (they should be addressed), but
don't take them badly: critic is meant to be constructive.
ubuntu733 ubuntu2...@126.com writes:
^
Please, use a real name when you contribute to Git.
Apply for GSOC.The microprojects is rewriter diff-index.c
This part of your message will become the commit message (i.e. cast in
stone forever in git.git's history). The point is not that you want to
apply for GSOC, but what the patch does and more importantly why it does
it.
+#define REMOVE 1
If the code is to be removed, then remove it. That's why we use a
version control system ;-).
- while ((e = readdir(dir)))
- if (strcmp(., e-d_name) strcmp(.., e-d_name))
- string_list_insert(list, e-d_name);
-
+ while ((e = readdir(dir))) {
+ while(is_dot_or_dotdot(e-d_name))
Missing space between while and (.
+ break;
Broken indentation (indent with space).
This while (...) break; seems really weird to me: if the condition is
false, then you exit the loop because it's a while loop, and if the
condition is true, you exit the loop because of the break. Isn't that a
no-op?
+ string_list_insert(list, e-d_name);
+ }
Broken indentation (misplaced }).
--
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
Apply commits from one branch to another branch (tree structure is different)
Hi, I have two branch in one repo that I need to maintain for 2 different deliveries. Say branch1 and branch2 in test.git repo. test.git - branch1 foo_v1/text.txt foo_v2/text.txt - branch2 foo/text.txt branch1 is developers branch all source looks version'ed manner and branch2 is superset for branch1, example foo_v1 and foo_v2 are the directories in branch1 where developer will update the latest one here foo_v2 and branch2 foo is same as the latest one of branch1 for an instance. Suppose developer send 10 patches on branch1 where are changes in terms of dir_version/ then I need to apply on my local repo branch1, till now is fine then I need to apply same 10 patches on to my branch2 where source tree dir which is quite question here how can I do. Request for any help! let me know for any questions. thanks! -- Jagan. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Apply commits from one branch to another branch (tree structure is different)
Don't know what happen, I'm unable to join #git channel [23:40] Jagan hi [23:40] == Cannot send to channel: #git Can any one help! On Fri, Mar 14, 2014 at 11:09 PM, Jagan Teki jagannadh.t...@gmail.com wrote: Hi, I have two branch in one repo that I need to maintain for 2 different deliveries. Say branch1 and branch2 in test.git repo. test.git - branch1 foo_v1/text.txt foo_v2/text.txt - branch2 foo/text.txt branch1 is developers branch all source looks version'ed manner and branch2 is superset for branch1, example foo_v1 and foo_v2 are the directories in branch1 where developer will update the latest one here foo_v2 and branch2 foo is same as the latest one of branch1 for an instance. Suppose developer send 10 patches on branch1 where are changes in terms of dir_version/ then I need to apply on my local repo branch1, till now is fine then I need to apply same 10 patches on to my branch2 where source tree dir which is quite question here how can I do. Request for any help! let me know for any questions. thanks! -- Jagan. -- Jagan. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re:Re: [PATCH] microproject for GSOC
Thank you for your comments.I will amend those issues .As a Chinese
student,what name should I use?My Chinese name is ok?
I am very interest in Open source.What can I do to increase my chance?
At 2014-03-15 01:10:57,Matthieu Moy matthieu@grenoble-inp.fr wrote:
Hi,
Welcome to the Git community, and welcome to the GSOC program. Below are
some comments to give you a taste of what a review looks like on this
list. Do take the comments seriously (they should be addressed), but
don't take them badly: critic is meant to be constructive.
ubuntu733 ubuntu2...@126.com writes:
^
Please, use a real name when you contribute to Git.
Apply for GSOC.The microprojects is rewriter diff-index.c
This part of your message will become the commit message (i.e. cast in
stone forever in git.git's history). The point is not that you want to
apply for GSOC, but what the patch does and more importantly why it does
it.
+#define REMOVE 1
If the code is to be removed, then remove it. That's why we use a
version control system ;-).
-while ((e = readdir(dir)))
-if (strcmp(., e-d_name) strcmp(.., e-d_name))
-string_list_insert(list, e-d_name);
-
+while ((e = readdir(dir))) {
+while(is_dot_or_dotdot(e-d_name))
Missing space between while and (.
+ break;
Broken indentation (indent with space).
This while (...) break; seems really weird to me: if the condition is
false, then you exit the loop because it's a while loop, and if the
condition is true, you exit the loop because of the break. Isn't that a
no-op?
+string_list_insert(list, e-d_name);
+ }
Broken indentation (misplaced }).
--
Matthieu Moy
http://www-verimag.imag.fr/~moy/
Re:Re: [PATCH] microproject for GSOC
Thank you for your comments.I will amend those issues .As a Chinese
student,what name should I use?My Chinese name is ok?
At 2014-03-15 01:10:57,Matthieu Moy matthieu@grenoble-inp.fr wrote:
Hi,
Welcome to the Git community, and welcome to the GSOC program. Below are
some comments to give you a taste of what a review looks like on this
list. Do take the comments seriously (they should be addressed), but
don't take them badly: critic is meant to be constructive.
ubuntu733 ubuntu2...@126.com writes:
^
Please, use a real name when you contribute to Git.
Apply for GSOC.The microprojects is rewriter diff-index.c
This part of your message will become the commit message (i.e. cast in
stone forever in git.git's history). The point is not that you want to
apply for GSOC, but what the patch does and more importantly why it does
it.
+#define REMOVE 1
If the code is to be removed, then remove it. That's why we use a
version control system ;-).
-while ((e = readdir(dir)))
-if (strcmp(., e-d_name) strcmp(.., e-d_name))
-string_list_insert(list, e-d_name);
-
+while ((e = readdir(dir))) {
+while(is_dot_or_dotdot(e-d_name))
Missing space between while and (.
+ break;
Broken indentation (indent with space).
This while (...) break; seems really weird to me: if the condition is
false, then you exit the loop because it's a while loop, and if the
condition is true, you exit the loop because of the break. Isn't that a
no-op?
+string_list_insert(list, e-d_name);
+ }
Broken indentation (misplaced }).
--
Matthieu Moy
http://www-verimag.imag.fr/~moy/
Re: Git Config pushInsteadOf is not working
I thought you had the URLs backwards, but that doesn't seem to be the problem, assuming I am reading your transcription correctly. Maybe the 'insteadOf' is being applied in addition to (and cancelling out) the pushInsteadOf. Does it work as expected if you remove one or the other? In any case, it sounds like a Git issue, not a Gerrit one. You should ask on git@vger.kernel.org, which I have cc'ed here. Phil On Tue, Mar 11, 2014 at 4:44 AM, Raf rafeah.rahi...@gmail.com wrote: Hi All, I have been searching high and low for this issue, but somehow I do not see anyone encountering the same issue as me. Here is the scenario: I have created a local mirror for my group of developers to download the AOSP code from an external gerrit server. So the developers will download the code from the mirror but push to the external gerrit server. Hence, I have edited my /home/user/.gitconfig file to add the following: #To download from [url ssh://localMirror] insteadOf=ssh://gerritServer #to push [url ssh://gerritServer] pushInsteadOf = ssh://localMirror Some how, the pushInsteadOf does not work, when i tried to push the changes to the external gerrit server, it still pushes to the local mirror server. Also, when I tried to manually add the remote to the repository: git remote add gerrit_origin ssh://gerritServer I tried to push to the gerrit_origin, it still pushes to the local mirror server. Which is strange.. Please help. I have spent whole day looking for this solution to no avail. 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] microproject for GSOC
沈承恩 ubuntu2...@126.com writes: Thank you for your comments.I will amend those issues .As a Chinese student,what name should I use?My Chinese name is ok? I guess it is. The git.git history already has contribution from 张忠山 for example (I don't know if it is chineese, but it looks so from my French eyes). I am very interest in Open source.What can I do to increase my chance? Interesting blog post to read: http://softwareswirl.blogspot.fr/2014/03/my-secret-tip-for-gsoc-success.html -- 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: Apply commits from one branch to another branch (tree structure is different)
On Fri, Mar 14, 2014 at 1:39 PM, Jagan Teki jagannadh.t...@gmail.com wrote: Suppose developer send 10 patches on branch1 where are changes in terms of dir_version/ then I need to apply on my local repo branch1, till now is fine then I need to apply same 10 patches on to my branch2 where source tree dir which is quite question here how can I do. You might be able to use the subtree option in recursive merge. Try something like: git cherry-pick -X subtree=foo commit This tells git to apply the changes to the foo directory in your current branch (branch2). -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Apply commits from one branch to another branch (tree structure is different)
On Sat, Mar 15, 2014 at 12:48 AM, Andrew Wong andrew.k...@gmail.com wrote: On Fri, Mar 14, 2014 at 1:39 PM, Jagan Teki jagannadh.t...@gmail.com wrote: Suppose developer send 10 patches on branch1 where are changes in terms of dir_version/ then I need to apply on my local repo branch1, till now is fine then I need to apply same 10 patches on to my branch2 where source tree dir which is quite question here how can I do. You might be able to use the subtree option in recursive merge. Try something like: git cherry-pick -X subtree=foo commit This tells git to apply the changes to the foo directory in your current branch (branch2). How do I do this? Suppose I'm in branch1 with two commits on foo_v2 and I need to apply them on branch2 where in foo. thanks! -- Jagan. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Apply commits from one branch to another branch (tree structure is different)
On Fri, Mar 14, 2014 at 4:01 PM, Jagan Teki jagannadh.t...@gmail.com wrote: On Sat, Mar 15, 2014 at 12:48 AM, Andrew Wong andrew.k...@gmail.com wrote: On Fri, Mar 14, 2014 at 1:39 PM, Jagan Teki jagannadh.t...@gmail.com wrote: Suppose developer send 10 patches on branch1 where are changes in terms of dir_version/ then I need to apply on my local repo branch1, till now is fine then I need to apply same 10 patches on to my branch2 where source tree dir which is quite question here how can I do. You might be able to use the subtree option in recursive merge. Try something like: git cherry-pick -X subtree=foo commit This tells git to apply the changes to the foo directory in your current branch (branch2). How do I do this? Suppose I'm in branch1 with two commits on foo_v2 and I need to apply them on branch2 where in foo. Since this uses cherry-pick, the changes that you want to apply have to be on branch1 already. Let's say your branch1 looks like: --A--B--C--D and branch2 looks like: --1--2--3--4 And you want to apply commits B and C on branch2, but they modify foo_v1/ on branch1. You can tell git to apply the commits onto the directory foo/ on branch2: git checkout branch2# make sure you're on branch2 git cherry-pick -X subtree=foo B C# pick the commits If there's no conflict, the commits should apply cleanly, and your branch2 would become like: --1--2--3--4--B'--C' -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/3] reset: Print a warning when user uses git reset during a merge
Andrew Wong andrew.k...@gmail.com writes: On Fri, Mar 14, 2014 at 10:33 AM, Marc Branchaud marcn...@xiplink.com wrote: I know this approach was suggested earlier, but given these dangers it seems silly to give this big warning on a plain git reset but still go ahead and do the things the warning talks about. Is there any issue with changing git reset to error-out now but letting git reset --mixed proceed? Something like (note the reworded warning message): Yeah, I would have preferred to have git reset error out right now, because the messed up work tree can be quite a pain to clean up. The main argument for issuing the warning is about maintaining compatibility. For the users that really did mean --merge, the warning is silly. It's basically saying We know that you're about to mess up your work tree, but we let you mess up anyway. Learn the correct way so that you don't mess up next time. I suspect that you meant --mixed instead of --merge here. I do not agree with the We know that you are about to mess up above. Whoever is issuing that warning message may not know better than the user who is running reset. As you wrote most likely not what the user wants in your proposed log message, the only thing we know is that it _often_ is a newbie mistake. I recently needed to manually cherry-pick only one half of a patch, and the way I did so was git show $that_commit P.diff git apply -3 P.diff ... conflicts are expected; that is why I used -3 in the ... first place git reset git diff HEAD edit ... edit away the other half I did not want to cherry-pick ... while fixing the conflicted parts that happened to be ... in the part I did want to cherry-pick git cherry-pick --no-commit $that_commit could have been used as a replacement for the first two steps but being able to run the reset without erroring out was an essential part to make this workflow usable. So I am OK with eventually error out by default, but not OK with we know better than the user and will not allow it at all. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
bug report: regression in git subtree add
In this commit: https://github.com/git/git/commit/10a49587fabde88c0afbc80a99d97fae91811f5f git subtree add for a remote repository and branch was fundamentally broken. It works by chance if a local branch exists that matches the name of the desired remote branch. Thus, master happens to work and the bug possibly escapes common notice. cmd_add_repository() does a git fetch - the desired remote refspec can never be found in the local repo until that git fetch is run, so the attempt to validate it within cmd_add() is wrong - if desired, that should happen after the fetch in cmd_add_repository(). I think the additions shown in lines 505 and 506 should be reverted, since the rev is also checked in cmd_add_commit() Best, Peter Wolanin -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Apply commits from one branch to another branch (tree structure is different)
On Sat, Mar 15, 2014 at 2:07 AM, Andrew Wong andrew.k...@gmail.com wrote: On Fri, Mar 14, 2014 at 4:01 PM, Jagan Teki jagannadh.t...@gmail.com wrote: On Sat, Mar 15, 2014 at 12:48 AM, Andrew Wong andrew.k...@gmail.com wrote: On Fri, Mar 14, 2014 at 1:39 PM, Jagan Teki jagannadh.t...@gmail.com wrote: Suppose developer send 10 patches on branch1 where are changes in terms of dir_version/ then I need to apply on my local repo branch1, till now is fine then I need to apply same 10 patches on to my branch2 where source tree dir which is quite question here how can I do. You might be able to use the subtree option in recursive merge. Try something like: git cherry-pick -X subtree=foo commit This tells git to apply the changes to the foo directory in your current branch (branch2). How do I do this? Suppose I'm in branch1 with two commits on foo_v2 and I need to apply them on branch2 where in foo. Since this uses cherry-pick, the changes that you want to apply have to be on branch1 already. Let's say your branch1 looks like: --A--B--C--D and branch2 looks like: --1--2--3--4 And you want to apply commits B and C on branch2, but they modify foo_v1/ on branch1. You can tell git to apply the commits onto the directory foo/ on branch2: git checkout branch2# make sure you're on branch2 git cherry-pick -X subtree=foo B C# pick the commits If there's no conflict, the commits should apply cleanly, and your branch2 would become like: --1--2--3--4--B'--C' I created two commits on foo_v2 when I move branch2 and did cherry-pick it shows below: Mr.J git cherry-pick -X subtree=foo cc70089614de16b46c08f32ea61c972fea2132ce 14e9c9b20e3bf914f6a38ec720896b3d67f94c90 error: could not apply cc70089... A hint: after resolving the conflicts, mark the corrected paths hint: with 'git add paths' or 'git rm paths' hint: and commit the result with 'git commit' Mr.J ls foo Mr.J gs # On branch branch2 # Unmerged paths: # (use git add/rm file... as appropriate to mark resolution) # #deleted by us: foo/foo_v2/test.txt # no changes added to commit (use git add and/or git commit -a) thanks! -- Jagan. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: RFC GSoC idea: new git config features
Jeff King p...@peff.net writes: On Sat, Mar 01, 2014 at 12:01:44PM +0100, Matthieu Moy wrote: Jeff King p...@peff.net writes: If we had the keys in-memory, we could reverse this: config code asks for keys it cares about, and we can do an optimized lookup (binary search, hash, etc). I'm actually dreaming of a system where a configuration variable could be declared in Git's source code, with associated type (list/single value, boolean/string/path/...), default value and documentation (and then Documentation/config.txt could become a generated file). One could imagine a lot of possibilities like Yes, I think something like that would be very nice. ... ... Migrating the whole code to such system would take time, but creating the system and applying it to a few examples might be feasible as a GSoC project. Agreed, as long as we have enough examples to feel confident that the infrastructure is sufficient. I agree that it would give us a lot of enhancement opportunities if we had a central catalog of what the supported configuration variables are and what semantics (e.g. type, multi-value-ness, etc.) they have. One thing we need to be careful about is that we still must support random configuration items that git-core does not care about at all but scripts (and future versions of git-core) read off of, though. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] t5541: don't call start_httpd after sourcing lib-terminal.sh
Since 83d842dc8 make test using prove fails for some setups in t5541 with: Parse errors: No plan found in TAP output Running t5541 on its own fails with: error: Can't use skip_all after running some tests This happens because start_httpd (which determines if the test is to be skipped) is called after sourcing lib-terminal.sh (which sets up the terminal using test_expect_success). Fix that by calling start_httpd before sourcing lib-terminal.sh. Signed-off-by: Jens Lehmann jens.lehm...@web.de --- Since recently t5541 fails for me on master and pu. I'm not sure what detail in my setup causes this breakage (I have httpd installed and it is running), but this patch fixes it for me. t/t5541-http-push-smart.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/t/t5541-http-push-smart.sh b/t/t5541-http-push-smart.sh index 73af16f..597fb96 100755 --- a/t/t5541-http-push-smart.sh +++ b/t/t5541-http-push-smart.sh @@ -13,8 +13,8 @@ fi ROOT_PATH=$PWD . $TEST_DIRECTORY/lib-httpd.sh -. $TEST_DIRECTORY/lib-terminal.sh start_httpd +. $TEST_DIRECTORY/lib-terminal.sh test_expect_success 'setup remote repository' ' cd $ROOT_PATH -- 1.9.0.168.g1119394 -- To unsubscribe from this list: send the line 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][GSOC2014] install_branch_config: change logical chain to lookup table
Signed-off-by: TamerTas tamer...@outlook.com
---
Thanks again for the feedback it's been a great learning experience. Comments
Below :)
I have refactored the commit [1] to * suggested changes [2].
format-patch was placing 2 hyphens instead of 3 but it's fixed now.
I've turned the table into a multidimensional one and I didn't put
the inner braces since this was used method in other multidimensional arrays
throughout the project.
I've also changed shortname to short_name since that seems to be how variables
are named
in this project.
It appears that table-driven code might be more readable after all.
[1]http://git.661346.n2.nabble.com/PATCH-GSOC2014-install-branch-config-change-logical-chain-to-lookup-table-tp7605550.html
[2]http://git.661346.n2.nabble.com/PATCH-GSOC2014-install-branch-config-change-logical-chain-to-lookup-table-tp7605550p7605605.html
---
branch.c | 42 +-
1 file changed, 17 insertions(+), 25 deletions(-)
diff --git a/branch.c b/branch.c
index 723a36b..eab6fa4 100644
--- a/branch.c
+++ b/branch.c
@@ -49,13 +49,27 @@ static int should_setup_rebase(const char *origin)
void install_branch_config(int flag, const char *local, const char *origin,
const char *remote)
{
- const char *shortname = remote + 11;
+ const char *short_name = remote + 11;
+ const char *setup_message[][2][2] = {
+ N_(Branch %s set up to track local ref %s.),
+ N_(Branch %s set up to track local branch %s.),
+ N_(Branch %s set up to track remote ref %s.),
+ N_(Branch %s set up to track remote branch %s from %s.),
+ N_(Branch %s set up to track local ref %s by rebasing.),
+ N_(Branch %s set up to track local branch %s by rebasing.),
+ N_(Branch %s set up to track remote ref %s by rebasing.),
+ N_(Branch %s set up to track remote branch %s from %s by
rebasing.)
+ };
+
int remote_is_branch = starts_with(remote, refs/heads/);
struct strbuf key = STRBUF_INIT;
int rebasing = should_setup_rebase(origin);
+ const char *remote_name = remote_is_branch? short_name : remote;
+ const char *message =
setup_message[!!rebasing][!!origin][!!remote_is_branch];
+
if (remote_is_branch
-!strcmp(local, shortname)
+!strcmp(local, short_name)
!origin) {
warning(_(Not setting branch %s as its own upstream.),
local);
@@ -77,29 +91,7 @@ void install_branch_config(int flag, const char *local,
const char *origin, cons
strbuf_release(key);
if (flag BRANCH_CONFIG_VERBOSE) {
- if (remote_is_branch origin)
- printf_ln(rebasing ?
- _(Branch %s set up to track remote branch %s
from %s by rebasing.) :
- _(Branch %s set up to track remote branch %s
from %s.),
- local, shortname, origin);
- else if (remote_is_branch !origin)
- printf_ln(rebasing ?
- _(Branch %s set up to track local branch %s
by rebasing.) :
- _(Branch %s set up to track local branch
%s.),
- local, shortname);
- else if (!remote_is_branch origin)
- printf_ln(rebasing ?
- _(Branch %s set up to track remote ref %s by
rebasing.) :
- _(Branch %s set up to track remote ref %s.),
- local, remote);
- else if (!remote_is_branch !origin)
- printf_ln(rebasing ?
- _(Branch %s set up to track local ref %s by
rebasing.) :
- _(Branch %s set up to track local ref %s.),
- local, remote);
- else
- die(BUG: impossible combination of %d and %p,
- remote_is_branch, origin);
+ printf_ln(_(message), local, remote_name, origin);
}
}
---
1.7.9.5
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/3] reset: Print a warning when user uses git reset during a merge
On Fri, Mar 14, 2014 at 4:55 PM, Junio C Hamano gits...@pobox.com wrote: For the users that really did mean --merge, the warning is silly. It's basically saying We know that you're about to mess up your work tree, but we let you mess up anyway. Learn the correct way so that you don't mess up next time. I suspect that you meant --mixed instead of --merge here. No, I did mean --merge. It's silly for inexperienced users because it's too late to use --merge by the time they realized they should not have used the default. The work tree has already become a mess. So they'd immediately think if git was smart enough to warn me about the mess, why not prevent me from getting into the mess in the first place? For the experienced users, they would understand the warning, because they would be aware of the index, and the effect that --mixed and --merge have on it. So I am OK with eventually error out by default, but not OK with we know better than the user and will not allow it at all. Again, I didn't mean we know better than the user. However, from a new user's perspective, they won't understand why git reset gives the warning, but still knowingly messes up their work tree. And we don't know better than the user is exactly why I think we should eventually error out rather than automatically switching to --merge. As Matthieu was saying, automatically switching to --merge could discard conflict resolutions, which would be undesirable. So it's better for git to error out then having git decides what the user (probably) wants. -- To unsubscribe from this list: send the line unsubscribe 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] t5541: don't call start_httpd after sourcing lib-terminal.sh
On Fri, Mar 14, 2014 at 10:18:32PM +0100, Jens Lehmann wrote:
Since 83d842dc8 make test using prove fails for some setups in t5541
with:
Parse errors: No plan found in TAP output
Running t5541 on its own fails with:
error: Can't use skip_all after running some tests
This happens because start_httpd (which determines if the test is to
be skipped) is called after sourcing lib-terminal.sh (which sets up the
terminal using test_expect_success).
Fix that by calling start_httpd before sourcing lib-terminal.sh.
Thanks, your solution seems reasonable. lib-terminal runs a test behind
our back when we source it, which is a little funny.
Potentially we could turn its test into a lazy prereq, but I think that
does not quite work. In addition to setting the TTY prereq, it defines
the test_terminal function, and lazy prereqs happen in a subshell, IIRC.
One option would be to _always_ define test_terminal. Right now we rely
on it failing to exist to catch tests which should fail to correctly
depend on the TTY prerequisite. But we could just as easily have it
report failure in such a case.
Something like the patch below (looks like we should be using $PERL_PATH
instead of perl, too).
Since recently t5541 fails for me on master and pu. I'm not sure what
detail in my setup causes this breakage (I have httpd installed and it
is running), but this patch fixes it for me.
Yeah, this is because we now try to run the tests by default, and skip
them if webserver setup fails. If you want to know why it's failing on
your machine, try running with -v -i to see output, and/or looking in
httpd/error.log in the trash directory.
---
diff --git a/t/lib-terminal.sh b/t/lib-terminal.sh
index 9a2dca5..55b708f 100644
--- a/t/lib-terminal.sh
+++ b/t/lib-terminal.sh
@@ -1,35 +1,36 @@
# Helpers for terminal output tests.
-test_expect_success PERL 'set up terminal for tests' '
+# Catch tests which should depend on TTY but forgot to. There's no need
+# to check that TTY is set here. If the test declared it and we are running
+# it, then it is set.
+test_terminal() {
+ if ! test_declared_prereq TTY
+ then
+ echo 4 test_terminal: need to declare TTY prerequisite
+ return 127
+ fi
+ perl $TEST_DIRECTORY/test-terminal.perl $@
+}
+
+test_lazy_prereq TTY '
+ test_have_prereq PERL
+
# Reading from the pty master seems to get stuck _sometimes_
# on Mac OS X 10.5.0, using Perl 5.10.0 or 5.8.9.
#
# Reproduction recipe: run
#
# i=0
# while ./test-terminal.perl echo hi $i
# do
# : $((i = $i + 1))
# done
#
# After 2000 iterations or so it hangs.
# https://rt.cpan.org/Ticket/Display.html?id=65692
#
- if test $(uname -s) = Darwin
- then
- :
- elif
- perl $TEST_DIRECTORY/test-terminal.perl \
- sh -c test -t 1 test -t 2
- then
- test_set_prereq TTY
- test_terminal () {
- if ! test_declared_prereq TTY
- then
- echo 4 test_terminal: need to declare TTY
prerequisite
- return 127
- fi
- perl $TEST_DIRECTORY/test-terminal.perl $@
- }
- fi
+ test $(uname -s) != Darwin
+
+ perl $TEST_DIRECTORY/test-terminal.perl \
+ sh -c test -t 1 test -t 2
'
--
To unsubscribe from this list: send the line unsubscribe 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] t5541: don't call start_httpd after sourcing lib-terminal.sh
Jeff King p...@peff.net writes:
One option would be to _always_ define test_terminal
That looks like the right direction to go.
Something like the patch below (looks like we should be using $PERL_PATH
instead of perl, too).
;-) Also a SP between test_terminal and (), perhaps.
diff --git a/t/lib-terminal.sh b/t/lib-terminal.sh
index 9a2dca5..55b708f 100644
--- a/t/lib-terminal.sh
+++ b/t/lib-terminal.sh
@@ -1,35 +1,36 @@
# Helpers for terminal output tests.
-test_expect_success PERL 'set up terminal for tests' '
+# Catch tests which should depend on TTY but forgot to. There's no need
+# to check that TTY is set here. If the test declared it and we are running
+# it, then it is set.
+test_terminal() {
+ if ! test_declared_prereq TTY
+ then
+ echo 4 test_terminal: need to declare TTY prerequisite
+ return 127
+ fi
+ perl $TEST_DIRECTORY/test-terminal.perl $@
+}
+
+test_lazy_prereq TTY '
+ test_have_prereq PERL
+
# Reading from the pty master seems to get stuck _sometimes_
# on Mac OS X 10.5.0, using Perl 5.10.0 or 5.8.9.
#
# Reproduction recipe: run
#
# i=0
# while ./test-terminal.perl echo hi $i
# do
# : $((i = $i + 1))
# done
#
# After 2000 iterations or so it hangs.
# https://rt.cpan.org/Ticket/Display.html?id=65692
#
- if test $(uname -s) = Darwin
- then
- :
- elif
- perl $TEST_DIRECTORY/test-terminal.perl \
- sh -c test -t 1 test -t 2
- then
- test_set_prereq TTY
- test_terminal () {
- if ! test_declared_prereq TTY
- then
- echo 4 test_terminal: need to declare TTY
prerequisite
- return 127
- fi
- perl $TEST_DIRECTORY/test-terminal.perl $@
- }
- fi
+ test $(uname -s) != Darwin
+
+ perl $TEST_DIRECTORY/test-terminal.perl \
+ sh -c test -t 1 test -t 2
'
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Apply commits from one branch to another branch (tree structure is different)
On Fri, Mar 14, 2014 at 4:57 PM, Jagan Teki jagannadh.t...@gmail.com wrote: Mr.J git cherry-pick -X subtree=foo cc70089614de16b46c08f32ea61c972fea2132ce 14e9c9b20e3bf914f6a38ec720896b3d67f94c90 error: could not apply cc70089... A hint: after resolving the conflicts, mark the corrected paths hint: with 'git add paths' or 'git rm paths' hint: and commit the result with 'git commit' Mr.J ls foo Mr.J gs # On branch branch2 # Unmerged paths: # (use git add/rm file... as appropriate to mark resolution) # #deleted by us: foo/foo_v2/test.txt # no changes added to commit (use git add and/or git commit -a) Does the foo_v2/test.txt file already exist in branch2 before you try to apply? i.e. does foo/test.txt exist in branch2? What might be happening is: the commit modifies foo_v2/test.txt on branch1, but foo/test.txt doesn't exist on branch2. So even when you use the subtree option, there's no foo/test.txt on branch2 to receive the changes of foo_v2/test.txt. This is an actual conflict that git doesn't know what to do, so you have resolve it. This probably means one of two things for you: 1. You _want_ foo/test.txt on branch2, then: git add foo/foo_v2/test.txt# get the entire test.txt file from that commit on branch1 git mv foo/foo_v2/test.txt foo/test.txt# move/rename the file to the right location 2. You _don't_ want foo/test.txt on branch2, then: git rm foo/foo_v2/test.txt# just remove it And then run git cherry-pick --continue to continue with the cherry-pick. -- To unsubscribe from this list: send the line 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] t/lib-terminal: make TTY a lazy prerequisite
On Fri, Mar 14, 2014 at 02:47:14PM -0700, Junio C Hamano wrote:
Something like the patch below (looks like we should be using $PERL_PATH
instead of perl, too).
Actually, we don't need to do this, as of 94221d2 (t: use perl instead
of $PERL_PATH where applicable, 2013-10-28). If only the author of
that commit were here to correct me...
;-) Also a SP between test_terminal and (), perhaps.
Fixed below. Here it is with a commit message.
-- 8 --
Subject: t/lib-terminal: make TTY a lazy prerequisite
When lib-terminal.sh is sourced by a test script, we
immediately set up the TTY prerequisite. We do so inside a
test_expect_success, because that nicely isolates any
generated output.
However, this early test can interfere with a script that
later wants to skip all tests (e.g., t5541 then goes on to
set up the httpd server, and wants to skip_all if that
fails). TAP output doesn't let us skip everything after we
have already run at least one test.
We could fix this by reordering the inclusion of
lib-terminal.sh in t5541 to go after the httpd setup. That
solves this case, but we might eventually hit a case with
circular dependencies, where either lib-*.sh include might
want to skip_all after the other has run a test. So
instead, let's just remove the ordering constraint entirely
by doing the setup inside a test_lazy_prereq construct,
rather than in a regular test. We never cared about the
test outcome anyway (it was written to always succeed).
Note that in addition to setting up the prerequisite, the
current test also defines test_terminal. Since we can't
affect the environment from a lazy_prereq, we have to hoist
that out. We previously depended on it _not_ being defined
when the TTY prereq isn't set as a way to ensure that tests
properly declare their dependency on TTY. However, we still
cover the case (see the in-code comment for details).
Reported-by: Jens Lehmann jens.lehm...@web.de
Signed-off-by: Jeff King p...@peff.net
---
t/lib-terminal.sh | 37 +++--
1 file changed, 19 insertions(+), 18 deletions(-)
diff --git a/t/lib-terminal.sh b/t/lib-terminal.sh
index 9a2dca5..5184549 100644
--- a/t/lib-terminal.sh
+++ b/t/lib-terminal.sh
@@ -1,6 +1,20 @@
# Helpers for terminal output tests.
-test_expect_success PERL 'set up terminal for tests' '
+# Catch tests which should depend on TTY but forgot to. There's no need
+# to aditionally check that the TTY prereq is set here. If the test declared
+# it and we are running the test, then it must have been set.
+test_terminal () {
+ if ! test_declared_prereq TTY
+ then
+ echo 4 test_terminal: need to declare TTY prerequisite
+ return 127
+ fi
+ perl $TEST_DIRECTORY/test-terminal.perl $@
+}
+
+test_lazy_prereq TTY '
+ test_have_prereq PERL
+
# Reading from the pty master seems to get stuck _sometimes_
# on Mac OS X 10.5.0, using Perl 5.10.0 or 5.8.9.
#
@@ -15,21 +29,8 @@ test_expect_success PERL 'set up terminal for tests' '
# After 2000 iterations or so it hangs.
# https://rt.cpan.org/Ticket/Display.html?id=65692
#
- if test $(uname -s) = Darwin
- then
- :
- elif
- perl $TEST_DIRECTORY/test-terminal.perl \
- sh -c test -t 1 test -t 2
- then
- test_set_prereq TTY
- test_terminal () {
- if ! test_declared_prereq TTY
- then
- echo 4 test_terminal: need to declare TTY
prerequisite
- return 127
- fi
- perl $TEST_DIRECTORY/test-terminal.perl $@
- }
- fi
+ test $(uname -s) != Darwin
+
+ perl $TEST_DIRECTORY/test-terminal.perl \
+ sh -c test -t 1 test -t 2
'
--
1.9.0.417.gc6bea4f
--
To unsubscribe from this list: send the line unsubscribe 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] t/lib-terminal: make TTY a lazy prerequisite
Jeff King p...@peff.net writes:
On Fri, Mar 14, 2014 at 02:47:14PM -0700, Junio C Hamano wrote:
Something like the patch below (looks like we should be using $PERL_PATH
instead of perl, too).
Actually, we don't need to do this, as of 94221d2 (t: use perl instead
of $PERL_PATH where applicable, 2013-10-28). If only the author of
that commit were here to correct me...
Yuck. I forgot all about that, too.
I wonder if that commit (actually the one before it) invites subtle
bugs by tempting us to say
sane_unset VAR
VAR=VAL perl -e 0
test ${VAR+isset} != isset
-- 8 --
Subject: t/lib-terminal: make TTY a lazy prerequisite
When lib-terminal.sh is sourced by a test script, we
immediately set up the TTY prerequisite. We do so inside a
test_expect_success, because that nicely isolates any
generated output.
However, this early test can interfere with a script that
later wants to skip all tests (e.g., t5541 then goes on to
set up the httpd server, and wants to skip_all if that
fails). TAP output doesn't let us skip everything after we
have already run at least one test.
We could fix this by reordering the inclusion of
lib-terminal.sh in t5541 to go after the httpd setup. That
solves this case, but we might eventually hit a case with
circular dependencies, where either lib-*.sh include might
want to skip_all after the other has run a test. So
instead, let's just remove the ordering constraint entirely
by doing the setup inside a test_lazy_prereq construct,
rather than in a regular test. We never cared about the
test outcome anyway (it was written to always succeed).
Note that in addition to setting up the prerequisite, the
current test also defines test_terminal. Since we can't
affect the environment from a lazy_prereq, we have to hoist
that out. We previously depended on it _not_ being defined
when the TTY prereq isn't set as a way to ensure that tests
properly declare their dependency on TTY. However, we still
cover the case (see the in-code comment for details).
Reported-by: Jens Lehmann jens.lehm...@web.de
Signed-off-by: Jeff King p...@peff.net
---
Thanks.
t/lib-terminal.sh | 37 +++--
1 file changed, 19 insertions(+), 18 deletions(-)
diff --git a/t/lib-terminal.sh b/t/lib-terminal.sh
index 9a2dca5..5184549 100644
--- a/t/lib-terminal.sh
+++ b/t/lib-terminal.sh
@@ -1,6 +1,20 @@
# Helpers for terminal output tests.
-test_expect_success PERL 'set up terminal for tests' '
+# Catch tests which should depend on TTY but forgot to. There's no need
+# to aditionally check that the TTY prereq is set here. If the test declared
+# it and we are running the test, then it must have been set.
+test_terminal () {
+ if ! test_declared_prereq TTY
+ then
+ echo 4 test_terminal: need to declare TTY prerequisite
+ return 127
+ fi
+ perl $TEST_DIRECTORY/test-terminal.perl $@
+}
+
+test_lazy_prereq TTY '
+ test_have_prereq PERL
+
# Reading from the pty master seems to get stuck _sometimes_
# on Mac OS X 10.5.0, using Perl 5.10.0 or 5.8.9.
#
@@ -15,21 +29,8 @@ test_expect_success PERL 'set up terminal for tests' '
# After 2000 iterations or so it hangs.
# https://rt.cpan.org/Ticket/Display.html?id=65692
#
- if test $(uname -s) = Darwin
- then
- :
- elif
- perl $TEST_DIRECTORY/test-terminal.perl \
- sh -c test -t 1 test -t 2
- then
- test_set_prereq TTY
- test_terminal () {
- if ! test_declared_prereq TTY
- then
- echo 4 test_terminal: need to declare TTY
prerequisite
- return 127
- fi
- perl $TEST_DIRECTORY/test-terminal.perl $@
- }
- fi
+ test $(uname -s) != Darwin
+
+ perl $TEST_DIRECTORY/test-terminal.perl \
+ sh -c test -t 1 test -t 2
'
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
What's cooking in git.git (Mar 2014, #03; Fri, 14)
Here are the topics that have been cooking. Commits prefixed with '-' are only in 'pu' (proposed updates) while commits prefixed with '+' are in 'next'. More topics merged to 'master', some of which have been cooking before the v1.9.0 final release. You can find the changes described here in the integration branches of the repositories listed at http://git-blame.blogspot.com/p/git-public-repositories.html -- [Graduated to master] * ak/gitweb-fit-image (2014-02-20) 1 commit (merged to 'next' on 2014-03-06 at ba8cb50) + gitweb: Avoid overflowing page body frame with large images Instead of allowing an img to be shown in whatever size, force scaling it to fit on the page with max-height/max-width css style attributes. * da/difftool-git-files (2014-03-05) 2 commits (merged to 'next' on 2014-03-06 at a563ec1) + t7800: add a difftool test for .git-files + difftool: support repositories with .git-files git difftool misbehaved when the repository is bound to the working tree with the .git file mechanism, where a textual file .git tells us where it is. * jc/check-attr-honor-working-tree (2014-02-06) 2 commits (merged to 'next' on 2014-03-06 at 960d679) + check-attr: move to the top of working tree when in non-bare repository + t0003: do not chdir the whole test process git check-attr when (trying to) work on a repository with a working tree did not work well when the working tree was specified via --work-tree (and obviously with --git-dir). The command also works in a bare repository but it reads from the (possibly stale, irrelevant and/or nonexistent) index, which may need to be fixed to read from HEAD, but that is a completely separate issue. As a related tangent to this separate issue, we may want to also fix check-ignore, which refuses to work in a bare repository, to also operate in a bare one. * jh/note-trees-record-blobs (2014-02-20) 1 commit (merged to 'next' on 2014-03-06 at f46852d) + notes: disallow reusing non-blob as a note object git notes -C blob should not take an object that is not a blob. * jk/commit-dates-parsing-fix (2014-03-07) 6 commits (merged to 'next' on 2014-03-07 at 01e9d92) + show_ident_date: fix tz range check (merged to 'next' on 2014-03-06 at dd641e2) + log: do not segfault on gmtime errors + log: handle integer overflow in timestamps + date: check date overflow against time_t + fsck: report integer overflow in author timestamps + t4212: test bogus timestamps with git-log Codepaths that parse timestamps in commit objects have been tightened. * jk/doc-coding-guideline (2014-02-28) 1 commit (merged to 'next' on 2014-03-06 at c33101d) + CodingGuidelines: mention C whitespace rules Elaborate on a style niggle that has been part of mimic existing code. * jk/http-no-curl-easy (2014-02-18) 1 commit (merged to 'next' on 2014-03-06 at 56d3f6f) + http: never use curl_easy_perform Uses of curl's multi interface and easy interface do not mix well when we attempt to reuse outgoing connections. Teach the RPC over http code, used in the smart HTTP transport, not to use the easy interface. * jk/janitorial-fixes (2014-02-18) 5 commits (merged to 'next' on 2014-03-06 at dac2de6) + open_istream(): do not dereference NULL in the error case + builtin/mv: don't use memory after free + utf8: use correct type for values in interval table + utf8: fix iconv error detection + notes-utils: handle boolean notes.rewritemode correctly * jk/remote-pushremote-config-reading (2014-02-24) 1 commit (merged to 'next' on 2014-03-06 at 9e71ecb) + remote: handle pushremote config in any order git push did not pay attention to branch.*.pushremote if it is defined earlier than remote.pushdefault; the order of these two variables in the configuration file should not matter, but it did by mistake. * jl/doc-submodule-update-checkout (2014-02-28) 1 commit (merged to 'next' on 2014-03-06 at 8cdf5cb) + submodule update: consistently document the '--checkout' option Add missing documentation for submodule update --checkout. * jm/stash-doc-k-for-keep (2014-02-24) 1 commit (merged to 'next' on 2014-03-06 at ddd8e48) + stash doc: mention short form -k in save description * jn/am-doc-hooks (2014-02-24) 1 commit (merged to 'next' on 2014-03-06 at 5c1c372) + am doc: add a pointer to relevant hooks * jn/bisect-coding-style (2014-03-03) 1 commit (merged to 'next' on 2014-03-06 at e1de2a5) + git-bisect.sh: fix a few style issues * ks/config-file-stdin (2014-02-18) 4 commits (merged to 'next' on 2014-03-06 at 3e77313) + config: teach git config --file - to read from the standard input + config: change git_config_with_options() interface + builtin/config.c: rename check_blob_write() - check_write() + config: disallow relative include paths from blobs git config learned to read from the standard input when - is given as the value to its --file parameter
Re: [PATCH] t/lib-terminal: make TTY a lazy prerequisite
Am 14.03.2014 22:57, schrieb Jeff King:
On Fri, Mar 14, 2014 at 02:47:14PM -0700, Junio C Hamano wrote:
Something like the patch below (looks like we should be using $PERL_PATH
instead of perl, too).
Actually, we don't need to do this, as of 94221d2 (t: use perl instead
of $PERL_PATH where applicable, 2013-10-28). If only the author of
that commit were here to correct me...
;-) Also a SP between test_terminal and (), perhaps.
Fixed below. Here it is with a commit message.
Thanks, this fixes the problem for me :-)
-- 8 --
Subject: t/lib-terminal: make TTY a lazy prerequisite
When lib-terminal.sh is sourced by a test script, we
immediately set up the TTY prerequisite. We do so inside a
test_expect_success, because that nicely isolates any
generated output.
However, this early test can interfere with a script that
later wants to skip all tests (e.g., t5541 then goes on to
set up the httpd server, and wants to skip_all if that
fails). TAP output doesn't let us skip everything after we
have already run at least one test.
We could fix this by reordering the inclusion of
lib-terminal.sh in t5541 to go after the httpd setup. That
solves this case, but we might eventually hit a case with
circular dependencies, where either lib-*.sh include might
want to skip_all after the other has run a test. So
instead, let's just remove the ordering constraint entirely
by doing the setup inside a test_lazy_prereq construct,
rather than in a regular test. We never cared about the
test outcome anyway (it was written to always succeed).
Note that in addition to setting up the prerequisite, the
current test also defines test_terminal. Since we can't
affect the environment from a lazy_prereq, we have to hoist
that out. We previously depended on it _not_ being defined
when the TTY prereq isn't set as a way to ensure that tests
properly declare their dependency on TTY. However, we still
cover the case (see the in-code comment for details).
Reported-by: Jens Lehmann jens.lehm...@web.de
Signed-off-by: Jeff King p...@peff.net
---
t/lib-terminal.sh | 37 +++--
1 file changed, 19 insertions(+), 18 deletions(-)
diff --git a/t/lib-terminal.sh b/t/lib-terminal.sh
index 9a2dca5..5184549 100644
--- a/t/lib-terminal.sh
+++ b/t/lib-terminal.sh
@@ -1,6 +1,20 @@
# Helpers for terminal output tests.
-test_expect_success PERL 'set up terminal for tests' '
+# Catch tests which should depend on TTY but forgot to. There's no need
+# to aditionally check that the TTY prereq is set here. If the test declared
+# it and we are running the test, then it must have been set.
+test_terminal () {
+ if ! test_declared_prereq TTY
+ then
+ echo 4 test_terminal: need to declare TTY prerequisite
+ return 127
+ fi
+ perl $TEST_DIRECTORY/test-terminal.perl $@
+}
+
+test_lazy_prereq TTY '
+ test_have_prereq PERL
+
# Reading from the pty master seems to get stuck _sometimes_
# on Mac OS X 10.5.0, using Perl 5.10.0 or 5.8.9.
#
@@ -15,21 +29,8 @@ test_expect_success PERL 'set up terminal for tests' '
# After 2000 iterations or so it hangs.
# https://rt.cpan.org/Ticket/Display.html?id=65692
#
- if test $(uname -s) = Darwin
- then
- :
- elif
- perl $TEST_DIRECTORY/test-terminal.perl \
- sh -c test -t 1 test -t 2
- then
- test_set_prereq TTY
- test_terminal () {
- if ! test_declared_prereq TTY
- then
- echo 4 test_terminal: need to declare TTY
prerequisite
- return 127
- fi
- perl $TEST_DIRECTORY/test-terminal.perl $@
- }
- fi
+ test $(uname -s) != Darwin
+
+ perl $TEST_DIRECTORY/test-terminal.perl \
+ sh -c test -t 1 test -t 2
'
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 0/8] Hiding refs
On Fri, Mar 14, 2014 at 11:45 PM, Shawn Pearce spea...@spearce.org wrote: On Fri, Mar 14, 2014 at 5:37 AM, Duy Nguyen pclo...@gmail.com wrote: On Wed, Mar 12, 2014 at 3:36 AM, Jeff King p...@peff.net wrote: If the client is limited to setting a few flags, then something like http can get away with: GET foo.git/info/refs?service=git-upload-packadvertise-symrefsrefspec=refs/heads/* And it does not need to worry about upload-pack2 at all. Either the server recognizes and acts on them, or it ignores them. But given that we do not have such a magic out-of-band method for passing values over ssh and git, maybe it is not worth worrying about. git could go the same if we lift the restriction in 73bb33a (daemon: Strictly parse the extra arg part of the command - 2009-06-04). It's been five years. Old daemons hopefully have all died out by now. For ssh, I suppose upload-pack and receive-pack can take an extra argument like advertise-symrefsrefspec=refs/heads/* (daemon would use it too to pass the advertiment to upload-pack and receive-pack). Heh. IIRC you are talking about the DoS attack for git-daemon where you send an extra header and the process infinite loops forever? We really don't want a modern client attempting to upgrade the protocol with an ancient daemon to DoS attack that server. Shouldn't vulnerable daemons be upgraded anyway? If they keep using the vulnerable version for all these 5 years, I feel no sorry for new clients DoSing them. Jeff's idea about remote.*.useUploadPack2 still applies here so after we attack the server once, it'll be black listed for a while (or forever). That would make all three not need to change the underlying protocol for capability advertisement. Old git-daemon, upload-pack and receive-pack will fail hard on the new advertisement though, unlike http. But that's no worse than upload-pack2. You missed the SSH case. It doesn't have this slot to hide the data into. Right now we run this for ssh case: ssh host git-upload-pack repo-path. New client can do this instead ssh host git-upload-pack repo-path client capability flags -- 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: Corner case bug caused by shell dependent behavior
On Mar 13, Jonathan Nieder wrote: May we have your sign-off? (See Documentation/SubmittingPatches section Sign your work for what this means. I could have found that myself .. thanks! I'll try to follow it now. :) I'll resend the patch. Hopefully I'll do it right. Would it make sense to add this as a test to e.g. t/t3404-rebase-interactive.sh? It's a rather special case, so I'm not sure if it's worth it. I'll send a patch which adds a test for it. The test works for me, but as I don't understand the test mechanisms already good enough a few questions: - Is it correct to call the fake editor with an empty variable FAKE_LINES when you want it to not change the todo list of a rebase -i and use it as it is (the work is already done by the autosquash option)? I can achieve the same with EDITOR=true. What's the preferred way? Is there an advantage to use the fake editor also in this case? - The tests in t3404-rebase-interactive.sh use their variables a bit differently, some just set the variables, some export the variables and some use a subshell to encapsulate them. Also some of the tests reset their rebase state so that subsequent tests, which also use rebase, do not fail when the rebase fails. Other tests don't do that. What's the expected resp. recommended behavior? While trying to understand the test mechanisms I stumbled over two other problematic uses of echo. These although only affect the test output, not git itself. Uwe -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: No progress from push when using bitmaps
On Fri, Mar 14, 2014 at 10:29 PM, Jeff King p...@peff.net wrote: If an object is reused, we already know its compressed size. If it's not reused and is a loose object, we could use on-disk size. It's a lot harder to estimate an not-reused, deltified object. All we have is the uncompressed size, and the size of each delta in the delta chain.. Neither gives a good hint of what the compressed size would be. Hmm. I think we do have the compressed delta size after having run the compression phase (because that is ultimately what we compare to find the best delta). There are cases when we try not to find deltas (large blobs, file too small, or -delta attribute). The large blob case is especially interesting because progress bar crawls slowly when they write these objects. Loose objects are probably the hardest here, as we actually recompress them (IIRC, because packfiles encode the type/size info outside of the compressed bit, whereas it is inside for loose objects; the experimental loose format harmonized this, but it never caught on). Without doing that recompression, any value you came up with would be an estimate, though it would be pretty close (not off by more than a few bytes per object). That's my hope. Although if they tweak compression level then the estimation could be off (gzip -9 and gzip -1 produce big difference in size) However, you can't just run through the packing list and add up the object sizes; you'd need to do a real dry-run through the writing phase. There are probably more I'm missing, but you need at least to figure out: 1. The actual compressed size of a full loose object, as described above. 2. The variable-length headers for each object based on its type and size. We could run through a typical repo, calculate the average header length then use it for all objects? 3. The final form that the object will take based on what has come before. For example, if there is a max pack size, we may split an object from its delta base, in which case we have to throw away the delta. We don't know where those breaks will be until we walk through the whole list. Ah this could probably be avoided. max pack size does not apply to streaming pack-objects, where progress bar is most shown. Falling back to object number in this case does not sound too bad. 4. If an object we attempt to reuse turns out to be corrupted, we fall back to the non-reuse code path, which will have a different size. So you'd need to actually check the reused object CRCs during the dry-run (and for local repacks, not transfers, we actually inflate and check the zlib, too, for safety). Ugh.. So I think it's _possible_. But it's definitely not trivial. For now, I think it makes sense to go with something like the patch I posted earlier (which I'll re-roll in a few minutes). That fixes what is IMHO a regression in the bitmaps case. And it does not make it any harder for somebody to later convert us to a true byte-counter (i.e., it is the easy half already). Agreed. -- 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 v2] rebase -i: replace an echo command by printf
to avoid shell dependent behavior.
When your system shell (/bin/sh) is a dash backslash sequences
in strings are interpreted by the echo command. A commit message
which ends with the string '\n' may result in a garbage line in
the todo list of an interactive rebase which causes the rebase
to fail.
To reproduce the behavior (with dash as /bin/sh):
mkdir test cd test git init
echo 1 foo git add foo
git commit -mthis commit message ends with '\n'
echo 2 foo git commit -a --fixup HEAD
git rebase -i --autosquash --root
Now the editor opens with garbage in line 3 which has to be
removed or the rebase fails.
Signed-off-by: Uwe Storbeck u...@ibr.ch
---
git-rebase--interactive.sh | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index 43c19e0..43631b4 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -739,7 +739,7 @@ rearrange_squash () {
;;
esac
done
- echo $sha1 $action $prefix $rest
+ printf '%s %s %s %s\n' $sha1 $action $prefix
$rest
# if it's a single word, try to resolve to a full sha1
and
# emit a second copy. This allows us to match on both
message
# and on sha1 prefix
--
1.9.0
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] test-lib.sh: use printf instead of echo
when variables may contain backslash sequences.
Backslash sequences are interpreted as control characters
by the echo command of some shells (e.g. dash).
Signed-off-by: Uwe Storbeck u...@ibr.ch
---
t/test-lib.sh | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/t/test-lib.sh b/t/test-lib.sh
index 1531c24..8209204 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -277,7 +277,7 @@ error Test script did not set test_description.
if test $help = t
then
- echo $test_description
+ printf '%s\n' $test_description
exit 0
fi
@@ -328,7 +328,7 @@ test_failure_ () {
test_failure=$(($test_failure + 1))
say_color error not ok $test_count - $1
shift
- echo $@ | sed -e 's/^/# /'
+ printf '%s\n' $@ | sed -e 's/^/# /'
test $immediate = || { GIT_EXIT_OK=t; exit 1; }
}
--
1.9.0
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH/RFC] t3404: test autosquash for fixup! commits with funny messages
This commit adds a test to verify the correct behavior when rebase -i is used to autosquash fixup! commits where the commit message contains a backslash sequence (\n). When echo is used instead of printf to handle such a commit message the test will fail on shells (e.g. dash) where the echo command interprets backslash sequences as control characters. Signed-off-by: Uwe Storbeck u...@ibr.ch --- t/t3404-rebase-interactive.sh | 12 1 file changed, 12 insertions(+) diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh index 50e22b1..6d32661 100755 --- a/t/t3404-rebase-interactive.sh +++ b/t/t3404-rebase-interactive.sh @@ -798,6 +798,18 @@ test_expect_success 'rebase-i history with funny messages' ' test_cmp expect actual ' +test_expect_success 'autosquash fixup! commits with funny messages' ' + test_when_finished git rebase --abort || : + echo file1 + git commit -a -m something that looks like a newline (\n) + echo file1 + git commit -a --fixup HEAD + set_fake_editor + FAKE_LINES= + export FAKE_LINES + git rebase -i --autosquash HEAD~2 +' + test_expect_success 'prepare for rebase -i --exec' ' git checkout master -- 1.9.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 v3 0/8] Hiding refs
On Fri, Mar 14, 2014 at 4:30 PM, Duy Nguyen pclo...@gmail.com wrote: On Fri, Mar 14, 2014 at 11:45 PM, Shawn Pearce spea...@spearce.org wrote: You missed the SSH case. It doesn't have this slot to hide the data into. Right now we run this for ssh case: ssh host git-upload-pack repo-path. New client can do this instead ssh host git-upload-pack repo-path client capability flags Older servers will fail on this command, and the client must reconnect over SSH, which may mean supplying their password/passphrase again. But its remembered that the uploadPack2 didn't work so this can be blacklisted and not retried for a while. -- To unsubscribe from this list: send the line unsubscribe 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] test-lib.sh: use printf instead of echo
Uwe Storbeck wrote:
Backslash sequences are interpreted as control characters
by the echo command of some shells (e.g. dash).
This has bothered me for a while but never enough to do anything about
it. Thanks for fixing it.
Signed-off-by: Uwe Storbeck u...@ibr.ch
Reviewed-by: Jonathan Nieder jrnie...@gmail.com
(patch left unsnipped for reference)
---
t/test-lib.sh | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/t/test-lib.sh b/t/test-lib.sh
index 1531c24..8209204 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -277,7 +277,7 @@ error Test script did not set test_description.
if test $help = t
then
- echo $test_description
+ printf '%s\n' $test_description
exit 0
fi
@@ -328,7 +328,7 @@ test_failure_ () {
test_failure=$(($test_failure + 1))
say_color error not ok $test_count - $1
shift
- echo $@ | sed -e 's/^/# /'
+ printf '%s\n' $@ | sed -e 's/^/# /'
test $immediate = || { GIT_EXIT_OK=t; exit 1; }
}
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] [GSoC] Use strchrnul to save additional scan of string
From c422507408824403ed18e89ec0bbc32b8764e09c Mon Sep 17 00:00:00 2001
From: Shubham Chaudhary shubham.chaudh...@kdemail.net
Date: Sat, 15 Mar 2014 05:56:18 +0530
Subject: [PATCH] [GSoC] Use strchrnul to save additional scan of string
Some strings are scanned twice unnecessarily, once with strchr() and
then with strlen().
I rewrote these sites using strchrnul() when appropriate.
Signed-off-by: Shubham Chaudhary shubham.chaudh...@kdemail.net
---
archive.c | 4 ++--
builtin/blame.c| 7 ++-
builtin/fmt-merge-msg.c| 8
builtin/mailinfo.c | 8 ++--
builtin/reset.c| 4 ++--
builtin/shortlog.c | 10 +++---
cache-tree.c | 9 +++--
contrib/examples/builtin-fetch--tool.c | 3 +--
daemon.c | 4 +---
diff.c | 7 ++-
fast-import.c | 17 -
match-trees.c | 9 +++--
parse-options.c| 5 +
pretty.c | 5 ++---
remote-testsvn.c | 4 ++--
ws.c | 7 ++-
16 files changed, 36 insertions(+), 75 deletions(-)
diff --git a/archive.c b/archive.c
index 346f3b2..d196215 100644
--- a/archive.c
+++ b/archive.c
@@ -259,8 +259,8 @@ static void parse_treeish_arg(const char **argv,
/* Remotes are only allowed to fetch actual refs */
if (remote) {
char *ref = NULL;
- const char *colon = strchr(name, ':');
- int refnamelen = colon ? colon - name : strlen(name);
+ const char *colon = strchrnul(name, ':');
+ int refnamelen = colon - name;
if (!dwim_ref(name, refnamelen, sha1, ref))
die(no such ref: %.*s, refnamelen, name);
diff --git a/builtin/blame.c b/builtin/blame.c
index e5b5d71..24c9676 100644
--- a/builtin/blame.c
+++ b/builtin/blame.c
@@ -1332,11 +1332,8 @@ static void get_ac_line(const char *inbuf,
const char *what,
if (!tmp)
goto error_out;
tmp += strlen(what);
- endp = strchr(tmp, '\n');
- if (!endp)
- len = strlen(tmp);
- else
- len = endp - tmp;
+ endp = strchrnul(tmp, '\n');
+ len = endp - tmp;
if (split_ident_line(ident, tmp, len)) {
error_out:
diff --git a/builtin/fmt-merge-msg.c b/builtin/fmt-merge-msg.c
index 3906eda..9e8da5b 100644
--- a/builtin/fmt-merge-msg.c
+++ b/builtin/fmt-merge-msg.c
@@ -539,12 +539,12 @@ static void find_merge_parents(struct
merge_parents *result,
while (pos in-len) {
int len;
char *p = in-buf + pos;
- char *newline = strchr(p, '\n');
+ char *newline = strchrnul(p, '\n');
unsigned char sha1[20];
struct commit *parent;
struct object *obj;
- len = newline ? newline - p : strlen(p);
+ len = newline - p;
pos += len + !!newline;
if (len 43 ||
@@ -615,8 +615,8 @@ int fmt_merge_msg(struct strbuf *in, struct strbuf *out,
int len;
char *newline, *p = in-buf + pos;
- newline = strchr(p, '\n');
- len = newline ? newline - p : strlen(p);
+ newline = strchrnul(p, '\n');
+ len = newline - p;
pos += len + !!newline;
i++;
p[len] = 0;
diff --git a/builtin/mailinfo.c b/builtin/mailinfo.c
index 2c3cd8e..f901cf3 100644
--- a/builtin/mailinfo.c
+++ b/builtin/mailinfo.c
@@ -902,12 +902,8 @@ static void output_header_lines(FILE *fout, const
char *hdr, const struct strbuf
{
const char *sp = data-buf;
while (1) {
- char *ep = strchr(sp, '\n');
- int len;
- if (!ep)
- len = strlen(sp);
- else
- len = ep - sp;
+ char *ep = strchrnul(sp, '\n');
+ int len = ep - sp;
fprintf(fout, %s: %.*s\n, hdr, len, sp);
if (!ep)
break;
diff --git a/builtin/reset.c b/builtin/reset.c
index 4fd1c6c..fa0ecf5 100644
--- a/builtin/reset.c
+++ b/builtin/reset.c
@@ -103,8 +103,8 @@ static void print_new_head_line(struct commit *commit)
const char *eol;
size_t len;
body += 2;
- eol = strchr(body, '\n');
- len = eol ? eol - body : strlen(body);
+ eol = strchrnul(body, '\n');
+ len = eol - body;
printf( %.*s\n, (int) len, body);
}
else
diff --git a/builtin/shortlog.c b/builtin/shortlog.c
index 4b7e536..f03174f 100644
---
Re: [PATCH v3 0/8] Hiding refs
On Tue, Mar 11, 2014 at 8:49 AM, Jeff King p...@peff.net wrote: Right, I recall the general feeling being that such a system would work, and the transition would be managed by a config variable like remote.*.useUploadPack2. Probably with settings like: true: always try, but allow fall back to upload-pack false: never try, always use upload-pack auto: try, but if we fail, set remote.*.uploadPackTimestamp, and do not try again for N days The default would start at false, and people who know their server is very up-to-date can turn it on. And then when many server implementations support it, flip the default to auto. And either leave it there forever, or eventually just set it to true and drop auto entirely as a code cleanup. I would add that upload-pack also advertises about the availability of upload-pack2 and the client may set the remote.*.useUploadPack2 to either yes or auto so next time upload-pack2 will be used. -- 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] t/lib-terminal: make TTY a lazy prerequisite
On Fri, Mar 14, 2014 at 03:05:45PM -0700, Junio C Hamano wrote:
Actually, we don't need to do this, as of 94221d2 (t: use perl instead
of $PERL_PATH where applicable, 2013-10-28). If only the author of
that commit were here to correct me...
Yuck. I forgot all about that, too.
I wonder if that commit (actually the one before it) invites subtle
bugs by tempting us to say
sane_unset VAR
VAR=VAL perl -e 0
test ${VAR+isset} != isset
I dunno. A more subtle case is:
write_script foo -\EOF
perl ...
EOF
which uses the real perl and not the function. So it's not as airtight
as I would like, but I think it may be a net win, as the common case can
just use perl.
Hmph. It seems like I raised both of those concerns initially:
http://article.gmane.org/gmane.comp.version-control.git/236879
We can revisit it if you want. I think the only options besides leaving
it or reverting it would be to put perl into bin-wrappers as a wrapper
script. That's fine for the tests, but I suspect it might annoy people
who use bin-wrappers to run git straight out of the build directory
without installing.
-- 8 --
Subject: t/lib-terminal: make TTY a lazy prerequisite
[...]
Thanks.
By the way, I checked for other cases that could use the same treatment
by grepping for test_expect_* in t/lib-*.sh. Most of them are inside
functions, so presumably the scripts call them at the appropriate time.
The exceptions are:
1. lib-read-tree-m-3way.sh; this one has a whole battery of tests
that sourced into t1000 and t4002. It could be split into functions
and modernized, but it's probably not worth the effort. It's not
causing ordering problems, and it's not likely to get used
elsewhere.
2. lib-pager.sh; this one is weird, as it is really about setting the
$less variable to git's default pager. And then the prereq is
really just checking that said pager is syntactically simple, I
think, so we can override it by writing to a file with the same
name. At least that's my impression; frankly I found it a bit
confusing to read.
Converting it to a lazy prereq wouldn't work because we care about
its side effect of setting the less variable. There are no
ordering issues with it currently, so I'm inclined to leave it.
-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
Kitchen Designer London
K*i t c h e n Designer London. Thirty Ex D1splay K*i t c h e n s To Clear. W w w . e x d I s p la y k I t ch e n s 1 . c o .u k £ 595 Each with appliances. -- View this message in context: http://git.661346.n2.nabble.com/Kitchen-Designer-London-tp7605683.html Sent from the git mailing list archive at Nabble.com. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 0/2] pack-bitmap progress meters
Here are patches to make the pack-objects progress meters behave the same both with and without pack reuse. The first one is basically the patch I posted earlier. The second one drops the Reusing existing pack, and just rolls those numbers into Counting objects. I have mixed feelings on it. _I_ find it interesting to know whether the pack was reused. But then I am often debugging bitmaps and packfiles. :) From a regular user's perspective, it is an implementation detail that may not be so interesting (git should just magically be faster, and they do not have to care why). So I dunno. Opinions welcome. [1/2]: pack-objects: show progress for reused packfiles [2/2]: pack-objects: show reused packfile objects in Counting 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
[PATCH 1/2] pack-objects: show progress for reused packfiles
When the --all-progress option is in effect, pack-objects
shows a progress report for the writing phase. If the
repository has bitmaps and we are reusing a packfile, the
user sees no progress update until the whole packfile is
sent. Since this is typically the bulk of what is being
written, it can look like git hangs during this phase, even
though the transfer is proceeding.
This generally only happens with git push from a
repository with bitmaps. We do not use --all-progress for
fetch (since the result is going to index-pack on the
client, which takes care of progress reporting). And for
regular repacks to disk, we do not reuse packfiles.
We already have the progress meter setup during
write_reused_pack; we just need to call display_progress
whiel we are writing out the pack. The progress meter is
attached to our output descriptor, so it automatically
handles the throughput measurements.
However, we need to update the object count as we go, since
that is what feeds the percentage we show. We aren't
actually parsing the packfile as we send it, so we have no
idea how many objects we have sent; we only know that at the
end of N bytes, we will have sent M objects. So we cheat a
little and assume each object is M/N bytes (i.e., the mean
of the objects we are sending). While this isn't strictly
true, it actually produces a more pleasing progress meter
for the user, as it moves smoothly and predictably (and
nobody really cares about the object count; they care about
the percentage, and the object count is a proxy for that).
One alternative would be to actually show two progress
meters: one for the reused pack, and one for the rest of the
objects. That would more closely reflect the data we have
(the first would be measured in bytes, and the second
measured in objects). But it would also be more complex and
annoying to the user; rather than seeing one progress meter
counting up to 100%, they would finish one meter, then start
another one at zero.
Signed-off-by: Jeff King p...@peff.net
---
builtin/pack-objects.c | 19 ---
1 file changed, 16 insertions(+), 3 deletions(-)
diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
index f2365a4..12aa94c 100644
--- a/builtin/pack-objects.c
+++ b/builtin/pack-objects.c
@@ -708,7 +708,7 @@ static struct object_entry **compute_write_order(void)
static off_t write_reused_pack(struct sha1file *f)
{
unsigned char buffer[8192];
- off_t to_write;
+ off_t to_write, total;
int fd;
if (!is_pack_valid(reuse_packfile))
@@ -725,7 +725,7 @@ static off_t write_reused_pack(struct sha1file *f)
if (reuse_packfile_offset 0)
reuse_packfile_offset = reuse_packfile-pack_size - 20;
- to_write = reuse_packfile_offset - sizeof(struct pack_header);
+ total = to_write = reuse_packfile_offset - sizeof(struct pack_header);
while (to_write) {
int read_pack = xread(fd, buffer, sizeof(buffer));
@@ -738,10 +738,23 @@ static off_t write_reused_pack(struct sha1file *f)
sha1write(f, buffer, read_pack);
to_write -= read_pack;
+
+ /*
+* We don't know the actual number of objects written,
+* only how many bytes written, how many bytes total, and
+* how many objects total. So we can fake it by pretending all
+* objects we are writing are the same size. This gives us a
+* smooth progress meter, and at the end it matches the true
+* answer.
+*/
+ written = reuse_packfile_objects *
+ (((double)(total - to_write)) / total);
+ display_progress(progress_state, written);
}
close(fd);
- written += reuse_packfile_objects;
+ written = reuse_packfile_objects;
+ display_progress(progress_state, written);
return reuse_packfile_offset - sizeof(struct pack_header);
}
--
1.9.0.417.gc6bea4f
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/2] pack-objects: show reused packfile objects in Counting objects
When we are sending a pack for push or fetch, we may reuse a
chunk of packfile without even parsing it. The progress
meter then looks like this:
Reusing existing pack: 3440489, done.
Counting objects: 3, done.
The first line shows that we are reusing a large chunk of
objects, and then we further count any objects not included
in the reused portion with an actual traversal.
These are all implementation details that the user does not
need to care about. Instead, we can show the reused objects
in the normal counting... progress meter (which will
simply go much faster than normal), and then continue to add
to it as we traverse.
Signed-off-by: Jeff King p...@peff.net
---
builtin/pack-objects.c | 11 +++
1 file changed, 3 insertions(+), 8 deletions(-)
diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
index 12aa94c..4ca3946 100644
--- a/builtin/pack-objects.c
+++ b/builtin/pack-objects.c
@@ -1038,7 +1038,7 @@ static int add_object_entry(const unsigned char *sha1,
enum object_type type,
exclude, name no_try_delta(name),
index_pos, found_pack, found_offset);
- display_progress(progress_state, to_pack.nr_objects);
+ display_progress(progress_state, nr_result);
return 1;
}
@@ -1054,7 +1054,7 @@ static int add_object_entry_from_bitmap(const unsigned
char *sha1,
create_object_entry(sha1, type, name_hash, 0, 0, index_pos, pack,
offset);
- display_progress(progress_state, to_pack.nr_objects);
+ display_progress(progress_state, nr_result);
return 1;
}
@@ -2456,12 +2456,7 @@ static int get_object_list_from_bitmap(struct rev_info
*revs)
reuse_packfile_offset)) {
assert(reuse_packfile_objects);
nr_result += reuse_packfile_objects;
-
- if (progress) {
- fprintf(stderr, Reusing existing pack: %d, done.\n,
- reuse_packfile_objects);
- fflush(stderr);
- }
+ display_progress(progress_state, nr_result);
}
traverse_bitmap_commit_list(add_object_entry_from_bitmap);
--
1.9.0.417.gc6bea4f
--
To unsubscribe from this list: send the line 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] pack-objects: turn off bitmaps when skipping objects
The pack bitmap format requires that we have a single bit
for each object in the pack, and that each object's bitmap
represents its complete set of reachable objects. Therefore
we have no way to represent the bitmap of an object which
references objects outside the pack.
We notice this problem while generating the bitmaps, as we
try to find the offset of a particular object and realize
that we do not have it. In this case we die, and neither the
bitmap nor the pack is generated. This is correct, but
perhaps a little unfriendly. If you have bitmaps turned on
in the config, many repacks will fail which would otherwise
succeed. E.g., incremental repacks, repacks with -l when
you have alternates, .keep files.
Instead, this patch notices early that we are omitting some
objects from the pack and turns off bitmaps (with a
warning). Note that this is not strictly correct, as it's
possible that the object being omitted is not reachable from
any other object in the pack. In practice, this is almost
never the case, and there are two advantages to doing it
this way:
1. The code is much simpler, as we do not have to cleanly
abort the bitmap-generation process midway through.
2. We do not waste time partially generating bitmaps only
to find out that some object deep in the history is not
being packed.
Signed-off-by: Jeff King p...@peff.net
---
I posted this earlier here:
http://article.gmane.org/gmane.comp.version-control.git/240969
The discussion resulted in the jk/repack-pack-keep-objects topic.
However, I think this is still worth applying, as it means git behaves
sensibly when objects are omitted for other reasons (e.g., because you
tried to use -b with an incremental repack, or because you favor
.keep files to bitmaps by explicitly setting repack.packKeptObjects
to false). In our previous discussions, I had assumed this patch had
already been picked up, but I don't see it anywhere. Without it, setting
repack.packKeptObjects to false is largely pointless (instead of
continuing without bitmaps, git will die).
builtin/pack-objects.c | 12 +++-
t/t5310-pack-bitmaps.sh | 5 -
2 files changed, 15 insertions(+), 2 deletions(-)
diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
index 418801f..4ca3946 100644
--- a/builtin/pack-objects.c
+++ b/builtin/pack-objects.c
@@ -1011,6 +1011,10 @@ static void create_object_entry(const unsigned char
*sha1,
entry-no_try_delta = no_try_delta;
}
+static const char no_closure_warning[] = N_(
+disabling bitmap writing, as some objects are not being packed
+);
+
static int add_object_entry(const unsigned char *sha1, enum object_type type,
const char *name, int exclude)
{
@@ -1021,8 +1025,14 @@ static int add_object_entry(const unsigned char *sha1,
enum object_type type,
if (have_duplicate_entry(sha1, exclude, index_pos))
return 0;
- if (!want_object_in_pack(sha1, exclude, found_pack, found_offset))
+ if (!want_object_in_pack(sha1, exclude, found_pack, found_offset)) {
+ /* The pack is missing an object, so it will not have closure */
+ if (write_bitmap_index) {
+ warning(_(no_closure_warning));
+ write_bitmap_index = 0;
+ }
return 0;
+ }
create_object_entry(sha1, type, pack_name_hash(name),
exclude, name no_try_delta(name),
diff --git a/t/t5310-pack-bitmaps.sh b/t/t5310-pack-bitmaps.sh
index d3a3afa..f13525c 100755
--- a/t/t5310-pack-bitmaps.sh
+++ b/t/t5310-pack-bitmaps.sh
@@ -91,7 +91,10 @@ test_expect_success 'fetch (partial bitmap)' '
test_expect_success 'incremental repack cannot create bitmaps' '
test_commit more-1
- test_must_fail git repack -d
+ find .git/objects/pack -name *.bitmap expect
+ git repack -d
+ find .git/objects/pack -name *.bitmap actual
+ test_cmp expect actual
'
test_expect_success 'incremental repack can disable bitmaps' '
--
1.9.0.417.gc6bea4f
--
To unsubscribe from this list: send the line unsubscribe 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] [GSoC] Use strchrnul to save additional scan of string
On Sat, Mar 15, 2014 at 06:19:08AM +0530, Shubham Chaudhary wrote:
From c422507408824403ed18e89ec0bbc32b8764e09c Mon Sep 17 00:00:00 2001
You can drop this line; it's just part of the mbox format.
From: Shubham Chaudhary shubham.chaudh...@kdemail.net
Date: Sat, 15 Mar 2014 05:56:18 +0530
Subject: [PATCH] [GSoC] Use strchrnul to save additional scan of string
And these are redundant with your mail headers. You can drop all of them.
Your patch also appears to be whitespace-damaged (it looks like extra
wrawpping). Consider using git-send-email, which takes care of all of
these issues.
diff --git a/archive.c b/archive.c
index 346f3b2..d196215 100644
--- a/archive.c
+++ b/archive.c
@@ -259,8 +259,8 @@ static void parse_treeish_arg(const char **argv,
/* Remotes are only allowed to fetch actual refs */
if (remote) {
char *ref = NULL;
- const char *colon = strchr(name, ':');
- int refnamelen = colon ? colon - name : strlen(name);
+ const char *colon = strchrnul(name, ':');
+ int refnamelen = colon - name;
This one is pretty straightforward, as we do not need ever look at
colon after this. But this one:
diff --git a/builtin/mailinfo.c b/builtin/mailinfo.c
index 2c3cd8e..f901cf3 100644
--- a/builtin/mailinfo.c
+++ b/builtin/mailinfo.c
@@ -902,12 +902,8 @@ static void output_header_lines(FILE *fout, const
char *hdr, const struct strbuf
{
const char *sp = data-buf;
while (1) {
- char *ep = strchr(sp, '\n');
- int len;
- if (!ep)
- len = strlen(sp);
- else
- len = ep - sp;
+ char *ep = strchrnul(sp, '\n');
+ int len = ep - sp;
fprintf(fout, %s: %.*s\n, hdr, len, sp);
if (!ep)
break;
...does not look right. Before your patch, ep pointed to a newline, or
NULL if we did not find one. After the fprintf, we try to break out of
the loop if we did not find a newline by checking !ep. But that will
never trigger after your patch, and we loop forever reading bogus data
past the end of the string.
I didn't check the other sites; this might be the only problematic one,
but each one needs to be examined by hand. Please double-check the
result by running make test, which does find this bug.
-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] shallow: verify shallow file after taking lock
Before writing the shallow file, we stat() the existing file
to make sure it has not been updated since our operation
began. However, we do not do so under a lock, so there is a
possible race:
1. Process A takes the lock.
2. Process B calls check_shallow_file_for_update and finds
no update.
3. Process A commits the lockfile.
4. Process B takes the lock, then overwrite's process A's
changes.
We can fix this by doing our check while we hold the lock.
Signed-off-by: Jeff King p...@peff.net
---
This is a repost of:
http://article.gmane.org/gmane.comp.version-control.git/242795
You picked up the other two related patches as jk/shallow-update-fix,
but I suspect this one just got lost in the noise because I didn't
format it as a proper series.
shallow.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/shallow.c b/shallow.c
index bbc98b5..75da07a 100644
--- a/shallow.c
+++ b/shallow.c
@@ -246,9 +246,9 @@ void setup_alternate_shallow(struct lock_file *shallow_lock,
struct strbuf sb = STRBUF_INIT;
int fd;
- check_shallow_file_for_update();
fd = hold_lock_file_for_update(shallow_lock, git_path(shallow),
LOCK_DIE_ON_ERROR);
+ check_shallow_file_for_update();
if (write_shallow_commits(sb, 0, extra)) {
if (write_in_full(fd, sb.buf, sb.len) != sb.len)
die_errno(failed to write to %s,
@@ -293,9 +293,9 @@ void prune_shallow(int show_only)
strbuf_release(sb);
return;
}
- check_shallow_file_for_update();
fd = hold_lock_file_for_update(shallow_lock, git_path(shallow),
LOCK_DIE_ON_ERROR);
+ check_shallow_file_for_update();
if (write_shallow_commits_1(sb, 0, NULL, SEEN_ONLY)) {
if (write_in_full(fd, sb.buf, sb.len) != sb.len)
die_errno(failed to write to %s,
--
1.9.0.532.g9ab8f58
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
