Re: Permission denied ONLY after pulling bundles
Dear Philip and Christian, Here my answers: 1) We have a repository that we got from another person in another city.We use the same CENTOS_6 . We put the repository on Windows machine, on which we can access remotely and mounted the directory on CENTOS_6, that we use by the WMVare Player (basically we all have Windows on our machines, through the VMPlayer we access the local files of CENTOS_6) . As i got the bundle file i pulled it on my CENTOS_6 machine on the same branch : git pull NAME_BRANCH.bundle NAME_BRANCH The bundle has been created : git bundle create NAME_BRANCH.bundle NAME_BRANCH The 2 repositories, the one we get from our colleagues and our local one are the same.Now are trying to share changes through bundles and we are using the same branch name to create bundles and pull bundles. 2) I did not check the permission before and after , we have several files and several directories, ,but after having the problem i just gave the chmod -R 777 to all the files, i am sure that all the files involved have those permission. Still i get the problem when trying to push. 3) I tryed to clone the repository using --bare, and i used it as a local repository on linux. Only in this case i can push without problems after pulling the bundle. So if the repository is on the local machine itself (CENTOS_6) i do not have the problem. But we need to have it in a shared folder on windows. When we try to push on the repository on Windows we get permission problems.(Of course we dont get problems before pulling the bundle). So again the 2 possibilities that we tryed are: FIST ONE (PERMISSION PROBLEMS) - Repo is on windows - Repo folder is shared -Repo is a copy of another repository being on a machine in another city on which we cannot access. We got all the files, included the folder .git a put everything in our shared folder - Mounted the Repo folder on Linux -Created the clone - got a bundle from the original repository (bundle created from a branch) -pulled the bundle in the same branch SECOND ONE (NO PROBLEMS BUT WE CANT USE THIS) - Repo is on Linux -Repo is a copy of another repository being on a machine in another city on which we cannot access. - got a bundle from the original repository (bundle created from a branch) -pulled the bundle in the same branch 4) Git version is 1.7.1 5) For Philip: The config file has not changed. Thank you all for your support 2015-06-04 21:25 GMT+02:00 Philip Oakley : > From: "Christian Couder" > >> Hi, >> >> On Thu, Jun 4, 2015 at 3:04 PM, Rossella Barletta >> wrote: >>> >>> Dear git group, >>> >>> >>> I would like to ask your help for a problem that we cannot fix in any >>> way. >>> >>> We have a git repository in folder on Windows. >>> >>> Then we use VMware player on CentOS_6 on which we create a clone of >>> the git repository, after of course having mounted the directory in >>> which the repository is. >>> >>> So the repository is on windows and the clone on Linux. >>> >>> We are able to perfom all the git operations we need, except for the >>> pull .bundle, which is successful in itself but prevent us from >>> pushing after that. >> >> >> It is not very clear how the bundle has been made, and on which >> machine you made it and you pulled from it. >> >>> As we try to push after pulling a .bundle in a branch we get the error >>> message >>> >>> NODE1:fdp> git push >>> Counting objects: 1977, done. >>> Delta compression using up to 2 threads. >>> Compressing objects: 100% (423/423), done. >>> fatal: write error: Permission denied00 KiB | 158 KiB/s >>> error: pack-objects died of signal 13 >>> error: pack-objects died with strange error >> >> >> Can you have a look at the machine you push to and see if some file or >> directory permissions changed between before and after you made the >> bundle or you pulled the bundle? >> >>> We have checked all the permissions, changed the users, recreated the >>> clone but nothing worked. >> >> >> What do you mean by checked all the permissions? >> You mean that permissions haven't changed at all since before you >> pulled the first bundle? >> >>> The push operation works perfectly until we pull a bundle. After >>> pulling a bundle we are not able to push anymore.We tryed to delete >>> the branches, recreate others and all works perfectly, also the >>> push.As we pull the .bundle we cannot get the permission to do the >>> push anymore. >>> >>> What has this to do with the bundle? >> >> >> Did you try to everything (cloning, creating a bundle, pulling it and >> pushing on the same machine to see if it makes a difference? Also did >> you try with another smaller fake repository? >> >> If you can reproduce with a smaller fake repo on just one machine it >> could help us reproduce on one of our machine and have a look. >> >> And could you tell us which version of git (using git --version) you >> are using on both machines? >> > -- > Also, check the config file to see if the push url has somehow changed to > the bundle
[PATCH] utf8: NO_ICONV: silence uninitialized variable warning
The last argument of reencode_string_len() is an 'int *' which is
assigned the length of the converted string. When NO_ICONV is defined,
however, reencode_string_len() is stubbed out by the macro:
#define reencode_string_len(a,b,c,d,e) NULL
which never assigns a value to the final argument. When called like
this:
int n;
char *s = reencode_string_len(..., &n);
if (s)
do_something(s, n);
some compilers complain that 'n' is used uninitialized within the
conditional.
Signed-off-by: Eric Sunshine
---
utf8.h | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/utf8.h b/utf8.h
index e7b2aa4..5a9e94b 100644
--- a/utf8.h
+++ b/utf8.h
@@ -31,7 +31,9 @@ char *reencode_string_len(const char *in, int insz,
const char *in_encoding,
int *outsz);
#else
-#define reencode_string_len(a,b,c,d,e) NULL
+static inline char *reencode_string_len(const char *a, int b,
+ const char *c, const char *d, int *e)
+{ if (e) *e = 0; return NULL; }
#endif
static inline char *reencode_string(const char *in,
--
2.4.2.613.g328fd50
--
To unsubscribe from this list: send the line "unsubscribe 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] git-am: handling unborn branches
On Fri, Jun 5, 2015 at 1:26 AM, Junio C Hamano wrote: > Paul Tan writes: > >> git-am generally supports applying patches to unborn branches. >> However, there are 2 cases where git-am does not handle unborn >> branches which I would like to address before the git-am rewrite to C: > >> 1. am --skip >> >> For git am --skip, git-am.sh does a fast-forward checkout from HEAD to >> HEAD, discarding unmerged entries, and then resets the index to HEAD >> so that the index is not dirty. >> >> git read-tree --reset -u HEAD HEAD >> orig_head=$(cat "$GIT_DIR/ORIG_HEAD") >> git reset HEAD >> git update-ref ORIG_HEAD $orig_head >> >> This requires a valid HEAD. Since git-am requires an empty index for >> unborn branches in the patch application stage anyway, I think we >> should discard all entires in the index if we are on an unborn branch? > > Yes, and it should also remove the new files the failed application > brought in to the working tree, if any, to match the "--skip" done > in the normal case (i.e. when we already have a history to apply > patches to), I would think. Hmm, actually git-am.sh doesn't seem to do that even when we have a history to apply patches to. This is okay in the non-3way case, as git-apply will check to see if the patch applies before it modifies the index, but if we fall back on 3-way merge, any new files the failed merge added will not be deleted in the "git read-tree --reset -u HEAD HEAD". Should we do that? I dunno, but if we want to introduce this feature, I think a "git read-tree --reset HEAD HEAD && git read-tree -m -u $(git write-tree) HEAD" will do the trick? (We discard all unmerged entries, then fast-forward from the tree the failed merged left us with back to HEAD). Clearing the index in the unborn branch case seems to be the most consistent thing to do for now (for the purpose of the git-am rewrite). Thanks, Paul -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
AW: Re*: AW: Getting the full path of a conflicting file within a custom merge driver?
Thanks, thanks, thanks.
One last question. If I don't want to compile Git myself, how long may the pu
branch take approx. to get into a next release?
Mit freundlichen Grüßen
Andreas Gondek
Applications
Deutsche WertpapierService Bank AG
ITTAS
Derendorfer Allee 2
40476 Düsseldorf
Tel.: +49 69 5099 9503
Fax: +49 69 5099 85 9503
E-Mail: andreas.gon...@dwpbank.de
http://www.dwpbank.de
Deutsche WertpapierService Bank AG | Wildunger Straße 14 | 60487 Frankfurt am
Main
Sitz der AG: Frankfurt am Main, HRB 56913 | USt.-ID: DE 813759005
Vorstand: Thomas Klanten, Dr. Christian Tonnesen
Aufsichtsrat: Wilfried Groos (Vors.)
-Ursprüngliche Nachricht-
Von: Junio C Hamano [mailto:jch2...@gmail.com] Im Auftrag von Junio C Hamano
Gesendet: Freitag, 5. Juni 2015 00:11
An: Gondek, Andreas
Cc: git@vger.kernel.org
Betreff: Re*: AW: Getting the full path of a conflicting file within a custom
merge driver?
Junio C Hamano writes:
> "Gondek, Andreas" writes:
>
>> thank you for responding this fast. I would suggest providing this
>> information as an additional parameter (like %A %O %B and %L) maybe
>> %P.
>
> Yes, per-cent plus a letter is more in line with the way information
> is passed to the scripts already. Thanks for making a more sensible
> counter-suggestion. And your %P(ath) sounds like a sensible choice.
>
> It won't be a two-liner, though, as the path is an arbitrary string,
> unlike the names of the three temporary files, and needs to be quoted
> for the shell.
>
> Let me see if I can find time today to cook up something.
I had this in my outbox but then forgot to send it out.
-- >8 --
Subject: ll-merge: pass the original path to external drivers
The interface to custom low-level merge driver was modeled to be capable of
driving programs like "merge" (from the RCS suite) that can produce result
solely by looking at three files that hold contents of common ancestor, ours
and theirs. The information we feed to the external drivers via the command
line placeholders %O, %A, and %B were designed to be purely about contents by
giving names of the temporary files that hold these variants without exposing
the original pathname. No matter where the result goes, merging the same three
variants should produce the same result, contents is the king, that is the Git
way.
The external driver interface, however, is meant to help people to step outside
the Git worldview, and sometimes people want to know the final path that the
resulting merged contents would be stored in. Expose this to the external
drivers via a new placeholder %P.
Requested-by: Andreas Gondek
Signed-off-by: Junio C Hamano
---
* t6026 is ancient and its style may need to be modernised to use
test_cmp intead of cmp, etc.
Documentation/gitattributes.txt | 5 -
ll-merge.c | 10 --
t/t6026-merge-attr.sh | 14 +-
3 files changed, 21 insertions(+), 8 deletions(-)
diff --git a/Documentation/gitattributes.txt b/Documentation/gitattributes.txt
index 70899b3..81fe586 100644
--- a/Documentation/gitattributes.txt
+++ b/Documentation/gitattributes.txt
@@ -774,7 +774,7 @@ To define a custom merge driver `filfre`, add a section to
your
[merge "filfre"]
name = feel-free merge driver
- driver = filfre %O %A %B
+ driver = filfre %O %A %B %L %P
recursive = binary
@@ -800,6 +800,9 @@ merge between common ancestors, when there are more than
one.
When left unspecified, the driver itself is used for both internal merge and
the final merge.
+The merge driver can learn the pathname in which the merged result will
+be stored via placeholder `%P`.
+
`conflict-marker-size`
^^
diff --git a/ll-merge.c b/ll-merge.c
index 8ea03e5..fc3c049 100644
--- a/ll-merge.c
+++ b/ll-merge.c
@@ -9,6 +9,7 @@
#include "xdiff-interface.h"
#include "run-command.h"
#include "ll-merge.h"
+#include "quote.h"
struct ll_merge_driver;
@@ -166,17 +167,20 @@ static int ll_ext_merge(const struct ll_merge_driver *fn,
{
char temp[4][50];
struct strbuf cmd = STRBUF_INIT;
- struct strbuf_expand_dict_entry dict[5];
+ struct strbuf_expand_dict_entry dict[6];
+ struct strbuf path_sq = STRBUF_INIT;
const char *args[] = { NULL, NULL };
int status, fd, i;
struct stat st;
assert(opts);
+ sq_quote_buf(&path_sq, path);
dict[0].placeholder = "O"; dict[0].value = temp[0];
dict[1].placeholder = "A"; dict[1].value = temp[1];
dict[2].placeholder = "B"; dict[2].value = temp[2];
dict[3].placeholder = "L"; dict[3].value = temp[3];
- dict[4].placeholder = NULL; dict[4].value = NULL;
+ dict[4].placeholder = "P"; dict[4].value = path_sq.buf;
+ dict[5].placeholder = NULL; dict[5].
Pack files, standards compliance, and efficiency
There was discussion sometime back about the object_id conversions and
handling direct offsets in pack files. In some places in sha1_file.c,
we return direct pointers to the SHA-1 values in the mmap'd pack file
and use those in other parts of the code.
However, with the object_id conversion, we run into a problem: casting
those unsigned char * values into struct object_id * values is not
allowed by the C standard. There are two possible solutions: copying;
and the just-do-it solution, where we cast and hope for the best.
It looks like we use the latter in nth_packed_object_offset, where we
cast an unsigned char * value to uint32_t *, which is clearly not
allowed. I'm to the point where I need to make a decision on how to
proceed, and I've included a patch with the copying conversion of
nth_packed_object_sha1 below for comparison. The casting solution is
obviously more straightforward. I haven't tested either implementation
for performance.
People interested in the (still-to-change) state of the branch can see
it at https://github.com/bk2204/git object-id-part2.
8<--
From 0f7a958ebbf6c25af526c5c46cc9b5f29f7caa15 Mon Sep 17 00:00:00 2001
From: "brian m. carlson"
Date: Thu, 4 Jun 2015 01:26:10 +
Subject: [PATCH] Convert nth_packed_object_sha1 to object_id.
nth_packed_object_sha1 returned a pointer directly into the mmap'd
memory of the pack in question, but the C standard does not allow
converting a pointer to unsigned char directly into a pointer to struct
object_id, even though the data represented is exactly the same size.
Rename the function nth_packed_object_oid, and make it take an
additional argument of type pointer to struct object_id. Copy the data,
even though this is less efficient, and adjust the callers to account
for the change in calling conventions. Adjust get_delta_base_sha1
similarly.
Signed-off-by: brian m. carlson
---
builtin/pack-objects.c | 22 +++---
cache.h| 8 ++---
pack-bitmap.c | 24 +++
pack-check.c | 15 +-
sha1_file.c| 81 +++---
sha1_name.c| 14 -
6 files changed, 84 insertions(+), 80 deletions(-)
diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
index 870a266..9e12bd8 100644
--- a/builtin/pack-objects.c
+++ b/builtin/pack-objects.c
@@ -1329,6 +1329,7 @@ static void check_object(struct object_entry *entry)
struct packed_git *p = entry->in_pack;
struct pack_window *w_curs = NULL;
const unsigned char *base_ref = NULL;
+ struct object_id base_ref_oid;
struct object_entry *base_entry;
unsigned long used, used_0;
unsigned long avail;
@@ -1394,7 +1395,8 @@ static void check_object(struct object_entry *entry)
revidx = find_pack_revindex(p, ofs);
if (!revidx)
goto give_up;
- base_ref = nth_packed_object_sha1(p,
revidx->nr);
+ nth_packed_object_oid(p, &base_ref_oid,
revidx->nr);
+ base_ref = base_ref_oid.hash;
}
entry->in_pack_header_size = used + used_0;
break;
@@ -2350,7 +2352,7 @@ static void add_objects_in_unpacked_packs(struct rev_info
*revs)
memset(&in_pack, 0, sizeof(in_pack));
for (p = packed_git; p; p = p->next) {
- const unsigned char *sha1;
+ struct object_id oid;
struct object *o;
if (!p->pack_local || p->pack_keep)
@@ -2363,8 +2365,8 @@ static void add_objects_in_unpacked_packs(struct rev_info
*revs)
in_pack.alloc);
for (i = 0; i < p->num_objects; i++) {
- sha1 = nth_packed_object_sha1(p, i);
- o = lookup_unknown_object(sha1);
+ nth_packed_object_oid(p, &oid, i);
+ o = lookup_unknown_object(oid.hash);
if (!(o->flags & OBJECT_ADDED))
mark_in_pack_object(o, p, &in_pack);
o->flags |= OBJECT_ADDED;
@@ -2430,7 +2432,7 @@ static void loosen_unused_packed_objects(struct rev_info
*revs)
{
struct packed_git *p;
uint32_t i;
- const unsigned char *sha1;
+ struct object_id oid;
for (p = packed_git; p; p = p->next) {
if (!p->pack_local || p->pack_keep)
@@ -2440,11 +2442,11 @@ static void loosen_unused_packed_objects(struct
rev_info *revs)
die("cannot open pack index");
for (i = 0; i < p->num_objects; i++) {
- sha1 = nth_packed_object_sha1(p, i);
- if (!packlist_find(&to_pack, sha1, NULL) &&
Re: [RFC] send-email aliases when editing patches or using --xx-cmd
On Thu, Jun 4, 2015 at 8:38 PM, Allen Hubbe wrote: > On Thu, Jun 4, 2015 at 6:53 PM, Remi Lespinet > wrote: >> On Thu, Jun 4, 2015 at 1:17 PM, Remi LESPINET >> wrote: >>> >>> Hi, >>> >>> Working on git-send-email, I've seen that there's no aliases support >>> when manually adding a recipient in a 'To' or 'Cc' field in a patch >>> and for the --to-cmd and --cc-cmd. >>> >>> I would like to add this support, and I wonder if there are reasons >>> not to do it. >>> >>> Thanks. >> >> I just realize that I totally messed up, Of course I don't want to add >> To or Cc fields to patches. >> >> In fact I want to add aliases support for --to-cmd and --cc-cmd options. >> But the modification depends on wheter we can use aliases in fields >> used by send-email (such as author or signoff-by..) when manually >> editing a patch or not. >> >> Sorry for this mistake :( > > You are right that Signed-off-by: myAlias wouldn't make sense, but is > that really what you intended to propose? It's definitely not a good > idea to write patches that way, but on the other hand, if git happens > to accept an alias there, what's the harm? Git will send an email... > the patch will be rejected. > > Also, I believe git-send-email looks for signed-off-by, regardless of > the presence or output of to-cmd (around line 1490 -- "Now parse the > message body"). > I don't see any harm to accepting aliases in the /header/ portion of the patch, either. Those are not part of the commit message, and git will reformat all the headers before actually sending the email, anyway. Again, this is not the same as to-cmd. Note that git-send-email parses the header fields regardless of to-cmd. > Something like to-cmd is much more general than just looking for > Signed-off-by, etc, or scanning a patch for things that look like > email addresses. The to-cmd can also look for keywords in the patch, > like component names, and return an array of maintainer email > addresses for those components, for instance. It might be the case > that none of the emails returned by the to-cmd are actually ever > explicitly mentioned in the patch. In my workspace, I might indeed > write a script, specify it as the to-cmd, and have the script return > an array of email addresses according to whatever criteria I can > imagine. What's the harm if, in my script, in my own workspace, I > return an array of email aliases in the to_cmd, instead? > > Now, as long as the to-cmd is already a script, executed by the > computer, there's really very little to gain in using email aliases > versus whole email addresses. A script can emit a whole properly > formatted email address just as easily as it can emit an email alias. > > Your proposal is just a different use for email aliases, which is > independent from where the aliases come from. There is no conflict > with any of the email alias parsers. -- To unsubscribe from this list: send the line "unsubscribe 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] stash: require a clean index to apply
I'm writing about the patch that Jeff King submitted on April 22, in
<20150422193101.gc27...@peff.net>, in particular,
https://github.com/git/git/commit/ed178ef13a26136d86ff4e33bb7b1afb5033f908
. It appears that this patch was included in git 2.4.2, and it breaks my
workflow.
In particular, I have a pre-commit hook whith does the following:
1. Stash unstaged changes ("git stash -k").
2. Run flake8 over all staged changes.
3. If flake8 complains, then error out of the commit.
4. Otherwise, apply the stash and exit.
This way I am prevented from committing staged changes that don't pass
flake8. I can't imagine that this is a terribly uncommon workflow.
This worked fine until the aforementioned comment, after which my hook
complains, "Cannot apply stash: Your index contains uncommitted changes."
The reason I have to do things this way is as follows. Suppose I did the
following:
1. Stage changes that have a flake8 violation.
2. Fix the flake8 violation in the unstaged version of the staged file.
3. Commit the previously staged changes.
If my commit hook runs over the unstaged version of the file, then it
won't detect the flake8 violation, and as a result the violation will be
committed.
If anyone has a suggestion for how I can achieve the desired goal within
the constraints of the 2.4.2 version of git-stash.sh, I'd love to hear
it. Otherwise, I'd like to ask for this patch to be reconsidered.
Thank you,
Jonathan Kamens
--
To unsubscribe from this list: send the line "unsubscribe 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] send-email aliases when editing patches or using --xx-cmd
On Thu, Jun 4, 2015 at 6:53 PM, Remi Lespinet wrote: > On Thu, Jun 4, 2015 at 1:17 PM, Remi LESPINET > wrote: >> >> Hi, >> >> Working on git-send-email, I've seen that there's no aliases support >> when manually adding a recipient in a 'To' or 'Cc' field in a patch >> and for the --to-cmd and --cc-cmd. >> >> I would like to add this support, and I wonder if there are reasons >> not to do it. >> >> Thanks. > > I just realize that I totally messed up, Of course I don't want to add > To or Cc fields to patches. > > In fact I want to add aliases support for --to-cmd and --cc-cmd options. > But the modification depends on wheter we can use aliases in fields > used by send-email (such as author or signoff-by..) when manually > editing a patch or not. > > Sorry for this mistake :( You are right that Signed-off-by: myAlias wouldn't make sense, but is that really what you intended to propose? It's definitely not a good idea to write patches that way, but on the other hand, if git happens to accept an alias there, what's the harm? Git will send an email... the patch will be rejected. Also, I believe git-send-email looks for signed-off-by, regardless of the presence or output of to-cmd (around line 1490 -- "Now parse the message body"). Something like to-cmd is much more general than just looking for Signed-off-by, etc, or scanning a patch for things that look like email addresses. The to-cmd can also look for keywords in the patch, like component names, and return an array of maintainer email addresses for those components, for instance. It might be the case that none of the emails returned by the to-cmd are actually ever explicitly mentioned in the patch. In my workspace, I might indeed write a script, specify it as the to-cmd, and have the script return an array of email addresses according to whatever criteria I can imagine. What's the harm if, in my script, in my own workspace, I return an array of email aliases in the to_cmd, instead? Now, as long as the to-cmd is already a script, executed by the computer, there's really very little to gain in using email aliases versus whole email addresses. A script can emit a whole properly formatted email address just as easily as it can emit an email alias. Your proposal is just a different use for email aliases, which is independent from where the aliases come from. There is no conflict with any of the email alias parsers. -- To unsubscribe from this list: send the line "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 v5 3/3] git-am: add am.threeWay config variable
Add the am.threeWay configuration variable to use the -3 or --3way option of git am by default. When am.threeway is set and not desired for a specific git am command, the --no-3way option can be used to override it. Signed-off-by: Remi Lespinet --- Only one change compared to previous version: "git-config(1)" replaced by "linkgit:git-config[1]" Documentation/config.txt | 8 Documentation/git-am.txt | 7 +-- git-am.sh| 9 + t/t4150-am.sh| 19 +++ 4 files changed, 41 insertions(+), 2 deletions(-) diff --git a/Documentation/config.txt b/Documentation/config.txt index d44bc85..36b75d9 100644 --- a/Documentation/config.txt +++ b/Documentation/config.txt @@ -769,6 +769,14 @@ am.keepcr:: by giving '--no-keep-cr' from the command line. See linkgit:git-am[1], linkgit:git-mailsplit[1]. +am.threeWay:: + By default, `git am` will fail if the patch does not apply cleanly. When + set to true, this setting tells `git am` to fall back on 3-way merge if + the patch records the identity of blobs it is supposed to apply to and + we have those blobs available locally (equivalent to giving the `--3way` + option from the command line). Defaults to `false`. + See linkgit:git-am[1]. + apply.ignoreWhitespace:: When set to 'change', tells 'git apply' to ignore changes in whitespace, in the same way as the '--ignore-space-change' diff --git a/Documentation/git-am.txt b/Documentation/git-am.txt index 0d8ba48..dbea6e7 100644 --- a/Documentation/git-am.txt +++ b/Documentation/git-am.txt @@ -10,7 +10,7 @@ SYNOPSIS [verse] 'git am' [--signoff] [--keep] [--[no-]keep-cr] [--[no-]utf8] -[--3way] [--interactive] [--committer-date-is-author-date] +[--[no-]3way] [--interactive] [--committer-date-is-author-date] [--ignore-date] [--ignore-space-change | --ignore-whitespace] [--whitespace=] [-C] [-p] [--directory=] [--exclude=] [--include=] [--reject] [-q | --quiet] @@ -90,10 +90,13 @@ default. You can use `--no-utf8` to override this. -3:: --3way:: +--no-3way:: When the patch does not apply cleanly, fall back on 3-way merge if the patch records the identity of blobs it is supposed to apply to and we have those blobs - available locally. + available locally. `--no-3way` can be used to override + am.threeWay configuration variable. For more information, + see am.threeWay in linkgit:git-config[1]. --ignore-space-change:: --ignore-whitespace:: diff --git a/git-am.sh b/git-am.sh index c460dd0..75e701a 100755 --- a/git-am.sh +++ b/git-am.sh @@ -390,6 +390,11 @@ then keepcr=t fi +if test "$(git config --bool --get am.threeWay)" = true +then +threeway=t +fi + while test $# != 0 do case "$1" in @@ -401,6 +406,8 @@ it will be removed. Please do not use it anymore." ;; -3|--3way) threeway=t ;; + --no-3way) + threeway=f ;; -s|--signoff) sign=t ;; -u|--utf8) @@ -658,6 +665,8 @@ fi if test "$(cat "$dotest/threeway")" = t then threeway=t +else + threeway=f fi git_apply_opt=$(cat "$dotest/apply-opt") if test "$(cat "$dotest/sign")" = t diff --git a/t/t4150-am.sh b/t/t4150-am.sh index 6ced98c..b822a39 100755 --- a/t/t4150-am.sh +++ b/t/t4150-am.sh @@ -303,6 +303,25 @@ test_expect_success 'am -3 -p0 can read --no-prefix patch' ' git diff --exit-code lorem ' +test_expect_success 'am with config am.threeWay falls back to 3-way merge' ' + rm -fr .git/rebase-apply && + git reset --hard && + git checkout -b lorem4 base3way && + test_config am.threeWay 1 && + git am lorem-move.patch && + test_path_is_missing .git/rebase-apply && + git diff --exit-code lorem +' + +test_expect_success 'am with config am.threeWay overridden by --no-3way' ' + rm -fr .git/rebase-apply && + git reset --hard && + git checkout -b lorem5 base3way && + test_config am.threeWay 1 && + test_must_fail git am --no-3way lorem-move.patch && + test_path_is_dir .git/rebase-apply +' + test_expect_success 'am can rename a file' ' grep "^rename from" rename.patch && rm -fr .git/rebase-apply && -- 1.9.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 v5 1/3] git-am.sh: fix initialization of the threeway variable
Initialization for the threeway variable was missing. This caused a behavior change for command lines like: threeway=t git am ... This commit adds initialization for this variable. Signed-off-by: Remi Lespinet --- git-am.sh | 1 + 1 file changed, 1 insertion(+) diff --git a/git-am.sh b/git-am.sh index 761befb..c460dd0 100755 --- a/git-am.sh +++ b/git-am.sh @@ -378,6 +378,7 @@ committer_date_is_author_date= ignore_date= allow_rerere_autoupdate= gpg_sign_opt= +threeway= if test "$(git config --bool --get am.messageid)" = true then -- 1.9.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 v5 2/3] t4150-am: refactor am -3 tests
Create a setup for git am -3 in a separate test instead of creating this setup each time. This prepares for the next commit which will use this setup as well. Signed-off-by: Remi Lespinet --- t/t4150-am.sh | 32 ++-- 1 file changed, 14 insertions(+), 18 deletions(-) diff --git a/t/t4150-am.sh b/t/t4150-am.sh index 306e6f3..6ced98c 100755 --- a/t/t4150-am.sh +++ b/t/t4150-am.sh @@ -274,15 +274,21 @@ test_expect_success 'am --keep-non-patch really keeps the non-patch part' ' grep "^\[foo\] third" actual ' +test_expect_success 'setup am -3' ' + rm -fr .git/rebase-apply && + git reset --hard && + git checkout -b base3way master2 && + sed -n -e "3,\$p" msg >file && + head -n 9 msg >>file && + git add file && + test_tick && + git commit -m "copied stuff" +' + test_expect_success 'am -3 falls back to 3-way merge' ' rm -fr .git/rebase-apply && git reset --hard && - git checkout -b lorem2 master2 && - sed -n -e "3,\$p" msg >file && - head -n 9 msg >>file && - git add file && - test_tick && - git commit -m "copied stuff" && + git checkout -b lorem2 base3way && git am -3 lorem-move.patch && test_path_is_missing .git/rebase-apply && git diff --exit-code lorem @@ -291,12 +297,7 @@ test_expect_success 'am -3 falls back to 3-way merge' ' test_expect_success 'am -3 -p0 can read --no-prefix patch' ' rm -fr .git/rebase-apply && git reset --hard && - git checkout -b lorem3 master2 && - sed -n -e "3,\$p" msg >file && - head -n 9 msg >>file && - git add file && - test_tick && - git commit -m "copied stuff" && + git checkout -b lorem3 base3way && git am -3 -p0 lorem-zero.patch && test_path_is_missing .git/rebase-apply && git diff --exit-code lorem @@ -338,12 +339,7 @@ test_expect_success 'am -3 can rename a file after falling back to 3-way merge' test_expect_success 'am -3 -q is quiet' ' rm -fr .git/rebase-apply && git checkout -f lorem2 && - git reset master2 --hard && - sed -n -e "3,\$p" msg >file && - head -n 9 msg >>file && - git add file && - test_tick && - git commit -m "copied stuff" && + git reset base3way --hard && git am -3 -q lorem-move.patch >output.out 2>&1 && ! test -s output.out ' -- 1.9.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
format-patch and submodules
(Seen in git versions: 2.1.0 and 1.9.3 et al.) $ git format-patch --stdout X^..X | git apply check - fatal: unrecognized input This fails when the commit consists of nothing but a submodule change (as in 'git add submodule foo'), but it passes when a file change is added to the same commit. There used to be a similar problem for empty commits, but that was fixed around git-1.8: http://stackoverflow.com/questions/20775132/cannot-apply-git-patch-replacing-a-file-with-a-link Now, 'git format-patch' outputs nothing for an empty commit. I suppose that needs to be the behavior also when only submodules are changed, since in that case there is no 'diff' section from 'format-patch'. Use-case: git-p4 Of course, we do not plan to add the submodule into Perforce, but we would like this particular command to behave the same whether there are other diffs or not. ~Christopher Dunn (cdunn2001) -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[RFC] send-email aliases when editing patches or using --xx-cmd
On Thu, Jun 4, 2015 at 1:17 PM, Remi LESPINET wrote: > > Hi, > > Working on git-send-email, I've seen that there's no aliases support > when manually adding a recipient in a 'To' or 'Cc' field in a patch > and for the --to-cmd and --cc-cmd. > > I would like to add this support, and I wonder if there are reasons > not to do it. > > Thanks. I just realize that I totally messed up, Of course I don't want to add To or Cc fields to patches. In fact I want to add aliases support for --to-cmd and --cc-cmd options. But the modification depends on wheter we can use aliases in fields used by send-email (such as author or signoff-by..) when manually editing a patch or not. Sorry for this mistake :( -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re*: AW: Getting the full path of a conflicting file within a custom merge driver?
Junio C Hamano writes:
> "Gondek, Andreas" writes:
>
>> thank you for responding this fast. I would suggest providing this
>> information as an additional parameter (like %A %O %B and %L) maybe
>> %P.
>
> Yes, per-cent plus a letter is more in line with the way information
> is passed to the scripts already. Thanks for making a more sensible
> counter-suggestion. And your %P(ath) sounds like a sensible choice.
>
> It won't be a two-liner, though, as the path is an arbitrary string,
> unlike the names of the three temporary files, and needs to be
> quoted for the shell.
>
> Let me see if I can find time today to cook up something.
I had this in my outbox but then forgot to send it out.
-- >8 --
Subject: ll-merge: pass the original path to external drivers
The interface to custom low-level merge driver was modeled to be
capable of driving programs like "merge" (from the RCS suite) that
can produce result solely by looking at three files that hold
contents of common ancestor, ours and theirs. The information we
feed to the external drivers via the command line placeholders %O,
%A, and %B were designed to be purely about contents by giving
names of the temporary files that hold these variants without
exposing the original pathname. No matter where the result goes,
merging the same three variants should produce the same result,
contents is the king, that is the Git way.
The external driver interface, however, is meant to help people to
step outside the Git worldview, and sometimes people want to know
the final path that the resulting merged contents would be stored
in. Expose this to the external drivers via a new placeholder %P.
Requested-by: Andreas Gondek
Signed-off-by: Junio C Hamano
---
* t6026 is ancient and its style may need to be modernised to use
test_cmp intead of cmp, etc.
Documentation/gitattributes.txt | 5 -
ll-merge.c | 10 --
t/t6026-merge-attr.sh | 14 +-
3 files changed, 21 insertions(+), 8 deletions(-)
diff --git a/Documentation/gitattributes.txt b/Documentation/gitattributes.txt
index 70899b3..81fe586 100644
--- a/Documentation/gitattributes.txt
+++ b/Documentation/gitattributes.txt
@@ -774,7 +774,7 @@ To define a custom merge driver `filfre`, add a section to
your
[merge "filfre"]
name = feel-free merge driver
- driver = filfre %O %A %B
+ driver = filfre %O %A %B %L %P
recursive = binary
@@ -800,6 +800,9 @@ merge between common ancestors, when there are more than
one.
When left unspecified, the driver itself is used for both
internal merge and the final merge.
+The merge driver can learn the pathname in which the merged result
+will be stored via placeholder `%P`.
+
`conflict-marker-size`
^^
diff --git a/ll-merge.c b/ll-merge.c
index 8ea03e5..fc3c049 100644
--- a/ll-merge.c
+++ b/ll-merge.c
@@ -9,6 +9,7 @@
#include "xdiff-interface.h"
#include "run-command.h"
#include "ll-merge.h"
+#include "quote.h"
struct ll_merge_driver;
@@ -166,17 +167,20 @@ static int ll_ext_merge(const struct ll_merge_driver *fn,
{
char temp[4][50];
struct strbuf cmd = STRBUF_INIT;
- struct strbuf_expand_dict_entry dict[5];
+ struct strbuf_expand_dict_entry dict[6];
+ struct strbuf path_sq = STRBUF_INIT;
const char *args[] = { NULL, NULL };
int status, fd, i;
struct stat st;
assert(opts);
+ sq_quote_buf(&path_sq, path);
dict[0].placeholder = "O"; dict[0].value = temp[0];
dict[1].placeholder = "A"; dict[1].value = temp[1];
dict[2].placeholder = "B"; dict[2].value = temp[2];
dict[3].placeholder = "L"; dict[3].value = temp[3];
- dict[4].placeholder = NULL; dict[4].value = NULL;
+ dict[4].placeholder = "P"; dict[4].value = path_sq.buf;
+ dict[5].placeholder = NULL; dict[5].value = NULL;
if (fn->cmdline == NULL)
die("custom merge driver %s lacks command line.", fn->name);
@@ -210,6 +214,7 @@ static int ll_ext_merge(const struct ll_merge_driver *fn,
for (i = 0; i < 3; i++)
unlink_or_warn(temp[i]);
strbuf_release(&cmd);
+ strbuf_release(&path_sq);
return status;
}
@@ -269,6 +274,7 @@ static int read_merge_config(const char *var, const char
*value, void *cb)
*%A - temporary file name for our version.
*%B - temporary file name for the other branches' version.
*%L - conflict marker length
+*%P - the original path (safely quoted for the shell)
*
* The external merge driver should write the results in the
* file named by %A, and signal that it has done with zero exit
diff --git a/t/t6026-merge-attr.sh b/t/t6026-merge-attr.sh
i
[RFC] send-email aliases when editing patches or using --xx-cmd
> On Thu, Jun 4, 2015 at 5:11 PM, Stefan Beller wrote: > > On Thu, Jun 4, 2015 at 1:17 PM, Remi LESPINET > > wrote: > >> Working on git-send-email, I've seen that there's no aliases support > >> when manually adding a recipient in a 'To' or 'Cc' field in a patch > >> and for the --to-cmd and --cc-cmd. > >> > >> I would like to add this support, and I wonder if there are reasons > >> not to do it. > > > > Just recently Allen Hubbe did work on alias support, > > I did not follow that topic though. > > Allen's patch added support for another aliases format file but is > unrelated to Remi's proposal and would not conflict with it. Yes, I have read and applied the patches, this does not seems to be a problem. Thanks -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 1/2] extract setting of wt_status.commitable flag out of wt_status_print_updated()
Tay Ray Chuan writes: > It may not be obvious from its name that wt_status_print_updated() that > it also sets wt_status.commitable, which affects commit functionality. > Extract this out into a separate function for improved clarity, though > at the expense of executing another loop. Makes sense. > @@ -1360,6 +1374,7 @@ void wt_status_print(struct wt_status *s) > > wt_status_print_updated(s); > wt_status_print_unmerged(s); > + wt_status_mark_commitable(s); > wt_status_print_changed(s); > if (s->submodule_summary && > (!s->ignore_submodule_arg || As this is the only callsite of _updated(), we can be assured that the conversion would not change the behaviour. But I am not sure the placement of the new call is sensible. The standard pattern used in the wt-status infrastructure is to first collect the information and then make output based on what was collected. Because the value of this patch is to separte the "is it committable?" information gathering step out of the output step, shouldn't the call be made a lot earlier than these sequence of wt_status_print_blah() calls? I am wondering if the flipping of the "is it committable?" bit belongs to wt_status_collect(). It could be that some other crufty checks that wt_status_print() have accumulated over time might be better moved to the "collect" phase, but that is a separate topic. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC] send-email aliases when editing patches or using --xx-cmd
On Thu, Jun 4, 2015 at 5:11 PM, Stefan Beller wrote: > On Thu, Jun 4, 2015 at 1:17 PM, Remi LESPINET > wrote: >> Working on git-send-email, I've seen that there's no aliases support >> when manually adding a recipient in a 'To' or 'Cc' field in a patch >> and for the --to-cmd and --cc-cmd. >> >> I would like to add this support, and I wonder if there are reasons >> not to do it. > > Just recently Allen Hubbe did work on alias support, > I did not follow that topic though. Allen's patch added support for another aliases format file but is unrelated to Remi's proposal and would not conflict with it. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC] send-email aliases when editing patches or using --xx-cmd
On Thu, Jun 4, 2015 at 1:17 PM, Remi LESPINET wrote: > > Hi, > > Working on git-send-email, I've seen that there's no aliases support > when manually adding a recipient in a 'To' or 'Cc' field in a patch > and for the --to-cmd and --cc-cmd. > > I would like to add this support, and I wonder if there are reasons > not to do it. > > Thanks. Just recently Allen Hubbe did work on alias support, I did not follow that topic 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 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[RFC] send-email aliases when editing patches or using --xx-cmd
Hi, Working on git-send-email, I've seen that there's no aliases support when manually adding a recipient in a 'To' or 'Cc' field in a patch and for the --to-cmd and --cc-cmd. I would like to add this support, and I wonder if there are reasons not to do it. 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: Suggestion: make git checkout safer
On 2015-06-04 13.00, Ed Avis wrote: > > >Updates files in the working tree to match... I think that this had been written with "git checkout " in mind, which is different from "git checkout -- " (or "git checkout .") Do you think you can write a patch to improve the documentation ? -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Permission denied ONLY after pulling bundles
From: "Christian Couder" Hi, On Thu, Jun 4, 2015 at 3:04 PM, Rossella Barletta wrote: Dear git group, I would like to ask your help for a problem that we cannot fix in any way. We have a git repository in folder on Windows. Then we use VMware player on CentOS_6 on which we create a clone of the git repository, after of course having mounted the directory in which the repository is. So the repository is on windows and the clone on Linux. We are able to perfom all the git operations we need, except for the pull .bundle, which is successful in itself but prevent us from pushing after that. It is not very clear how the bundle has been made, and on which machine you made it and you pulled from it. As we try to push after pulling a .bundle in a branch we get the error message NODE1:fdp> git push Counting objects: 1977, done. Delta compression using up to 2 threads. Compressing objects: 100% (423/423), done. fatal: write error: Permission denied00 KiB | 158 KiB/s error: pack-objects died of signal 13 error: pack-objects died with strange error Can you have a look at the machine you push to and see if some file or directory permissions changed between before and after you made the bundle or you pulled the bundle? We have checked all the permissions, changed the users, recreated the clone but nothing worked. What do you mean by checked all the permissions? You mean that permissions haven't changed at all since before you pulled the first bundle? The push operation works perfectly until we pull a bundle. After pulling a bundle we are not able to push anymore.We tryed to delete the branches, recreate others and all works perfectly, also the push.As we pull the .bundle we cannot get the permission to do the push anymore. What has this to do with the bundle? Did you try to everything (cloning, creating a bundle, pulling it and pushing on the same machine to see if it makes a difference? Also did you try with another smaller fake repository? If you can reproduce with a smaller fake repo on just one machine it could help us reproduce on one of our machine and have a look. And could you tell us which version of git (using git --version) you are using on both machines? -- Also, check the config file to see if the push url has somehow changed to the bundle file? I know that if you clone from a bundle the origin is set to the bundle file, so that you can push back into it for the return sneakernet (RFC 1194 ?) journey. It maybe that fetching from a bundle does the same setup (not looked at the code) -- Philip -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Permission denied ONLY after pulling bundles
Hi, On Thu, Jun 4, 2015 at 3:04 PM, Rossella Barletta wrote: > Dear git group, > > > I would like to ask your help for a problem that we cannot fix in any way. > > We have a git repository in folder on Windows. > > Then we use VMware player on CentOS_6 on which we create a clone of > the git repository, after of course having mounted the directory in > which the repository is. > > So the repository is on windows and the clone on Linux. > > We are able to perfom all the git operations we need, except for the > pull .bundle, which is successful in itself but prevent us from > pushing after that. It is not very clear how the bundle has been made, and on which machine you made it and you pulled from it. > As we try to push after pulling a .bundle in a branch we get the error message > > NODE1:fdp> git push > Counting objects: 1977, done. > Delta compression using up to 2 threads. > Compressing objects: 100% (423/423), done. > fatal: write error: Permission denied00 KiB | 158 KiB/s > error: pack-objects died of signal 13 > error: pack-objects died with strange error Can you have a look at the machine you push to and see if some file or directory permissions changed between before and after you made the bundle or you pulled the bundle? > We have checked all the permissions, changed the users, recreated the > clone but nothing worked. What do you mean by checked all the permissions? You mean that permissions haven't changed at all since before you pulled the first bundle? > The push operation works perfectly until we pull a bundle. After > pulling a bundle we are not able to push anymore.We tryed to delete > the branches, recreate others and all works perfectly, also the > push.As we pull the .bundle we cannot get the permission to do the > push anymore. > > What has this to do with the bundle? Did you try to everything (cloning, creating a bundle, pulling it and pushing on the same machine to see if it makes a difference? Also did you try with another smaller fake repository? If you can reproduce with a smaller fake repo on just one machine it could help us reproduce on one of our machine and have a look. And could you tell us which version of git (using git --version) you are using on both machines? Thanks, Christian. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 0/2] make commit --verbose work with --no-status
Tay Ray Chuan writes: > When running git-commit`, --verbose appends a diff to the prepared > message, while --no-status omits git-status output. The --verbose option is called --verbose and not --diff or --patch for a reason, though. The default is to show extra information as comments, and verbose tells us to make that extra information more verbose. We call that extra information "status", so it is natural for "--no-status" to drop that extra information. > ; thus, one would > expect --verbose --no-status to give a commit message with a diff of > the commit without git-status output. > > However, this is not what happens And for a good reason, I would think. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [WIP PATCH 1/3] git bisect old/new
Antoine Delaite writes: > From: Christian Couder > > When not looking for a regression during a bisect but for a fix or a > change in another given property, it can be confusing to use 'good' > and 'bad'. > > This patch introduce `git bisect new` and `git bisect old` as an > alternative to 'bad' and good': the commits which have the most > recent version of the property must be marked as `new` and the ones > with the older version as `old`. The phrase "the most recent version of" invites an unnecessary confusion. "git bisect" is a tool to find a single bit flip and if the behaviour changed twice in the history from A to B to C, you cannot look for the transition between A to B or B to C. You need to say "I classify behaviour B and C to be both good (new) and I know it used to have behaviour A which was bad (old)". You can mark the commits with the "new" property as "new", and the "old" property as "old"; "good" being "new" and "bad" being "old" becomes a mere special case of this. The "new" property in the traditional bisection is "commit has the bug we are hunting", while "old" is "commit lacks that bug". > diff --git a/Documentation/git-bisect.txt b/Documentation/git-bisect.txt > index 4cb52a7..12c7711 100644 > --- a/Documentation/git-bisect.txt > +++ b/Documentation/git-bisect.txt > ... > @@ -104,6 +104,33 @@ For example, `git bisect reset HEAD` will leave you on > the current > bisection commit and avoid switching commits at all, while `git bisect > reset bisect/bad` will check out the first bad revision. > > + > +Alternative terms: bisect new and bisect old > + > + > +If you are not at ease with the terms "bad" and "good", perhaps > +because you are looking for the commit that introduced a fix, you can > +alternatively use "new" and "old" instead. > +But note that you cannot mix "bad" and good" with "new" and "old". There is a dq missing somewhere on this line. > + > + > +git bisect new [] > + > + > +Marks the commit as new, which means the version is fixed. I am not sure ", which means the version is fixed." is a good idea. The whole point of introducing "old" vs "new", the reason why we may want to do this patch series, is to reduce confusion by removing the value judgement associated with "good" vs "bad". That is what made the bisect confusing when the old ones were bad and new ones were good. Saying "fixed" implicitly contrasts it with "broken", and introduces the value judgement on the states again. Side note: yes, if the reader is careful, "fixed" needs to have previous states to be "broken", so in that sense, "fixed" can be read about time-sequence and not about value judgement. But let's try to be understandable by even casual readers. We would instead want to emphasize that what we are judging is not values but time sequence. You would want to say "It used to be X but it no longer is X", without saying if X is a good thing or not, e.g. Marks the commit as new, e.g. "the bug is no longer there", if you are looking for a commit that fixed a bug, or "the feature that used to work is now broken at this point", if you are looking for a commit that introduced a bug. > + > +git bisect old [...] > + > + > +Marks the commit as old, which means the bug is present. This is much worse than the previous one. Unless you force the reader to _assume_ that you are hunting for a fix, "bug is present" can mean "bug is still present", or "bug was introduced somewhere along the line and now it is there". And a casual mention of "perhaps because you are looking for a fix" at the beginning of the section is not strong enough hint that these examples are _only_ about "hunting for bugfix". I'd stop here for now, but a casual skimming told me that the same issue exists throughout the remainder of the doc. 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
[PATCH v2 0/2] make commit --verbose work with --no-status
When running git-commit`, --verbose appends a diff to the prepared message, while --no-status omits git-status output; thus, one would expect --verbose --no-status to give a commit message with a diff of the commit without git-status output. However, this is not what happens - the prepared commit message body is empty, entirely. (Needless to say, no diff is appended.) This patch series attempts to make this work, as one would expect. [PATCH 1/2] extract setting of wt_status.commitable flag out of wt_status_print_updated() Currently, the wt_status.commitable flag is set only when we call wt_status_print(). * Extract this logic, so that we don't have to go through the git-status output code just to set the flag. * In fact, it is not set when --short or --porcelain are used. This series does not attempt to fix the bug, though it should be trivial to do so with this patch. See 9cbcc2a (demonstrate git-commit --dry-run exit code behaviour, Feb 22 2014). [PATCH 2/2] make commit --verbose work with --no-status Actual work here. -- 2.0.0.581.g64f2558 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 2/2] make commit --verbose work with --no-status
When running git-commit`, --verbose appends a diff to the prepared
message, while --no-status omits git-status output; thus, one would
expect --verbose --no-status to give a commit message with a diff of
the commit without git-status output.
However, this is not what happens - the prepared commit message body is
empty, entirely. (Needless to say, no diff is appended.) This is because
internally the git-status machinery is used to provide both the diff and
status output, but this machinery is skipped over entirely due to
--no-status.
We introduce a new status_format, STATUS_FORMAT_DIFFONLY, which triggers
the setting of the commitable flag, and the printing of the diff. This
is set only by git-commit, and when it detects that --verbose and
--no-status have been used.
Signed-off-by: Tay Ray Chuan
---
Changed since v1: adopted peff's suggestion in
20140224083312.gb32...@sigill.intra.peff.net, added tests.
builtin/commit.c | 12 +++-
t/t7507-commit-verbose.sh | 34 +-
wt-status.c | 2 +-
wt-status.h | 3 +++
4 files changed, 48 insertions(+), 3 deletions(-)
diff --git a/builtin/commit.c b/builtin/commit.c
index 254477f..d752899 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -144,6 +144,7 @@ static enum status_format {
STATUS_FORMAT_LONG,
STATUS_FORMAT_SHORT,
STATUS_FORMAT_PORCELAIN,
+ STATUS_FORMAT_DIFFONLY,
STATUS_FORMAT_UNSPECIFIED
} status_format = STATUS_FORMAT_UNSPECIFIED;
@@ -510,6 +511,10 @@ static int run_status(FILE *fp, const char *index_file,
const char *prefix, int
case STATUS_FORMAT_UNSPECIFIED:
die("BUG: finalize_deferred_config() should have been called");
break;
+ case STATUS_FORMAT_DIFFONLY:
+ wt_status_mark_commitable(s);
+ wt_status_print_verbose(s);
+ break;
case STATUS_FORMAT_NONE:
case STATUS_FORMAT_LONG:
wt_status_print(s);
@@ -1213,7 +1218,12 @@ static int parse_and_validate_options(int argc, const
char *argv[],
if (all && argc > 0)
die(_("Paths with -a does not make sense."));
- if (status_format != STATUS_FORMAT_NONE)
+ if (status_format == STATUS_FORMAT_NONE) {
+ if (verbose && !include_status) {
+ include_status = 1;
+ status_format = STATUS_FORMAT_DIFFONLY;
+ }
+ } else
dry_run = 1;
return argc;
diff --git a/t/t7507-commit-verbose.sh b/t/t7507-commit-verbose.sh
index 2ddf28c..9027dd4 100755
--- a/t/t7507-commit-verbose.sh
+++ b/t/t7507-commit-verbose.sh
@@ -26,7 +26,39 @@ test_expect_success 'initial commit shows verbose diff' '
git commit --amend -v
'
-test_expect_success 'second commit' '
+test_expect_success '--verbose appends diff' '
+ cat >expected <<-\EOF &&
+ # >8
+ # Do not touch the line above.
+ # Everything below will be removed.
+ diff --git a/file b/file
+ index d95f3ad..94ab063 100644
+ --- a/file
+ +++ b/file
+ @@ -1 +1,2 @@
+content
+ +content content
+ EOF
+ cat >editor <<-\EOF &&
+ #!/bin/sh
+ awk "/^# -+ >8 -+$/ { p=1 } p" "$1" >actual
+ echo commit > "$1"
+ EOF
+ chmod 755 editor &&
+ echo content content >> file &&
+ git add file &&
+ test_tick &&
+ EDITOR=./editor git commit --verbose &&
+ test_cmp expected actual
+'
+
+test_expect_success '--verbose --no-status appends diff' '
+ git reset --soft HEAD^ &&
+ EDITOR=./editor git commit --verbose --no-status &&
+ test_cmp expected actual
+'
+
+test_expect_success 'commit' '
echo content modified >file &&
git add file &&
git commit -F message
diff --git a/wt-status.c b/wt-status.c
index 87550ae..c4f7e48 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -857,7 +857,7 @@ void wt_status_add_cut_line(FILE *fp)
strbuf_release(&buf);
}
-static void wt_status_print_verbose(struct wt_status *s)
+void wt_status_print_verbose(struct wt_status *s)
{
struct rev_info rev;
struct setup_revision_opt opt;
diff --git a/wt-status.h b/wt-status.h
index e0a99f7..4388296 100644
--- a/wt-status.h
+++ b/wt-status.h
@@ -98,8 +98,11 @@ void wt_status_add_cut_line(FILE *fp);
void wt_status_prepare(struct wt_status *s);
void wt_status_print(struct wt_status *s);
void wt_status_collect(struct wt_status *s);
+void wt_status_mark_commitable(struct wt_status *state);
void wt_status_get_state(struct wt_status_state *state, int get_detached_from);
+void wt_status_print_verbose(struct wt_status *s);
+
void wt_shortstatus_print(struct wt_status *s);
void wt_porcelain_print(struct wt_status *s);
--
2.0.0.581.g64f2558
--
To unsubscribe from this list: send the line "unsubscribe git"
[PATCH v2 1/2] extract setting of wt_status.commitable flag out of wt_status_print_updated()
It may not be obvious from its name that wt_status_print_updated() that
it also sets wt_status.commitable, which affects commit functionality.
Extract this out into a separate function for improved clarity, though
at the expense of executing another loop.
Signed-off-by: Tay Ray Chuan
---
Changed since v1
- move call to _mark_committable() to match original control-flow
Originally, our placement of the call perhaps befitted aesthetics /
logical grouping. But perhaps it is a better idea to match the original
control flow to dispel any suspicion that this patch changed behaviour
unintendedly.
wt-status.c | 17 -
1 file changed, 16 insertions(+), 1 deletion(-)
diff --git a/wt-status.c b/wt-status.c
index c56c78f..87550ae 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -626,6 +626,21 @@ void wt_status_collect(struct wt_status *s)
wt_status_collect_untracked(s);
}
+void wt_status_mark_commitable(struct wt_status *s)
+{
+ int i;
+
+ for (i = 0; i < s->change.nr; i++) {
+ struct wt_status_change_data *d;
+ d = s->change.items[i].util;
+ if (!d->index_status ||
+ d->index_status == DIFF_STATUS_UNMERGED)
+ continue;
+ s->commitable = 1;
+ break;
+ }
+}
+
static void wt_status_print_unmerged(struct wt_status *s)
{
int shown_header = 0;
@@ -664,7 +679,6 @@ static void wt_status_print_updated(struct wt_status *s)
continue;
if (!shown_header) {
wt_status_print_cached_header(s);
- s->commitable = 1;
shown_header = 1;
}
wt_status_print_change_data(s, WT_STATUS_UPDATED, it);
@@ -1360,6 +1374,7 @@ void wt_status_print(struct wt_status *s)
wt_status_print_updated(s);
wt_status_print_unmerged(s);
+ wt_status_mark_commitable(s);
wt_status_print_changed(s);
if (s->submodule_summary &&
(!s->ignore_submodule_arg ||
--
2.0.0.581.g64f2558
--
To unsubscribe from this list: send the line "unsubscribe 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] index-pack: fix truncation of off_t in comparison
Jeff King writes: > On top of nd/slim-index-pack-memory-usage, which introduced the bug (but > it is already in master). Thanks. In this round, I decided to deliberately merge more iffy and larger topics to 'master' in early part of the cycle, and it seems to be paying off nicely ;-). Will queue. > builtin/index-pack.c | 8 ++-- > 1 file changed, 6 insertions(+), 2 deletions(-) > > diff --git a/builtin/index-pack.c b/builtin/index-pack.c > index 3ed53e3..06dd973 100644 > --- a/builtin/index-pack.c > +++ b/builtin/index-pack.c > @@ -616,7 +616,9 @@ static int compare_ofs_delta_bases(off_t offset1, off_t > offset2, > int cmp = type1 - type2; > if (cmp) > return cmp; > - return offset1 - offset2; > + return offset1 < offset2 ? -1 : > +offset1 > offset2 ? 1 : > +0; > } > > static int find_ofs_delta(const off_t offset, enum object_type type) > @@ -1051,7 +1053,9 @@ static int compare_ofs_delta_entry(const void *a, const > void *b) > const struct ofs_delta_entry *delta_a = a; > const struct ofs_delta_entry *delta_b = b; > > - return delta_a->offset - delta_b->offset; > + return delta_a->offset < delta_b->offset ? -1 : > +delta_a->offset > delta_b->offset ? 1 : > +0; > } > > static int compare_ref_delta_entry(const void *a, const void *b) -- To unsubscribe from this list: send the line "unsubscribe 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] git-am: handling unborn branches
On Thu, Jun 4, 2015 at 3:34 AM, Paul Tan wrote: > Hi, > > git-am generally supports applying patches to unborn branches. > However, there are 2 cases where git-am does not handle unborn > branches which I would like to address before the git-am rewrite to C: > > 1. am --skip > > For git am --skip, git-am.sh does a fast-forward checkout from HEAD to > HEAD, discarding unmerged entries, and then resets the index to HEAD > so that the index is not dirty. > > git read-tree --reset -u HEAD HEAD > orig_head=$(cat "$GIT_DIR/ORIG_HEAD") > git reset HEAD > git update-ref ORIG_HEAD $orig_head > > This requires a valid HEAD. Since git-am requires an empty index for > unborn branches in the patch application stage anyway, I think we > should discard all entires in the index if we are on an unborn branch? That makes sense. > > Or, the current behavior of git-am.sh will print some scary errors > about the missing HEAD, but will then continue on to the next patch. > If the index is not clean, it will then error out. Should we preserve > this behavior? (without the errors about the missing HEAD) > > 2. am --abort > > For git am --abort, git-am.sh does something similar. It does a > fast-forward checkout from HEAD to ORIG_HEAD, discarding unmerged > entries, and then resets the index to ORIG_HEAD so that local changes > will be unstaged. > > if safe_to_abort > then > git read-tree --reset -u HEAD ORIG_HEAD > git reset ORIG_HEAD > fi > rm -fr "$dotest" > > This, however, requires a valid HEAD and ORIG_HEAD. If we don't have a > HEAD or ORIG_HEAD (because we were on an unborn branch when we started > git am), what should we do? (Note: safe_to_abort returns true if we > git am with no HEAD because $dotest/abort-safety will not exist) > Should we discard all entires in the index as well? (Since we might > think of the "original HEAD" as an empty tree?) > > Or, the current behavior of git-am.sh will print some scary errors > about the missing HEAD and ORIG_HEAD, but will not touch the index at > all, and still delete the rebase-apply directory. Should we preserve > this behavior (without the errors)? I guess so, looking at the documentation --abort Restore the original branch and abort the patching operation. a user may want to not go to the unborn branch, but rather to the previous HEAD? > > Thanks, > Paul -- To unsubscribe from this list: send the line "unsubscribe 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] git-am: handling unborn branches
Paul Tan writes: > git-am generally supports applying patches to unborn branches. > However, there are 2 cases where git-am does not handle unborn > branches which I would like to address before the git-am rewrite to C: > 1. am --skip > > For git am --skip, git-am.sh does a fast-forward checkout from HEAD to > HEAD, discarding unmerged entries, and then resets the index to HEAD > so that the index is not dirty. > > git read-tree --reset -u HEAD HEAD > orig_head=$(cat "$GIT_DIR/ORIG_HEAD") > git reset HEAD > git update-ref ORIG_HEAD $orig_head > > This requires a valid HEAD. Since git-am requires an empty index for > unborn branches in the patch application stage anyway, I think we > should discard all entires in the index if we are on an unborn branch? Yes, and it should also remove the new files the failed application brought in to the working tree, if any, to match the "--skip" done in the normal case (i.e. when we already have a history to apply patches to), I would think. > 2. am --abort > > For git am --abort, git-am.sh does something similar. It does a > fast-forward checkout from HEAD to ORIG_HEAD, discarding unmerged > entries, and then resets the index to ORIG_HEAD so that local changes > will be unstaged. In general, the "apply to nothing" is more or less an afterthought and was not done as carefully as the rest of the program, so view whenever you see a strange behaviour as not a "strange spec" but likely to be a bug. You would do OK if you imagine what should happen if you were doing the same operation on top of a commit that records an empty tree and try to match the behaviour to that case. 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/4] status: give more information during rebase -i
Matthieu Moy writes:
>> +void get_two_last_lines(char *filename, int *numlines, char **lines)
>> +{
>> +...
>> +}
>> +
>> +void get_two_first_lines(char *filename, int *numlines, char **lines)
>> +{
>> +...
>> +}
I had a handful of comments on these:
- Do we need two separate and overly specific functions like these,
i.e. "two" and "first/last"?
- Wouldn't people want to be able to configure the number of lines?
- Do we really want get_two_{first,last}_LINES() functions?
I am wondering if insn sheets these functions read include
comments, in which case get_n_{first,last}_commands() may be a
more correct name.
- Wouldn't it be necessary for these functions to report the total
number of commands, instead of giving "void" back? Otherwise how
would the caller produce summary like this:
An interactive rebase of 14 commits in progress. You have
replayed 4 commits so far, the last few of which were:
da66b27 remote.c: provide per-branch pushremote name
f052154 remote.c: hoist branch.*.remote lookup out of
and 10 more commits to go, the next few of which are:
a9f9f8c remote.c: introduce branch_get_upstream helper
8770e6f remote.c: hoist read_config into remote_get_1
Note that I am not suggesting the phrasing or presentation. The
information content is what I am interested in.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] test for '!' handling in rev-parse's named commits
Will Palmer writes:
> What I'm thinking now is that "@^{/foo}" can be thought of as a
> potential "shorthand-form" of what could be "@^{/!(m=foo)}", in which
> case "@^{/!-foo}" could similarly be thought of as a potential
> shorthand-form of what could be "@^{/!(m-foo)}".
Ah, our messages crossed, it seems. Yes, I think we are on the same
page, and it is sensible to think of "/!-string" as a short-hand for
the more complete syntax that uses descriptive word, not mnemonic,
e.g. "/!(unmatch=string)", that the old thread envisioned.
I think it is OK (and probably preferrable) to start with only
"/!-string" without the long-hand, as we do not know how multiple
long-hand instructions should interact with each other.
Thanks.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/WIP PATCH 11/11] Document protocol version 2
Jeff King writes: > We should definitely _not_ add anything that scales with the repository > size. For instance, the "symref" field only shows the "HEAD" now. That's > OK, as it's constant size. I agree that this is an easy-to-explain rule to keep the design sensible. > We do not show _all_ symrefs right now > because of pkt-line length limitations. With v2, we could in theory > start doing so (one per pkt-line). But that does not make it a good > idea. Very true. If we want symref information conveyed, then we should either make a new "symref advertisement" available (and it is OK to use a single-bit capability to enable or disable that new section), or make the "ref advertisement" natively capable of always doing so (i.e. if there is no need to make this optional). -- To unsubscribe from this list: send the line "unsubscribe 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/WIP PATCH 00/11] Protocol version 2, again!
On Thu, Jun 4, 2015 at 6:09 AM, Jeff King wrote:
> On Mon, Jun 01, 2015 at 10:49:45AM -0700, Stefan Beller wrote:
>
>> However the client side with builtin/fetch, builtin/fetch-pack, fetch-pack
>> is a bit of a mystery to me, as I cannot fully grasp the difference between
>> * connect.{h,c}
>> * remote.{h.c}
>> * transport.{h.c}
>> there. All of it seems to be doing network related stuff, but I have trouble
>> getting the big picture. I am assuming all of these 3 are rather a low level,
>> used like a library, though there must be even more hierarchy in there,
>> connect is most low level judging from the header file and used by
>> the other two.
>> transport.h seems to provide the most toplevel library stuff as it includes
>> remote.h in its header?
>
> connect.c was originally "the git protocol", and was used by send-pack
> and fetch-pack. Other individual programs implemented other transports.
> Later, as the interface moved towards everybody running "fetch" and
> "push", and those delegating work to the individual transports, we got
> transport.c, which is an abstract interface for all transports. It
> delegates actual git-protocol work to the functions in connect.c (or
> bundle work elsewhere, or handles remote-helpers itself).
>
> And then remote.c contains routines for dealing with the remotes at a
> logical level. E.g., which refs to fetch or push, etc.
>
> So in theory, the flow is something like:
>
> - fetch.c knows "the user wants to fetch from 'foo'"
>
> - fetch asks remote.c: "who is remote 'foo'"; we get back a URL
>
> - fetch asks transport.c: "what are the refs for $URL"
>
> - it turns out to be a git URL. transport.c calls into connect.c to
> implement get_refs_via_connect.
Currently the distinction which protocol to speak is made
here (in get_refs_via_connect), which may be a bit late. Though I updating
the git protocol only first would also be feasible.
So for the next git protocol
- get_refs_via_connect first asks for the capabilities and gets an answer from
upload-pack-2. Now what?
- we could have a callback in struct transport, which must be set
accordingly by
fetch in step 4 (it turns out to be a git URL. transport.c ...)
This callback is called with each pkt-line such as
void parse-capability(char *line, struct
*transport_capabilities, void *cdata);
The line would contain the pkt-line, while the transport_capabilities
would be a struct
similar as in "[RFCv2 06/16] remote.h: add new struct for options",
where the fetch
implementation must select the right bits. Looking at fetch-pack.{c,h}
we only expose
one do-it-all method there, so we currently don't have file wide
easily accessible variables,
but rather all in a struct fetch_pack_args, which carries important
information for the
selection process such as verbosity or desired options. This is why a
void* comes in
handy as well. (It will be easy later to adapt that to the sending
side as well).
Instead of a full grown line by line callback we could also just
collect all the capabilities
first in a string list and then only call back once into a
void select_capabilities(struct string_list *available, struct
string_list *selected);
I think I'd find this second approach more handy as there are subtle
details you'd miss in
the first approach. Looking at fetch-pack.c, do_fetch_pack (line 790),
we have one
selection (no_done) conditioned on another (multi_ack_detailed), so
having the full list
there makes the code easier.
This second approach however might not be as future proof as the
first, because we store
all received capabilities (which may grow large in the future) and not
throw unknowns away
immediately.
I tend to rather implement the second one (easier to read/maintain trumps a
maybe-performance-problem-in-the-future).
This performance-problem-in-the-future could be mitigated easily by having a
preselection in transport.c get_capabilities, which ignores any capabilities not
white listed there (harder to maintain though, as we have a more than one spot
where to put a list)
By writing this mail I realized another thing. I have had the patch
"[RFCv2 09/16] remote.h: add get_remote_capabilities, request_capabilities"
which has request_capabilities just translating from a struct
containing some bits
into a sequence of pkt-lines containing the actual protocol
capabilities. Maybe we
should not have that in the connect file, but rather as proposed in
this email, the
high level command directly selects the strings to put back on the
wire. (By having
"struct string_list *selected" as part of the select_capabilities arguments.)
then the request_capabilities in connect.c would be dumbed down to just:
void request_capabilities(int out, struct string_list *list)
{
struct string_list_item *item;
for_each_string_list_item(item, list) {
packet_write(out, item->string);
}
packet_flush(out);
}
I think that would be reasonable?
>
> -
Re: [PATCH mh/lockfile-retry] lockfile: replace random() by rand()
On Thu, Jun 4, 2015 at 4:42 AM, Michael Haggerty wrote:
> On 06/04/2015 10:40 AM, Johannes Sixt wrote:
> We *certainly* don't require high-quality random numbers for this
> application. Regarding portability, there is one definite point in favor
> of rand() (it's available on Windows) vs. Junio's recollection that
> random() might have portability advantages, presumably on other platforms.
I agree that anything is OK in this codepath. I just suspected that whichever
one we pick, there would be somebody who says "oh, my system lacks it",
and we will end up adding one of compat/{rand,random}.c to emulate.
And when I anticipated that future, my inclination was to prefer random(),
not rand(), used in the code, not the other way around. Yes, both are in
POSIX, but I was getting the impression that rand() is on its way out
(and rand_r() is already marked as obsolete).
> Maybe the easiest thing would be to switch to using rand() and see if
> the OS/2 and VMS users complain ;-)
We can certainly go that route and I am fine with that as the solution for
today, as long as somebody will remember this discussion when that
complaint comes, and make compat/random.c, and switch the
in-code use to random(), instead of sticking to use of rand() in code.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH/RFC v4 3/3] git-am: add am.threeWay config variable
Remi Lespinet writes: > @@ -90,10 +90,13 @@ default. You can use `--no-utf8` to override this. > > -3:: > --3way:: > +--no-3way:: > When the patch does not apply cleanly, fall back on > 3-way merge if the patch records the identity of blobs > it is supposed to apply to and we have those blobs > - available locally. > + available locally. `--no-3way` can be used to override > + am.threeWay configuration variable. For more information, > + see am.threeWay in git-config(1). ... in linkgit:git-config[1]. -- 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
[PATCH/RFC v4 1/3] git-am.sh: fix initialization of the threeway variable
Initialization for the threeway variable was missing. This caused a behavior change for command lines like: threeway=t git am ... This commit adds initialization for this variable. Signed-off-by: Remi Lespinet --- git-am.sh | 1 + 1 file changed, 1 insertion(+) diff --git a/git-am.sh b/git-am.sh index 761befb..c460dd0 100755 --- a/git-am.sh +++ b/git-am.sh @@ -378,6 +378,7 @@ committer_date_is_author_date= ignore_date= allow_rerere_autoupdate= gpg_sign_opt= +threeway= if test "$(git config --bool --get am.messageid)" = true then -- 1.9.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/RFC v4 2/3] t4150-am: refactor am -3 tests
Create a setup for git am -3 in a separate test instead of creating this setup each time. This prepares for the next commit which will use this setup as well. Signed-off-by: Remi Lespinet --- t/t4150-am.sh | 32 ++-- 1 file changed, 14 insertions(+), 18 deletions(-) diff --git a/t/t4150-am.sh b/t/t4150-am.sh index 306e6f3..6ced98c 100755 --- a/t/t4150-am.sh +++ b/t/t4150-am.sh @@ -274,15 +274,21 @@ test_expect_success 'am --keep-non-patch really keeps the non-patch part' ' grep "^\[foo\] third" actual ' +test_expect_success 'setup am -3' ' + rm -fr .git/rebase-apply && + git reset --hard && + git checkout -b base3way master2 && + sed -n -e "3,\$p" msg >file && + head -n 9 msg >>file && + git add file && + test_tick && + git commit -m "copied stuff" +' + test_expect_success 'am -3 falls back to 3-way merge' ' rm -fr .git/rebase-apply && git reset --hard && - git checkout -b lorem2 master2 && - sed -n -e "3,\$p" msg >file && - head -n 9 msg >>file && - git add file && - test_tick && - git commit -m "copied stuff" && + git checkout -b lorem2 base3way && git am -3 lorem-move.patch && test_path_is_missing .git/rebase-apply && git diff --exit-code lorem @@ -291,12 +297,7 @@ test_expect_success 'am -3 falls back to 3-way merge' ' test_expect_success 'am -3 -p0 can read --no-prefix patch' ' rm -fr .git/rebase-apply && git reset --hard && - git checkout -b lorem3 master2 && - sed -n -e "3,\$p" msg >file && - head -n 9 msg >>file && - git add file && - test_tick && - git commit -m "copied stuff" && + git checkout -b lorem3 base3way && git am -3 -p0 lorem-zero.patch && test_path_is_missing .git/rebase-apply && git diff --exit-code lorem @@ -338,12 +339,7 @@ test_expect_success 'am -3 can rename a file after falling back to 3-way merge' test_expect_success 'am -3 -q is quiet' ' rm -fr .git/rebase-apply && git checkout -f lorem2 && - git reset master2 --hard && - sed -n -e "3,\$p" msg >file && - head -n 9 msg >>file && - git add file && - test_tick && - git commit -m "copied stuff" && + git reset base3way --hard && git am -3 -q lorem-move.patch >output.out 2>&1 && ! test -s output.out ' -- 1.9.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/RFC v4 3/3] git-am: add am.threeWay config variable
Add the am.threeWay configuration variable to use the -3 or --3way option of git am by default. When am.threeway is set and not desired for a specific git am command, the --no-3way option can be used to override it. Signed-off-by: Remi Lespinet --- Documentation/config.txt | 8 Documentation/git-am.txt | 7 +-- git-am.sh| 9 + t/t4150-am.sh| 19 +++ 4 files changed, 41 insertions(+), 2 deletions(-) diff --git a/Documentation/config.txt b/Documentation/config.txt index d44bc85..36b75d9 100644 --- a/Documentation/config.txt +++ b/Documentation/config.txt @@ -769,6 +769,14 @@ am.keepcr:: by giving '--no-keep-cr' from the command line. See linkgit:git-am[1], linkgit:git-mailsplit[1]. +am.threeWay:: + By default, `git am` will fail if the patch does not apply cleanly. When + set to true, this setting tells `git am` to fall back on 3-way merge if + the patch records the identity of blobs it is supposed to apply to and + we have those blobs available locally (equivalent to giving the `--3way` + option from the command line). Defaults to `false`. + See linkgit:git-am[1]. + apply.ignoreWhitespace:: When set to 'change', tells 'git apply' to ignore changes in whitespace, in the same way as the '--ignore-space-change' diff --git a/Documentation/git-am.txt b/Documentation/git-am.txt index 0d8ba48..d92b569 100644 --- a/Documentation/git-am.txt +++ b/Documentation/git-am.txt @@ -10,7 +10,7 @@ SYNOPSIS [verse] 'git am' [--signoff] [--keep] [--[no-]keep-cr] [--[no-]utf8] -[--3way] [--interactive] [--committer-date-is-author-date] +[--[no-]3way] [--interactive] [--committer-date-is-author-date] [--ignore-date] [--ignore-space-change | --ignore-whitespace] [--whitespace=] [-C] [-p] [--directory=] [--exclude=] [--include=] [--reject] [-q | --quiet] @@ -90,10 +90,13 @@ default. You can use `--no-utf8` to override this. -3:: --3way:: +--no-3way:: When the patch does not apply cleanly, fall back on 3-way merge if the patch records the identity of blobs it is supposed to apply to and we have those blobs - available locally. + available locally. `--no-3way` can be used to override + am.threeWay configuration variable. For more information, + see am.threeWay in git-config(1). --ignore-space-change:: --ignore-whitespace:: diff --git a/git-am.sh b/git-am.sh index c460dd0..75e701a 100755 --- a/git-am.sh +++ b/git-am.sh @@ -390,6 +390,11 @@ then keepcr=t fi +if test "$(git config --bool --get am.threeWay)" = true +then +threeway=t +fi + while test $# != 0 do case "$1" in @@ -401,6 +406,8 @@ it will be removed. Please do not use it anymore." ;; -3|--3way) threeway=t ;; + --no-3way) + threeway=f ;; -s|--signoff) sign=t ;; -u|--utf8) @@ -658,6 +665,8 @@ fi if test "$(cat "$dotest/threeway")" = t then threeway=t +else + threeway=f fi git_apply_opt=$(cat "$dotest/apply-opt") if test "$(cat "$dotest/sign")" = t diff --git a/t/t4150-am.sh b/t/t4150-am.sh index 6ced98c..b822a39 100755 --- a/t/t4150-am.sh +++ b/t/t4150-am.sh @@ -303,6 +303,25 @@ test_expect_success 'am -3 -p0 can read --no-prefix patch' ' git diff --exit-code lorem ' +test_expect_success 'am with config am.threeWay falls back to 3-way merge' ' + rm -fr .git/rebase-apply && + git reset --hard && + git checkout -b lorem4 base3way && + test_config am.threeWay 1 && + git am lorem-move.patch && + test_path_is_missing .git/rebase-apply && + git diff --exit-code lorem +' + +test_expect_success 'am with config am.threeWay overridden by --no-3way' ' + rm -fr .git/rebase-apply && + git reset --hard && + git checkout -b lorem5 base3way && + test_config am.threeWay 1 && + test_must_fail git am --no-3way lorem-move.patch && + test_path_is_dir .git/rebase-apply +' + test_expect_success 'am can rename a file' ' grep "^rename from" rename.patch && rm -fr .git/rebase-apply && -- 1.9.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 alerts
Hello, is there anywhere on the web that displays git's latest security alerts Sent from iPhone-- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH/RFC v3 2/4] git-am.txt: add configuration section in git am documentation
> Matthieu Moy writes > > > +CONFIGURATION > > > +- > > > + > > > +am.keepcr:: > > > +If true, git-am will call git-mailsplit for patches in mbox > > > format > > > > `git am` > > `git mailsplit` > > > > > +with parameter '--keep-cr'. In this case git-mailsplit will > > > > Likewise > > > > > +not remove `\r` from lines ending with `\r\n`. Can be overridden > > > +by giving '--no-keep-cr' from the command line. > > > > That should be backquote, not forward-quote, right? > > > > I know it's not your code since it's a cut-and-paste from config.txt, > > but that illustrates my point above: we used to have one place with > > wrong quotes, and we'd have two after the patch. > > Ok I'll correct it in a minor patch Actually I don't think that this is a good idea to correct that (since there's many occurences of forward-quoted options in git-config.txt). I'll just remove the configuration part in the git am documentation. -- To unsubscribe from this list: send the line "unsubscribe 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/WIP PATCH 11/11] Document protocol version 2
On Mon, Jun 01, 2015 at 04:40:54PM -0700, Stefan Beller wrote: > Thinking about this further, maybe it is a good idea to restrict the > capabilities advertising to alphabetical order? This seems like an unnecessary restriction. The main impetus seems to be: > So how does parse_capability scale w.r.t the number of capabilities? > If parse_capability is just a linear search then it is O(n) and with n > capabilities the client faces an O(n^2) computation which is bad. So > if we were to require alphabetic capabilities, you could internally > keep track and the whole operation is O(n). I just wonder if this is > premature optimization or some thought we need to think of. but that is an implementation problem, not a protocol problem. The parsing side can easily use something better than O(n) lookups (e.g., a binary search). I think we can live with O(n lg n) to parse the whole list. A true in-order merge would be O(n), but the difference is probably negligible here, because... > Now if we assume the number of capabilities grows over time a lot > (someone may "abuse" it for a cool feature, similar to the refs > currently. Nobody thought about having so many refs in advance) I think if it grows without bound, we are doing it wrong. We should generally only be adding capabilities that require a single short tag in the list (server supports "foo", client asks for "foo"). I'd find it acceptable to add ones that repeat, as long as we are very sure that the repetition is going to be small, or scales with the size of the config (e.g., servers says "here is a mirror you can seed from; here is another mirror you can seed from"). We should definitely _not_ add anything that scales with the repository size. For instance, the "symref" field only shows the "HEAD" now. That's OK, as it's constant size. We do not show _all_ symrefs right now because of pkt-line length limitations. With v2, we could in theory start doing so (one per pkt-line). But that does not make it a good idea. The right way to implement that is: 1. the server says "I can tell you about symrefs" 2. client says "OK, I understand how to parse your symref data" 3. for each ref in the advertisement, tack on "\0symref=..." (or whatever). The capability portion of the conversation remains constant-sized, and the O(# of refs) portion is part of the ref advertisement, which is inherently of that magnitude. -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] index-pack: fix truncation of off_t in comparison
On Thu, Jun 4, 2015 at 7:35 PM, Jeff King wrote:
> Commit c6458e6 (index-pack: kill union delta_base to save
> memory, 2015-04-18) refactored the comparison functions used
> in sorting and binary searching our delta list. The
> resulting code does something like:
>
> int cmp_offsets(off_t a, off_t b)
> {
> return a - b;
> }
>
> This works most of the time, but produces nonsensical
> results when the difference between the two offsets is
> larger than what can be stored in an "int".
Ugh.. thanks for fixing this. Too bad neither gcc, clang or coverity
spotted this.
--
Duy
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/WIP PATCH 00/11] Protocol version 2, again!
On Mon, Jun 01, 2015 at 10:49:45AM -0700, Stefan Beller wrote:
> However the client side with builtin/fetch, builtin/fetch-pack, fetch-pack
> is a bit of a mystery to me, as I cannot fully grasp the difference between
> * connect.{h,c}
> * remote.{h.c}
> * transport.{h.c}
> there. All of it seems to be doing network related stuff, but I have trouble
> getting the big picture. I am assuming all of these 3 are rather a low level,
> used like a library, though there must be even more hierarchy in there,
> connect is most low level judging from the header file and used by
> the other two.
> transport.h seems to provide the most toplevel library stuff as it includes
> remote.h in its header?
connect.c was originally "the git protocol", and was used by send-pack
and fetch-pack. Other individual programs implemented other transports.
Later, as the interface moved towards everybody running "fetch" and
"push", and those delegating work to the individual transports, we got
transport.c, which is an abstract interface for all transports. It
delegates actual git-protocol work to the functions in connect.c (or
bundle work elsewhere, or handles remote-helpers itself).
And then remote.c contains routines for dealing with the remotes at a
logical level. E.g., which refs to fetch or push, etc.
So in theory, the flow is something like:
- fetch.c knows "the user wants to fetch from 'foo'"
- fetch asks remote.c: "who is remote 'foo'"; we get back a URL
- fetch asks transport.c: "what are the refs for $URL"
- it turns out to be a git URL. transport.c calls into connect.c to
implement get_refs_via_connect.
- after fetch gets back the list of refs, it uses routines in remote.c
to figure out which refs we are interested in, handle refspecs, etc
- now fetch asks transport.c: "OK, fetch just these refs"
- transport.c again calls into connect.c to handle the actual fetch
Of course over the years a lot of cruft has grown around all of them. I
wouldn't be surprised if there are functions which cross these
abstractions, or other random functions inside each file that do not
belong.
> and the issue I am concerned about is the select_capabilities as well as
> the request_capabilities function here. The select_capabilities functionality
> is currently residing in the high level parts of the code as it both depends
> on
> the advertised server capabilities and on the user input (--use-frotz or
> config
> options), so the capability selection is done in fetchpack.c
>
> So there are 2 routes to go: Either we leave the select_capabilities in the
> upper layers (near the actual high level command, fetch, fetchpack) or we put
> it into the transport layer and just passing in a struct what the user
> desires.
> And when the users desire doesn't meet the server capabilities we die deep
> down
> in the transport layer.
I think you have to leave it in the fetch-pack code. As you note, it's
the place where we know about what the user is asking for and can
manipulate the list. And not all transports even support capabilities
like this.
-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
Permission denied ONLY after pulling bundles
Dear git group, I would like to ask your help for a problem that we cannot fix in any way. We have a git repository in folder on Windows. Then we use VMware player on CentOS_6 on which we create a clone of the git repository, after of course having mounted the directory in which the repository is. So the repository is on windows and the clone on Linux. We are able to perfom all the git operations we need, except for the pull .bundle, which is successful in itself but prevent us from pushing after that. As we try to push after pulling a .bundle in a branch we get the error message NODE1:fdp> git push Counting objects: 1977, done. Delta compression using up to 2 threads. Compressing objects: 100% (423/423), done. fatal: write error: Permission denied00 KiB | 158 KiB/s error: pack-objects died of signal 13 error: pack-objects died with strange error We have checked all the permissions, changed the users, recreated the clone but nothing worked. The push operation works perfectly until we pull a bundle. After pulling a bundle we are not able to push anymore.We tryed to delete the branches, recreate others and all works perfectly, also the push.As we pull the .bundle we cannot get the permission to do the push anymore. What has this to do with the bundle? Thanks for your support. -- Rossella -- To unsubscribe from this list: send the line "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] index-pack: fix truncation of off_t in comparison
Commit c6458e6 (index-pack: kill union delta_base to save
memory, 2015-04-18) refactored the comparison functions used
in sorting and binary searching our delta list. The
resulting code does something like:
int cmp_offsets(off_t a, off_t b)
{
return a - b;
}
This works most of the time, but produces nonsensical
results when the difference between the two offsets is
larger than what can be stored in an "int". This can lead to
unresolved deltas if the packsize is larger than 2G (even on
64-bit systems, an int is still typically 32 bits):
$ git clone git://github.com/mozilla/gecko-dev
Cloning into 'gecko-dev'...
remote: Counting objects: 4800161, done.
remote: Compressing objects: 100% (178/178), done.
remote: Total 4800161 (delta 88), reused 0 (delta 0), pack-reused 4799978
Receiving objects: 100% (4800161/4800161), 2.21 GiB | 3.26 MiB/s, done.
Resolving deltas: 99% (3808820/3811944), completed with 0 local objects.
fatal: pack has 3124 unresolved deltas
fatal: index-pack failed
We can fix it by doing direct comparisons between the
offsets and returning constants; the callers only care about
the sign of the comparison, not the magnitude.
Signed-off-by: Jeff King
---
On top of nd/slim-index-pack-memory-usage, which introduced the bug (but
it is already in master).
builtin/index-pack.c | 8 ++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/builtin/index-pack.c b/builtin/index-pack.c
index 3ed53e3..06dd973 100644
--- a/builtin/index-pack.c
+++ b/builtin/index-pack.c
@@ -616,7 +616,9 @@ static int compare_ofs_delta_bases(off_t offset1, off_t
offset2,
int cmp = type1 - type2;
if (cmp)
return cmp;
- return offset1 - offset2;
+ return offset1 < offset2 ? -1 :
+ offset1 > offset2 ? 1 :
+ 0;
}
static int find_ofs_delta(const off_t offset, enum object_type type)
@@ -1051,7 +1053,9 @@ static int compare_ofs_delta_entry(const void *a, const
void *b)
const struct ofs_delta_entry *delta_a = a;
const struct ofs_delta_entry *delta_b = b;
- return delta_a->offset - delta_b->offset;
+ return delta_a->offset < delta_b->offset ? -1 :
+ delta_a->offset > delta_b->offset ? 1 :
+ 0;
}
static int compare_ref_delta_entry(const void *a, const void *b)
--
2.4.2.745.g0aa058d
--
To unsubscribe from this list: send the line "unsubscribe 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/WIP 1/8] wrapper: implement xopen()
On Thu, May 28, 2015 at 3:03 AM, Torsten Bögershausen wrote:
> On 2015-05-27 15.33, Paul Tan wrote:
>> +/**
>> + * xopen() is the same as open(), but it die()s if the open() fails.
>> + */
>> +int xopen(const char *path, int flags, mode_t mode)
>> +{
>> + int fd;
>> +
>> + assert(path);
>> + fd = open(path, flags, mode);
>> + if (fd < 0) {
>> + if ((flags & O_WRONLY) || (flags & O_RDWR))
>> + die_errno(_("could not open '%s' for writing"), path);
> This is only partly true:
> it could be either "writing" or "read write".
Ah right, I see now that the POSIX spec allows for, and encourages
O_RDONLY | O_WRONLY == O_RDWR.
> I don't know if the info "for reading" or "for writing" is needed/helpful at
> all,
> or if a simple "could not open" would be enough.
Yeah, I agree that it may not be helpful, but I noticed that most
error messages in git are of the form "unable to open X for writing",
"unable to open X for reading", "could not create X" etc. Or rather I
thought I noticed, but it now seems to me that there are quite a lot
of uses of "could not open X" as well.
I guess I will remove the distinction.
Thanks,
Paul
--
To unsubscribe from this list: send the line "unsubscribe 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 mh/lockfile-retry] lockfile: replace random() by rand()
On 06/04/2015 10:40 AM, Johannes Sixt wrote: > Am 30.05.2015 um 19:12 schrieb Junio C Hamano: >> Johannes Sixt writes: >> >>> There you have it: Look the other way for a while, and people start >>> using exotic stuff... ;) >> >> Is it exotic to have random/srandom? Both are in POSIX and 4BSD; >> admittedly rand/srand are written down in C89 and later, so they >> might be more portable, but I recall the prevailing wisdom is to >> favor random over rand for quality of randomness and portability, so >> I am wondering if it may be a better approach to keep the code as-is >> and do a compat/random.c based on either rand/srand (or use posix >> sample implementation [*1*]). > > For our purposes here, the linear congruence of rand() is certainly > sufficient. At this time, compatibility functions for random/srandom > would just mean a lot of work for little gain. We *certainly* don't require high-quality random numbers for this application. Regarding portability, there is one definite point in favor of rand() (it's available on Windows) vs. Junio's recollection that random() might have portability advantages, presumably on other platforms. Maybe the easiest thing would be to switch to using rand() and see if the OS/2 and VMS users complain ;-) Michael -- Michael Haggerty mhag...@alum.mit.edu -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Suggestion: make git checkout safer
Ed Avis waniasset.com> writes: >Julio H. asked how I had learned to run 'git checkout .'. Sorry it was Torsten B. who asked that. But yes, I think it was just word of mouth. -- Ed Avis -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Suggestion: make git checkout safer
Kevin Daudt ikke.info> writes: >If people execute git checkout . as a habbit >without thinking, they will soon train to do git checkout -f . without >thinking, and then you still have the same problem. I don't quite agree; you only learn to use the -f flag if the plain command doesn't do what you want. With rm, I want to remove that file, dammit! The -f flag is often a necessity to stop the tool getting in my way. But when fixing up a working tree, I rarely want to silently trash any local changes. >I do share your sentiment that it's easy to loose uncomitted changes to >git checkout , but like Jeff said, the entire goal of this command >is to reset specific files from the index or commits. Well that's not quite the flavour given by the documentation, which says >Updates files in the working tree to match... 'Updating' files sounds like a fairly safe thing to do, right? Like 'cvs update' or 'svn update', which don't just overwrite working tree changes. The doc doesn't really make clear that any local changes will be discarded; indeed the only mention of that is -f, --force When switching branches... this is used to throw away local changes. To the casual reader, following 'the exception proves the rule', it appears that local changes are not thrown away except in this case. -- Ed Avis -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Suggestion: make git checkout safer
Stefan Beller google.com> writes: >So in one mode, we do actually warn about contents going missing, and the >other mode is designed to actually make things go missing without any >warning. I think this is a big part of the issue. Two rather different operations are given the name 'checkout', and the safety standards applied to them also differ greatly. The manual page doesn't make it clear that it can be quite a dangerous command to run, even without --force. >If I were to come up with a name for such an action it's >maybe "reset" or "reset-file(s)". Agreed. Or 'git clean' could become more powerful and able to reset file contents as well as deleting untracked files. The name and documentation of 'git clean' already make it clear that it's not something safe to run without thinking first. Julio H. asked how I had learned to run 'git checkout .'. I think it was just word of mouth. I had deleted some files from the working tree and asked a colleague how to restore them from the repository - which is, after all, a bread-and-butter operation for any version control system. What is the correct command to run, then, to safely restore missing files? And yes, it probably would be better to use git's native mechanisms to throw away local changes to a file, rather than the sledgehammer approach of just deleting it and checking it out again. Most of the time I do so. Sometimes when everything is a real mess it is more straighforward to reach for 'rm' - or indeed for the delete option in your IDE or file browser. -- Ed Avis -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[RFC] git-am: handling unborn branches
Hi, git-am generally supports applying patches to unborn branches. However, there are 2 cases where git-am does not handle unborn branches which I would like to address before the git-am rewrite to C: 1. am --skip For git am --skip, git-am.sh does a fast-forward checkout from HEAD to HEAD, discarding unmerged entries, and then resets the index to HEAD so that the index is not dirty. git read-tree --reset -u HEAD HEAD orig_head=$(cat "$GIT_DIR/ORIG_HEAD") git reset HEAD git update-ref ORIG_HEAD $orig_head This requires a valid HEAD. Since git-am requires an empty index for unborn branches in the patch application stage anyway, I think we should discard all entires in the index if we are on an unborn branch? Or, the current behavior of git-am.sh will print some scary errors about the missing HEAD, but will then continue on to the next patch. If the index is not clean, it will then error out. Should we preserve this behavior? (without the errors about the missing HEAD) 2. am --abort For git am --abort, git-am.sh does something similar. It does a fast-forward checkout from HEAD to ORIG_HEAD, discarding unmerged entries, and then resets the index to ORIG_HEAD so that local changes will be unstaged. if safe_to_abort then git read-tree --reset -u HEAD ORIG_HEAD git reset ORIG_HEAD fi rm -fr "$dotest" This, however, requires a valid HEAD and ORIG_HEAD. If we don't have a HEAD or ORIG_HEAD (because we were on an unborn branch when we started git am), what should we do? (Note: safe_to_abort returns true if we git am with no HEAD because $dotest/abort-safety will not exist) Should we discard all entires in the index as well? (Since we might think of the "original HEAD" as an empty tree?) Or, the current behavior of git-am.sh will print some scary errors about the missing HEAD and ORIG_HEAD, but will not touch the index at all, and still delete the rebase-apply directory. Should we preserve this behavior (without the errors)? Thanks, Paul -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Suggestion: make git checkout safer
On Wed, Jun 3, 2015 at 5:29 PM, Junio C Hamano wrote: [snip] > [Footnote] > > *1* In the context of this discussion, after screwing up the change > in hello.c, instead of expressing the wish to recover and to > start from scratch in two separate commands, i.e. > > rm hello.c && update-from-scm > > they will learn to use a single command that is designed for > that purpose, i.e. > > checkout-from-scm hello.c > > without the "rm" step, which _is_ an artificial workaround for > their other SCMs that do not update from the repository unless > they remove the files. Just to be clear, Subversion doesn't require you to remove the file to restore it (I'm sure most of you know that, but just in case others didn't). There is a one-step way to restore the file: svn revert hello.c Unfortunately, revert in the Git sense is about reverting commits, so there's a bit of friction between Subversion and Git's terminology. OTOH, once the team was educated how to think about it, "git checkout " has been pretty natural to use. -John -- To unsubscribe from this list: send the line "unsubscribe 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] log: diagnose empty HEAD more clearly
On Thu, Jun 04, 2015 at 09:31:04AM +0200, Stefan Näwe wrote:
> > + if (!rev->pending.nr && !opt->def)
> > + die("you do not have a commit yet on your branch");
>
> I am not a native english speaker but shouldn't this be:
>
> "you do not have a commit on your branch yet"
Both are fine, as is:
"you do not yet have a commit on your branch"
though I think yours is slightly more clear.
If you are wondering at the reason, "yet" is an adverb modifying "have".
So it may come before or after the verb. If we substitute a simpler verb
(that does not need a direct object "a commit") and drop the
prepositional phrase ("on your branch"), we can do either:
- you do not yet program
- you do not program yet
If we add back in the prepositional phrase (which is really acting as an
adverbial phrase, modifying the verb here), we can do it in either
order:
- you do not program yet in the office
- you do not program in the office yet
But the latter makes it more clear that the "yet" applies to "in the
office".
You can also add back in the object of the verb:
- you do not yet program computers
but you would not want:
- you do not program yet computers
because it splits the verb from its object.
-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 mh/lockfile-retry] lockfile: replace random() by rand()
Am 30.05.2015 um 19:12 schrieb Junio C Hamano:
Johannes Sixt writes:
There you have it: Look the other way for a while, and people start
using exotic stuff... ;)
Is it exotic to have random/srandom? Both are in POSIX and 4BSD;
admittedly rand/srand are written down in C89 and later, so they
might be more portable, but I recall the prevailing wisdom is to
favor random over rand for quality of randomness and portability, so
I am wondering if it may be a better approach to keep the code as-is
and do a compat/random.c based on either rand/srand (or use posix
sample implementation [*1*]).
For our purposes here, the linear congruence of rand() is certainly
sufficient. At this time, compatibility functions for random/srandom
would just mean a lot of work for little gain.
[Reference]
*1* http://pubs.opengroup.org/onlinepubs/9699919799/functions/rand.html
This is a build breakage of master on Windows. There are also a few
new test suite failures. On of them is in t1404#2, indicating that
a DF conflict takes a different error path. I haven't debugged, yet.
The lock file retry test fails, too. I'll report back as time permits.
lockfile.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/lockfile.c b/lockfile.c
index 5a93bc7..ee5cb01 100644
--- a/lockfile.c
+++ b/lockfile.c
@@ -191,7 +191,7 @@ static int lock_file_timeout(struct lock_file *lk, const
char *path,
return lock_file(lk, path, flags);
if (!random_initialized) {
- srandom((unsigned int)getpid());
+ srand((unsigned int)getpid());
random_initialized = 1;
}
@@ -218,7 +218,7 @@ static int lock_file_timeout(struct lock_file *lk, const
char *path,
backoff_ms = multiplier * INITIAL_BACKOFF_MS;
/* back off for between 0.75*backoff_ms and 1.25*backoff_ms */
- wait_us = (750 + random() % 500) * backoff_ms;
+ wait_us = (750 + rand() % 500) * backoff_ms;
sleep_microseconds(wait_us);
remaining_us -= wait_us;
-- Hannes
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] log: diagnose empty HEAD more clearly
On Wed, Jun 03, 2015 at 10:24:02AM -0700, Junio C Hamano wrote:
> Which is what led me to say "Why are we defaulting to HEAD before
> checking if it even exists? Isn't that the root cause of this
> confusion? What happens if we stopped doing it?"
>
> And I think the "diagnose after finding that pending is empty and we
> were given 'def'" approach almost gives us the equivalent, which is
> why I said "Okay" above.
>
> If we can make it possible to tell if that "def" was given by the
> user (i.e. --default parameter) or by the machinery (e.g. "git log"
> and friends), then we can remove "almost" from the above sentence.
My thought was that we would not stop dying (for compatibility), and
that if we keep dying, it is effectively the same as what I proposed
(perhaps slightly worse, in the sense that we do not diagnose "--default
foo" as thoroughly, but effectively the same in that basically nobody
uses "--default" in the first place).
But if you are OK to eventually stop dying, I think this line of
reasoning is OK.
> diff --git a/builtin/log.c b/builtin/log.c
> index 4c4e6be..3b568a1 100644
> --- a/builtin/log.c
> +++ b/builtin/log.c
> @@ -148,6 +148,9 @@ static void cmd_log_init_finish(int argc, const char
> **argv, const char *prefix,
> rev->diffopt.output_format |= DIFF_FORMAT_NO_OUTPUT;
> argc = setup_revisions(argc, argv, rev, opt);
>
> + if (!rev->pending.nr && !opt->def)
> + die("you do not have a commit yet on your branch");
Do we want to mention the name of the branch here? I guess it does not
really matter. Perhaps "the current branch" would be better than "your
branch", though. Maybe:
fatal: you do not have any commits yet on the current branch
This message hopefully goes away in the long run, but we'll have it for
a while.
> +static void default_to_head_if_exists(struct setup_revision_opt *opt)
> +{
> + unsigned char unused[20];
> +
> + if (resolve_ref_unsafe("HEAD", RESOLVE_REF_READING, unused, NULL))
> + opt->def = "HEAD";
> +}
This approach looks sane to me. Want to wrap it up with a commit
message and a test?
-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: [WIP PATCH 1/3] git bisect old/new
On Thu, Jun 4, 2015 at 9:59 AM, Antoine Delaite wrote: > From: Christian Couder > > When not looking for a regression during a bisect but for a fix or a > change in another given property, it can be confusing to use 'good' > and 'bad'. > > This patch introduce `git bisect new` and `git bisect old` as an > alternative to 'bad' and good': the commits which have the most > recent version of the property must be marked as `new` and the ones > with the older version as `old`. > > The output will be the first commit after the change in the property. > During a new/old bisect session you cannot use bad/good commands and > vice-versa. > > `git bisect replay` works fine for old/new bisect sessions. > > Some commands are still not available for old/new: > > * git bisect start [ [...]] is not possible: the >commits will be treated as bad and good. > * git rev-list --bisect does not treat the revs/bisect/new and >revs/bisect/old-SHA1 files. > * git bisect visualize seem to work partially: the tags are >displayed correctly but the tree is not limited to the bisect >section. > > Related discussions: > > - http://thread.gmane.org/gmane.comp.version-control.git/86063 > introduced bisect fix unfixed to find fix. > - http://thread.gmane.org/gmane.comp.version-control.git/182398 > discussion around bisect yes/no or old/new. > - http://thread.gmane.org/gmane.comp.version-control.git/199758 > last discussion and reviews > > Signed-off-by: Antoine Delaite > Signed-off-by: Louis Stuber > Signed-off-by: Valentin Duperray > Signed-off-by: Franck Jonas > Signed-off-by: Lucien Kong > Signed-off-by: Thomas Nguy > Signed-off-by: Huynh Khoi Nguyen Nguyen > > Signed-off-by: Matthieu Moy > --- > We took account of most of the "easy" reviews that were discuted two years > ago. > We hope we didn't missed any. > http://thread.gmane.org/gmane.comp.version-control.git/199758 > > We corrected various issues that were not reported: > -one that caused a "fatal ... not a valid ref" at the end of the bisection. > -the autostart was causing issues, the following lines were working : > git bisect new HEAD > git bisect bad HEAD > git bisect good aGoodCommit > > The hard review which we were thinking on was the issue of the maintaining > of old/new and allow easy support of new "tags" like yes/no in the future. > I tried to remove the maximum of bad/good and old/new which were hard wrote in > the code but I'm not completly satisfied. This patch is clearly a v1. Thanks for working on this. Here are some suggestions that I should probably have told you before, but didn't: - Take ownership of all the patches. - Patch 3/3 renames some variables in bisect.c, do the same thing in git-bisect.sh for consistency. - Squash all the patches together. - Try to find a way to break down the resulting patch into a logical patch series which adds tests at each logical step. This might be difficult. You might want to add features to git bisect--helper first for example, then test those features, then add features to git bisect and then test those features. Best, Christian. > We're currently working on: > > * rebasing the history of the patch > * git rev-list --bisect does not treat the revs/bisect/new and > revs/bisect/old-SHA1 files. > * git bisect visualize seem to work partially: the tags are displayed > correctly but the tree is not limited to the bisect section. > * adding tests about old/new > > Some other problems that might also be considerred later : > * change/add valid terms (e.g "unfixed/fixed" instead of "old/new") > * > * git bisect start [ [...]] is not possible: the commits > will be treated as bad and good. > -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Minor bug report
On Wed, Jun 03, 2015 at 05:39:14PM +0200, Dennis Kaarsemaker wrote: > On di, 2015-06-02 at 23:48 -0700, Junio C Hamano wrote: > > > > I am kind of surprised after reading these two threads that my > > take on this issue has changed over time, as my knee-jerk > > reaction before reading them was the opposite, something > > along the lines of "This is only immediately after 'git init'; why > > bother?". Or depending on my mood, that "How stupid do you > > have to be..." sounds exactly like a message I may advocate > > for us to send. Perhaps I grew more bitter with age. > > The "fatal: Failed to resolve 'HEAD' as a valid ref." message, closely > related to the "fatal: bad default revision 'HEAD'" message that started > this thread just came by in #git with the following situation: > > $ git init > $ git add . > # Oops, didn't want to add foo.xyz > $ git reset foo.xyz > fatal: Failed to resolve 'HEAD' as a valid ref. > > The solution there is simple, git rm --cached, but I think git could > produce more helpful messages when a repo is empty. Yeah, I think there are a lot of places we could handle unborn branches better. We've slowly been smoothing these rough edges over the years (usually by using the empty tree when we wanted HEAD to be a tree-ish). Patches welcome. :) -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: [WIP PATCH 1/3] git bisect old/new
Antoine Delaite writes:
> @@ -732,18 +736,25 @@ static void handle_bad_merge_base(void)
> if (is_expected_rev(current_bad_oid)) {
> char *bad_hex = oid_to_hex(current_bad_oid);
> char *good_hex = join_sha1_array_hex(&good_revs, ' ');
> -
> - fprintf(stderr, "The merge base %s is bad.\n"
> - "This means the bug has been fixed "
> - "between %s and [%s].\n",
> - bad_hex, bad_hex, good_hex);
> -
> + if (!strcmp(bisect_bad,"bad")) {
space after comma.
> @@ -954,6 +994,9 @@ int bisect_next_all(const char *prefix, int no_checkout)
> "(roughly %d step%s)\n", nr, (nr == 1 ? "" : "s"),
> steps, (steps == 1 ? "" : "s"));
>
> + free((char*)bisect_bad);
> + free((char*)bisect_good);
Already noted last time I think: these must not be freed as they are
pointing to static strings.
--
Matthieu Moy
http://www-verimag.imag.fr/~moy/
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 4/4] status: add new tests for status during rebase -i
I'd swap 3/4 and 4/4 so that we see the impact of your code on these new tests. I won't insist on that though. Please help reviewers by explaining in the commit message why these tests are needed (what was not covered properly by existing tests?) Guillaume Pagès writes: > +test_expect_success 'status: two commands done, two remainings' ' > + FAKE_LINES="1 exec_exit_15 2 3" && > + export FAKE_LINES && > + test_when_finished "git rebase --abort" && > + ONTO=$(git rev-parse --short HEAD~3) && > + COMMIT4=$(git rev-parse HEAD) && > + COMMIT3=$(git rev-parse HEAD^) && > + COMMIT2=$(git rev-parse HEAD^^) && > + (git rebase -i HEAD~3 || true)&& Space before && (same on second test) > + cat >expected
Re: [PATCH 3/4] status: give more information during rebase -i
Guillaume Pagès writes:
> + (use git rebase --edit-todo to view and edit)
You're still missing double-quotes around "git rebase --edit-todo".
Guillaume Pagès writes:
> +Last command(s) done (1 command(s) done):
Can't we just have "1 command"/"2 commands" instead of this (s). It's
particularly bad since it is already within parenthesis.
See the doc for Q_() in po/README and
http://www.gnu.org/savannah-checkouts/gnu/gettext/manual/html_node/Plural-forms.html
for more details.
> +void get_two_last_lines(char *filename, int *numlines, char **lines)
> +{
> + struct strbuf buf = STRBUF_INIT;
> + FILE *fp;
> +
> + *numlines = 0;
> + fp = fopen(git_path("%s", filename), "r");
> + if (!fp) {
> + strbuf_release(&buf);
> + return;
> + }
> + while (strbuf_getline(&buf, fp, '\n') != EOF) {
> + (*numlines)++;
> + lines[0] = lines[1];
> + lines[1] = strbuf_detach(&buf, NULL);
> + }
> + fclose(fp);
> +}
> +
> +void get_two_first_lines(char *filename, int *numlines, char **lines)
> +{
> + struct strbuf buf = STRBUF_INIT;;
> + char *line;
> + FILE *fp;
> +
> +
> + *numlines = 0;
> + fp = fopen(git_path("%s", filename), "r");
> + if (!fp) {
> + strbuf_release(&buf);
> + return;
> + }
> + while (strbuf_getline(&buf, fp, '\n') != EOF) {
> + stripspace(&buf, 1);
> + line = strbuf_detach(&buf, NULL);
> + if (strcmp(line, "") == 0)
> + continue;
> + if (*numlines < 2)
> + lines[*numlines] = line;
> + (*numlines)++;
> + }
> + fclose(fp);
> +}
> +
> +static void show_rebase_information(struct wt_status *s,
> + struct wt_status_state *state,
> + const char *color)
> +{
> + if (state->rebase_interactive_in_progress) {
> + int i, begin;
> + int numlines = 0;
> + char *lines[2];
> + get_two_last_lines("rebase-merge/done", &numlines, lines);
> + if (numlines == 0)
> + status_printf_ln(s,color,_("No command done."));
space after comma.
You may want to play with clang-format/clang-format-diff to have a tool
tell you this instead of asking a human.
I think english people would make this plural (No commands done).
> + else{
> + status_printf_ln(s,color,
> + _("Last command(s) done (%d command(s) done):"),
> + numlines);
Q_() ?
> + begin = numlines > 1? 0 : 1;
> + for (i = begin; i < 2; i++) {
> + status_printf_ln(s,color," %s",lines[i]);
> + }
> + if (numlines > 2 && s->hints)
> +status_printf_ln(s,color,
> + _(" (see more at .git/rebase-merge/done)"));
I'd write explicitly 'in file ...' instead of 'at'.
You need to use git_path here, it may not be ".git" in some contexts.
--
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
[PATCH 2/3] bisect: use name_(bad|good) instead of bisect_(bad|good)
From: Christian Couder
Signed-off-by: Antoine Delaite
---
bisect.c | 35 +--
1 file changed, 17 insertions(+), 18 deletions(-)
diff --git a/bisect.c b/bisect.c
index d6c19fd..68417bb 100644
--- a/bisect.c
+++ b/bisect.c
@@ -21,8 +21,8 @@ static const char *argv_checkout[] = {"checkout", "-q", NULL,
"--", NULL};
static const char *argv_show_branch[] = {"show-branch", NULL, NULL};
static const char *argv_update_ref[] = {"update-ref", "--no-deref",
"BISECT_HEAD", NULL, NULL};
-static const char *bisect_bad;
-static const char *bisect_good;
+static const char *name_bad;
+static const char *name_good;
/* Remember to update object flag allocation in object.h */
#define COUNTED(1u<<16)
@@ -406,7 +406,7 @@ struct commit_list *find_bisection(struct commit_list *list,
static int register_ref(const char *refname, const unsigned char *sha1,
int flags, void *cb_data)
{
- if (!strcmp(refname, bisect_bad)) {
+ if (!strcmp(refname, name_bad)) {
current_bad_oid = xmalloc(sizeof(*current_bad_oid));
hashcpy(current_bad_oid->hash, sha1);
} else if (starts_with(refname, "good-") ||
@@ -638,7 +638,7 @@ static void exit_if_skipped_commits(struct commit_list
*tried,
return;
printf("There are only 'skip'ped commits left to test.\n"
- "The first %s commit could be any of:\n", bisect_bad);
+ "The first %s commit could be any of:\n", name_bad);
print_commit_list(tried, "%s\n", "%s\n");
if (bad)
printf("%s\n", oid_to_hex(bad));
@@ -736,7 +736,7 @@ static void handle_bad_merge_base(void)
if (is_expected_rev(current_bad_oid)) {
char *bad_hex = oid_to_hex(current_bad_oid);
char *good_hex = join_sha1_array_hex(&good_revs, ' ');
- if (!strcmp(bisect_bad,"bad")) {
+ if (!strcmp(name_bad,"bad")) {
fprintf(stderr, "The merge base %s is bad.\n"
"This means the bug has been fixed "
"between %s and [%s].\n",
@@ -753,8 +753,7 @@ static void handle_bad_merge_base(void)
fprintf(stderr, "Some %s revs are not ancestor of the %s rev.\n"
"git bisect cannot work properly in this case.\n"
"Maybe you mistook %s and %s revs?\n",
- bisect_good, bisect_bad, bisect_good,
- bisect_bad);
+ name_good, name_bad, name_good, name_bad);
exit(1);
}
@@ -769,7 +768,7 @@ static void handle_skipped_merge_base(const unsigned char
*mb)
"So we cannot be sure the first %s commit is "
"between %s and %s.\n"
"We continue anyway.",
- bad_hex, good_hex, bisect_bad, mb_hex, bad_hex);
+ bad_hex, good_hex, name_bad, mb_hex, bad_hex);
free(good_hex);
}
@@ -850,7 +849,7 @@ static void check_good_are_ancestors_of_bad(const char
*prefix, int no_checkout)
int fd;
if (!current_bad_oid)
- die("a %s revision is needed", bisect_bad);
+ die("a %s revision is needed", name_bad);
/* Check if file BISECT_ANCESTORS_OK exists. */
if (!stat(filename, &st) && S_ISREG(st.st_mode))
@@ -913,13 +912,13 @@ void read_bisect_terms(void)
FILE *fp = fopen(filename, "r");
if (!fp) {
- bisect_bad = "bad";
- bisect_good = "good";
+ name_bad = "bad";
+ name_good = "good";
} else {
strbuf_getline(&str, fp, '\n');
- bisect_bad = strbuf_detach(&str, NULL);
+ name_bad = strbuf_detach(&str, NULL);
strbuf_getline(&str, fp, '\n');
- bisect_good = strbuf_detach(&str, NULL);
+ name_good = strbuf_detach(&str, NULL);
}
strbuf_release(&str);
fclose(fp);
@@ -965,8 +964,8 @@ int bisect_next_all(const char *prefix, int no_checkout)
printf("%s was both %s and %s\n",
oid_to_hex(current_bad_oid),
- bisect_good,
- bisect_bad);
+ name_good,
+ name_bad);
exit(1);
}
@@ -982,7 +981,7 @@ int bisect_next_all(const char *prefix, int no_checkout)
if (!hashcmp(bisect_rev, current_bad_oid->hash)) {
exit_if_skipped_commits(tried, current_bad_oid);
printf("%s is the first %s commit\n", bisect_rev_hex,
- bisect_bad);
+ name_bad);
show_diff_tree(prefix, revs.commits->item);
/* This means the bisection process succeeded. */
exit(10);
@@ -994,8 +993,8 @@ int bisect_next_all(const char *prefix, int no_checkout)
"(roughly %d step%s)\n", nr, (nr == 1 ? "" : "s"),
steps, (steps == 1
[WIP PATCH 1/3] git bisect old/new
From: Christian Couder When not looking for a regression during a bisect but for a fix or a change in another given property, it can be confusing to use 'good' and 'bad'. This patch introduce `git bisect new` and `git bisect old` as an alternative to 'bad' and good': the commits which have the most recent version of the property must be marked as `new` and the ones with the older version as `old`. The output will be the first commit after the change in the property. During a new/old bisect session you cannot use bad/good commands and vice-versa. `git bisect replay` works fine for old/new bisect sessions. Some commands are still not available for old/new: * git bisect start [ [...]] is not possible: the commits will be treated as bad and good. * git rev-list --bisect does not treat the revs/bisect/new and revs/bisect/old-SHA1 files. * git bisect visualize seem to work partially: the tags are displayed correctly but the tree is not limited to the bisect section. Related discussions: - http://thread.gmane.org/gmane.comp.version-control.git/86063 introduced bisect fix unfixed to find fix. - http://thread.gmane.org/gmane.comp.version-control.git/182398 discussion around bisect yes/no or old/new. - http://thread.gmane.org/gmane.comp.version-control.git/199758 last discussion and reviews Signed-off-by: Antoine Delaite Signed-off-by: Louis Stuber Signed-off-by: Valentin Duperray Signed-off-by: Franck Jonas Signed-off-by: Lucien Kong Signed-off-by: Thomas Nguy Signed-off-by: Huynh Khoi Nguyen Nguyen Signed-off-by: Matthieu Moy --- We took account of most of the "easy" reviews that were discuted two years ago. We hope we didn't missed any. http://thread.gmane.org/gmane.comp.version-control.git/199758 We corrected various issues that were not reported: -one that caused a "fatal ... not a valid ref" at the end of the bisection. -the autostart was causing issues, the following lines were working : git bisect new HEAD git bisect bad HEAD git bisect good aGoodCommit The hard review which we were thinking on was the issue of the maintaining of old/new and allow easy support of new "tags" like yes/no in the future. I tried to remove the maximum of bad/good and old/new which were hard wrote in the code but I'm not completly satisfied. This patch is clearly a v1. We're currently working on: * rebasing the history of the patch * git rev-list --bisect does not treat the revs/bisect/new and revs/bisect/old-SHA1 files. * git bisect visualize seem to work partially: the tags are displayed correctly but the tree is not limited to the bisect section. * adding tests about old/new Some other problems that might also be considerred later : * change/add valid terms (e.g "unfixed/fixed" instead of "old/new") * * git bisect start [ [...]] is not possible: the commits will be treated as bad and good. Documentation/git-bisect.txt | 43 +- bisect.c | 83 --- git-bisect.sh| 132 --- t/t6030-bisect-porcelain.sh | 40 + 4 files changed, 243 insertions(+), 55 deletions(-) mode change 100755 => 100644 git-bisect.sh diff --git a/Documentation/git-bisect.txt b/Documentation/git-bisect.txt index 4cb52a7..12c7711 100644 --- a/Documentation/git-bisect.txt +++ b/Documentation/git-bisect.txt @@ -18,8 +18,8 @@ on the subcommand: git bisect help git bisect start [--no-checkout] [ [...]] [--] [...] - git bisect bad [] - git bisect good [...] + git bisect (bad|new) [] + git bisect (good|old) [...] git bisect skip [(|)...] git bisect reset [] git bisect visualize @@ -104,6 +104,33 @@ For example, `git bisect reset HEAD` will leave you on the current bisection commit and avoid switching commits at all, while `git bisect reset bisect/bad` will check out the first bad revision. + +Alternative terms: bisect new and bisect old + + +If you are not at ease with the terms "bad" and "good", perhaps +because you are looking for the commit that introduced a fix, you can +alternatively use "new" and "old" instead. +But note that you cannot mix "bad" and good" with "new" and "old". + + +git bisect new [] + + +Marks the commit as new, which means the version is fixed. +It is the equivalent of "git bisect bad []". + + +git bisect old [...] + + +Marks the commit as old, which means the bug is present. +It is the equivalent of "git bisect good [...]". + +You must run `git bisect start` without commits as argument and run +`git bisect new `/`git bisect old ...` after to add the +commits. + Bisect visualize @@ -379,6 +406,18 @@ In this cas
[PATCH 3/3] bisect: fix indentation
From: Christian Couder
Signed-off-by: Antoine Delaite
---
bisect.c | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/bisect.c b/bisect.c
index 68417bb..87a5f6d 100644
--- a/bisect.c
+++ b/bisect.c
@@ -915,10 +915,10 @@ void read_bisect_terms(void)
name_bad = "bad";
name_good = "good";
} else {
- strbuf_getline(&str, fp, '\n');
- name_bad = strbuf_detach(&str, NULL);
- strbuf_getline(&str, fp, '\n');
- name_good = strbuf_detach(&str, NULL);
+ strbuf_getline(&str, fp, '\n');
+ name_bad = strbuf_detach(&str, NULL);
+ strbuf_getline(&str, fp, '\n');
+ name_good = strbuf_detach(&str, NULL);
}
strbuf_release(&str);
fclose(fp);
--
2.4.1.173.gd47f443.dirty
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [GSoC] Week 1: Unification of 'for-each-ref', 'tag -l' and 'branch -l'
On 06/04/2015 02:38 AM, Jeff King wrote: On Wed, Jun 03, 2015 at 06:08:50PM +0200, Matthieu Moy wrote: > Karthik Nayak writes: > >> Matthieu Moy suggested that I work on the unification of these >> commands let both the implementations stay where the user can select >> the implementation to be used > > Just to be clear: my advice is the above with "user" = "caller of the > API", not "human being using Git". In other words, "git branch > --contains" and "git tag --contains" would still be using two different > algorithms, but the user wouldn't notice (except for performance). Yeah, I think that is sensible. It should be a "feature" of the ref-filter that can hopefully go away one day (when we have a sensible implementation that works for both; this is something I've been meaning to push forward, but Karthik should not have to wait on me). -Peff Sounds good, Thanks Jeff. -- Regards, Karthik -- To unsubscribe from this list: send the line "unsubscribe 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] log: diagnose empty HEAD more clearly
Am 03.06.2015 um 19:24 schrieb Junio C Hamano:
> Jeff King writes:
>
>> My concern there would be risk of regression. I.e., that we would take
>> some case which used to error out and turn it into a silent noop. So I'd
>> prefer to keep the behavior the same, and just modify the error code
>> path. The end result is pretty similar to the user, because we obviously
>> cannot produce any actual output either way.
>
> Okay.
>
>> Something like:
>>
>> -- >8 --
>> Subject: log: diagnose empty HEAD more clearly
>>
>> If you init or clone an empty repository, the initial
>> message from running "git log" is not very friendly:
>>
>> $ git init
>> Initialized empty Git repository in /home/peff/foo/.git/
>> $ git log
>> fatal: bad default revision 'HEAD'
>>
>> Let's detect this situation and write a more friendly
>> message:
>>
>> $ git log
>> fatal: default revision 'HEAD' points to an unborn branch 'master'
>>
>> We also detect the case that 'HEAD' points to a broken ref;
>> this should be even less common, but is easy to see. Note
>> that we do not diagnose all possible cases. We rely on
>> resolve_ref, which means we do not get information about
>> complex cases. E.g., "--default master" would use dwim_ref
>> to find "refs/heads/master", but we notice only that
>> "master" does not exist. Similarly, a complex sha1
>> expression like "--default HEAD^2" will not resolve as a
>> ref.
>>
>> But that's OK. We fall back to the existing error message in
>> those cases, and they are unlikely to be used anyway.
>> Catching an empty or broken "HEAD" improves the common case,
>> and the other cases are not regressed.
>>
>> Signed-off-by: Jeff King
>> ---
>> I'm not a big fan of the new error message, either, just because the
>> term "unborn" is also likely to be confusing to new users.
>>
>> We can also probably get rid of mentioning "HEAD" completely. It is
>> always the default unless the user has overridden it explicitly.
>
> I think that still mentioning "HEAD" goes directly against the
> reasoning you made in an earlier part of your message:
>
>> I think it is one of those cases where the message makes sense when you
>> know how git works. But a new user who does not even know what "HEAD" or
>> a ref actually is may find it a bit confusing. Saying "we can't run
>> git-log, your repository empty!" may seem redundantly obvious even to
>> such a new user. But saying "bad revision 'HEAD'" may leave them saying
>> "eh, what is HEAD and why is it bad?".
>
> and I agree with that reasoning: the user didn't say anything about
> "HEAD", so why complain about it?
>
> Which is what led me to say "Why are we defaulting to HEAD before
> checking if it even exists? Isn't that the root cause of this
> confusion? What happens if we stopped doing it?"
>
> And I think the "diagnose after finding that pending is empty and we
> were given 'def'" approach almost gives us the equivalent, which is
> why I said "Okay" above.
>
> If we can make it possible to tell if that "def" was given by the
> user (i.e. --default parameter) or by the machinery (e.g. "git log"
> and friends), then we can remove "almost" from the above sentence.
>
>> So we
>> could go with something like:
>>
>> fatal: your default branch 'master' does not contain any commits
>>
>> or something. I dunno. Bikeshed colors welcome. Adding:
>>
>> advise("try running 'git commit'");
>>
>> is probably too pedantic. ;)
>
> ;-)
>
> I am wondering if the attached is better, if only because it is of
> less impact. I have die() there to avoid the behaviour change, but
> if we are going to have another future version where we are willing
> to take incompatiblity for better end-user experience like we did at
> 2.0, I suspect turning it to warning() or even removing it might be
> a candidate for such a version. If you have one commit and ask
> "log", you get one commit back. If you have no commit and ask "log",
> I think it is OK to say that you should get nothing back without
> fuss.
>
> builtin/log.c | 19 +++
> 1 file changed, 15 insertions(+), 4 deletions(-)
>
> diff --git a/builtin/log.c b/builtin/log.c
> index 4c4e6be..3b568a1 100644
> --- a/builtin/log.c
> +++ b/builtin/log.c
> @@ -148,6 +148,9 @@ static void cmd_log_init_finish(int argc, const char
> **argv, const char *prefix,
> rev->diffopt.output_format |= DIFF_FORMAT_NO_OUTPUT;
> argc = setup_revisions(argc, argv, rev, opt);
>
> + if (!rev->pending.nr && !opt->def)
> + die("you do not have a commit yet on your branch");
I am not a native english speaker but shouldn't this be:
"you do not have a commit on your branch yet"
??
> [...]
Stefan
--
/dev/random says: I bet you I could stop gambling.
python -c "print
'73746566616e2e6e616577654061746c61732d656c656b74726f6e696b2e636f6d'.decode('hex')"
GPG Key fingerprint = 2DF5 E01B 09C3 7501 BCA9 9666 829B 49C5 9221 27AF
