Re: implement a stable 'Last updated' in Documentation
On Fri, Jan 30, Jeff King wrote: I have 8.6.9-3 installed (it is part of Debian testing/unstable now), and confirmed that: diff --git a/Documentation/asciidoc.conf b/Documentation/asciidoc.conf index 2c16c53..10c777e 100644 --- a/Documentation/asciidoc.conf +++ b/Documentation/asciidoc.conf @@ -21,6 +21,7 @@ tilde=#126; apostrophe=#39; backtick=#96; litdd=#45;#45; +footer-style=none ifdef::backend-docbook[] [linkgit-inlinemacro] drops the last-updated footer. This does not remove Last updated from files like using-merge-subtree.html for me, using asciidoc-8.6.8. Olaf -- To unsubscribe from this list: send the line 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] contrib/completion: deprecate __git_remotes in bash completion
Bash auto completion supplies a function __git_remotes which
lists the git remotes of a repository by reading the
.git/remotes directory. As of git 1.7.6 this is handled
natively by the `git remote` command. This function is
now deprecated.
Signed-off-by: Matt Korostoff mkorost...@gmail.com
---
*
Great point Gabor! I would hesitate a little about removing the function
entirely, because users may have external scripts that rely on this function,
but certainly for our internal purposes the __git_remotes shell function can be
replaced with the native. Here's a short screen cast of the included patch
working on my local http://i.imgur.com/6ZSMXCO.gif
*
contrib/completion/git-completion.bash | 30 +++---
1 file changed, 15 insertions(+), 15 deletions(-)
diff --git a/contrib/completion/git-completion.bash
b/contrib/completion/git-completion.bash
index 2fece98..8b41871 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -409,14 +409,14 @@ __git_refs_remotes ()
done
}
+# Deprecated wrapper function around git remote. Supplied for legacy purposes,
+# to avoid breaking any external scripts which rely on this function.
+# Originally this function was used to programmatically generate a list of git
+# remotes by reading the .git/remotes directory, but as of git 1.7.6 that is
+# natively handled by the git remote command
__git_remotes ()
{
- local i IFS=$'\n' d=$(__gitdir)
- test -d $d/remotes ls -1 $d/remotes
- for i in $(git --git-dir=$d config --get-regexp 'remote\..*\.url'
2/dev/null); do
- i=${i#remote.}
- echo ${i/.url*/}
- done
+ git remote
}
__git_list_merge_strategies ()
@@ -558,7 +558,7 @@ __git_complete_remote_or_refspec ()
((c++))
done
if [ -z $remote ]; then
- __gitcomp_nl $(__git_remotes)
+ __gitcomp_nl $(git remote)
return
fi
if [ $no_complete_refspec = 1 ]; then
@@ -927,7 +927,7 @@ _git_archive ()
return
;;
--remote=*)
- __gitcomp_nl $(__git_remotes) ${cur##--remote=}
+ __gitcomp_nl $(git remote) ${cur##--remote=}
return
;;
--*)
@@ -1397,7 +1397,7 @@ _git_ls_files ()
_git_ls_remote ()
{
- __gitcomp_nl $(__git_remotes)
+ __gitcomp_nl $(git remote)
}
_git_ls_tree ()
@@ -1639,7 +1639,7 @@ _git_push ()
{
case $prev in
--repo)
- __gitcomp_nl $(__git_remotes)
+ __gitcomp_nl $(git remote)
return
;;
--recurse-submodules)
@@ -1649,7 +1649,7 @@ _git_push ()
esac
case $cur in
--repo=*)
- __gitcomp_nl $(__git_remotes) ${cur##--repo=}
+ __gitcomp_nl $(git remote) ${cur##--repo=}
return
;;
--recurse-submodules=*)
@@ -1797,7 +1797,7 @@ _git_config ()
{
case $prev in
branch.*.remote|branch.*.pushremote)
- __gitcomp_nl $(__git_remotes)
+ __gitcomp_nl $(git remote)
return
;;
branch.*.merge)
@@ -1809,7 +1809,7 @@ _git_config ()
return
;;
remote.pushdefault)
- __gitcomp_nl $(__git_remotes)
+ __gitcomp_nl $(git remote)
return
;;
remote.*.fetch)
@@ -1944,7 +1944,7 @@ _git_config ()
;;
remote.*)
local pfx=${cur%.*}. cur_=${cur#*.}
- __gitcomp_nl $(__git_remotes) $pfx $cur_ .
+ __gitcomp_nl $(git remote) $pfx $cur_ .
__gitcomp_nl_append pushdefault $pfx $cur_
return
;;
@@ -2250,7 +2250,7 @@ _git_remote ()
case $subcommand in
rename|remove|set-url|show|prune)
- __gitcomp_nl $(__git_remotes)
+ __gitcomp_nl $(git remote)
;;
set-head|set-branches)
__git_complete_remote_or_refspec
--
1.7.10.2 (Apple Git-33)
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: git-p4 is not cloning perforce code properly
When you say skipping, can you be more specific please? What command line are you using? If I want to clone the P4 tree at the current revision, I do something like: $ git p4 clone //depot/sometree/... That gets me just a single revision. If I want all revisions back to the start of time, I do: $ git p4 clone //depot/sometree/...@all Thanks, Luke On 10 February 2015 at 13:54, Sandeep varshney...@gmail.com wrote: Dear All, I am trying to clone perforce branch from git to my local drive, but it's skipping too many files and change list while fetching it from perforce. it'll be very helpful if anyone can suggest me about how to git rid with this issue. Thanks in Advance, Sandeep -- View this message in context: http://git.661346.n2.nabble.com/git-p4-is-not-cloning-perforce-code-properly-tp7625215.html Sent from the git mailing list archive at Nabble.com. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: EOL handling (EGit/svn/Windows)
On 2015-02-10 11.52, Piotr Krukowiecki wrote: On Tue, Feb 10, 2015 at 6:49 AM, Torsten Bögershausen tbo...@web.de wrote: Which Git versions are you using ? The one I'm testing currently: git version 1.7.9 (cygwin) git version 1.9.0.msysgit.0 (msys) EGit from Eclipse Luna Cygwin git is a bit old, as I see now. Will try to update later. How many people are there involved, how many on Windows, how many on Linux ? Less than 10 actively, most on Windows. Do you want to commit to svn, or is this a one-time conversion ? One-time. If it is a one-time conversion, and you continue to work in Git only, then the cleanest, most portable and future proof way is to use the .gitattributes file, I'm not sure if EGit supports .gitattributes: https://bugs.eclipse.org/bugs/show_bug.cgi?id=342372 add that to the repo, do the normalization and push. A line like this: * text=auto is the easiest way. I'm trying it. Have a look at https://www.kernel.org/pub/software/scm/git/docs/gitattributes.html take a tee or coffee, do some experiments first with a dummy repo, but all the client OS/Gits involved. That's one step we did not do carefully enough :( Please let us know the result (or feel free to ask more questions) For testing, I've converted all files to LF and commited it, also added the .gitattribute file. So far: 1. msysgit can't checkout a one file (saying filename too long, the relative path has 215 bytes) - probably not related to EOL issue. Please have a look here: https://github.com/msysgit/msysgit/releases/Git-1.9.5-preview20141217 I think we have support for long path names (Haven't tested it myself) Cygwin git works ok. So I did not check how msysgit works yet. 2. maybe due to old cygwin git, I have a problem of not displaying changes, if the changed line has LF eol (and the file was checked out on Windows with CRLF eols). Will try later with newer git. Normally you will not see any changes, and git diff will not show anything either. 2a. EGit handles such files gracefuly, but OTOH if the file is simple dos2unix'ed, it shows diffs showing all lines changed, and when you commit the files, it will create empty commit. Why this dos2unix ? Is there a special reason ? By the way, when people only use Egit, I assume they use Eclipse, and you don't use Notepad.exe or so at all. Then you don't need CRLF in the worktree at all, as Eclipse handle LF well. and in this case you should be able to set git config core.autocrlf input on all repos, just in case someone sneaks in a CRLF somewhere. (And after the normalizing of course) https://www.kernel.org/pub/software/scm/git/docs/git-config.html (and don't ask me if Egit supports that) $ git status # On branch master # nothing to commit (working directory clean) $ file master/settings.gradle master/settings.gradle: ASCII text, with CRLF line terminators That is under msysgit ? (Side note: Msysgit is called Git for Windows these days) $ dos2unix.exe master/settings.gradle Is this under Msysgit ? dos2unix: converting file master/settings.gradle to Unix format ... $ git status # On branch master # # Changes not staged for commit: # (use git add file... to update what will be committed) # (use git checkout -- file... to discard changes in working directory) # # modified: master/settings.gradle # no changes added to commit (use git add and/or git commit -a) $ git diff fatal: LF would be replaced by CRLF in master/settings.gradle That's interesting. What does git config -l | grep core give ? -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Gmail Message rejection
Stefan Beller stefanbel...@gmail.com writes: If you want email to send via gmail, you can do so by enabling text only mode for sending mails. Unfortunately, it is also a known bug that the Android Gmail client doesn't have this option (or didn't last time I checked). -- 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: EOL handling (EGit/svn/Windows)
On Tue, Feb 10, 2015 at 6:49 AM, Torsten Bögershausen tbo...@web.de wrote: Which Git versions are you using ? The one I'm testing currently: git version 1.7.9 (cygwin) git version 1.9.0.msysgit.0 (msys) EGit from Eclipse Luna Cygwin git is a bit old, as I see now. Will try to update later. How many people are there involved, how many on Windows, how many on Linux ? Less than 10 actively, most on Windows. Do you want to commit to svn, or is this a one-time conversion ? One-time. If it is a one-time conversion, and you continue to work in Git only, then the cleanest, most portable and future proof way is to use the .gitattributes file, I'm not sure if EGit supports .gitattributes: https://bugs.eclipse.org/bugs/show_bug.cgi?id=342372 add that to the repo, do the normalization and push. A line like this: * text=auto is the easiest way. I'm trying it. Have a look at https://www.kernel.org/pub/software/scm/git/docs/gitattributes.html take a tee or coffee, do some experiments first with a dummy repo, but all the client OS/Gits involved. That's one step we did not do carefully enough :( Please let us know the result (or feel free to ask more questions) For testing, I've converted all files to LF and commited it, also added the .gitattribute file. So far: 1. msysgit can't checkout a one file (saying filename too long, the relative path has 215 bytes) - probably not related to EOL issue. Cygwin git works ok. So I did not check how msysgit works yet. 2. maybe due to old cygwin git, I have a problem of not displaying changes, if the changed line has LF eol (and the file was checked out on Windows with CRLF eols). Will try later with newer git. 2a. EGit handles such files gracefuly, but OTOH if the file is simple dos2unix'ed, it shows diffs showing all lines changed, and when you commit the files, it will create empty commit. $ git status # On branch master # nothing to commit (working directory clean) $ file master/settings.gradle master/settings.gradle: ASCII text, with CRLF line terminators $ dos2unix.exe master/settings.gradle dos2unix: converting file master/settings.gradle to Unix format ... $ git status # On branch master # # Changes not staged for commit: # (use git add file... to update what will be committed) # (use git checkout -- file... to discard changes in working directory) # # modified: master/settings.gradle # no changes added to commit (use git add and/or git commit -a) $ git diff fatal: LF would be replaced by CRLF in master/settings.gradle $ echo hi master/settings.gradle $ file master/settings.gradle master/settings.gradle: ASCII text ### diff does not show changes! ### $ git diff fatal: LF would be replaced by CRLF in master/settings.gradle $ git diff -- master/settings.gradle fatal: LF would be replaced by CRLF in master/settings.gradle $ cat master/settings.gradle [the changes are there] $ unix2dos.exe master/settings.gradle unix2dos: converting file master/settings.gradle to DOS format ... $ git diff diff --git a/master/settings.gradle b/master/settings.gradle index a8d6609..7aa9e6b 100755 [changes are shown] $ vim -b master/settings.gradle [remove CR from the changed line] $ git status # On branch master # # Changes not staged for commit: # (use git add file... to update what will be committed) # (use git checkout -- file... to discard changes in working directory) # # modified: master/settings.gradle # no changes added to commit (use git add and/or git commit -a) $ git diff fatal: LF would be replaced by CRLF in master/settings.gradle -- Piotr Krukowiecki -- To unsubscribe from this list: send the line unsubscribe 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: reduce memory footprint a bit
On Mon, Feb 09, 2015 at 11:27:21AM -0800, Junio C Hamano wrote:
On a 3.4M object repo that's about 53MB. The saving is less impressive
compared to index-pack total memory use (about 400MB before delta
resolving, so the saving is just 13%)
Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
I'm not sure if this patch is worth pursuing. It makes the code a
little bit harder to read. I was just wondering how much memory could
be saved..
(text reordered)
I do not find the result all that harder to read. I however think
that the change would make it a lot harder to maintain, especially
because the name object-entry-extra does not have any direct link
to show-stat to hint us that this must be allocated when show-stat
is in use and must never be looked at when show-stat is not in use.
Noted. To be fixed.
I would say 13% is already impressive ;-).
The second patch makes the total saving 119MB, close to 30% (again on
x86-64, 32-bit platform number may be different). If we only compare
with the size of objects[] and deltas[], the saving percentage is 37%
(only for clone case) for this repo. Now it looks impressive to me :-D
The patch is larger than the previous one, but not really complex. And
the final index-pack.c is not hard to read either, probably becase we
already handle ofs-delta and ref-delta separately.
-- 8 --
Subject: [PATCH 2/2] index-pack: kill union delta_base to save memory
Once we know the number of objects in the input pack, we allocate an
array of nr_objects of struct delta_entry. On x86-64, this struct is
32 bytes long. The union delta_base, which is part of struct
delta_entry, provides enough space to store either ofs-delta (8 bytes)
or ref-delta (20 bytes).
Notice that with recent Git versions, ofs-delta objects are
preferred over ref-delta objects and ref-delta objects have no reason
to be present in a clone pack. So in clone case we waste
(20-8) * nr_objects bytes because of this union. That's about 38MB out
of 100MB for deltas[] with 3.4M objects, or 38%. deltas[] would be
around 62MB without the waste.
This patch attempts to eliminate that. deltas[] array is split into
two: one for ofs-delta and one for ref-delta. Many functions are also
duplicated because of this split. With this patch, ofs_delta_entry[]
array takes 38MB. ref_deltas[] should remain unallocated in clone case
(0 bytes). This array grows as we see ref-delta. We save more than
half in clone case, or 25% of total book keeping.
The saving is more than the calculation above because padding is
removed by __attribute__((packed)) on ofs_delta_entry. This attribute
should be ok to use, as we used to have it in our code base for some
time. The last use was removed because it may lead to incorrect
behavior when the struct is not packed, which is not the case in
index-pack.
A note about ofs_deltas allocation. We could use ref_deltas memory
allocation strategy for ofs_deltas. But that probably just adds more
overhead on top. ofs-deltas are generally the majority (1/2 to 2/3) in
any pack. Incremental realloc may lead to too many memcpy. And if we
preallocate, say 1/2 or 2/3 of nr_objects initially, the growth rate
of ALLOC_GROW() could make this array larger than nr_objects, wasting
more memory.
Brought-up-by: Matthew Sporleder msporle...@gmail.com
Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
builtin/index-pack.c | 260 +++
1 file changed, 160 insertions(+), 100 deletions(-)
diff --git a/builtin/index-pack.c b/builtin/index-pack.c
index 07b2c0c..27e3c8b 100644
--- a/builtin/index-pack.c
+++ b/builtin/index-pack.c
@@ -28,11 +28,6 @@ struct object_stat {
int base_object_no;
};
-union delta_base {
- unsigned char sha1[20];
- off_t offset;
-};
-
struct base_data {
struct base_data *base;
struct base_data *child;
@@ -52,26 +47,28 @@ struct thread_local {
int pack_fd;
};
-/*
- * Even if sizeof(union delta_base) == 24 on 64-bit archs, we really want
- * to memcmp() only the first 20 bytes.
- */
-#define UNION_BASE_SZ 20
-
#define FLAG_LINK (1u20)
#define FLAG_CHECKED (1u21)
-struct delta_entry {
- union delta_base base;
+struct ofs_delta_entry {
+ off_t offset;
+ int obj_no;
+} __attribute__((packed));
+
+struct ref_delta_entry {
+ unsigned char sha1[20];
int obj_no;
};
static struct object_entry *objects;
static struct object_stat *obj_stat;
-static struct delta_entry *deltas;
+static struct ofs_delta_entry *ofs_deltas;
+static struct ref_delta_entry *ref_deltas;
static struct thread_local nothread_data;
static int nr_objects;
-static int nr_deltas;
+static int nr_ofs_deltas;
+static int nr_ref_deltas;
+static int ref_deltas_alloc;
static int nr_resolved_deltas;
static int nr_threads;
@@ -480,7 +477,8 @@ static void *unpack_entry_data(unsigned long offset,
unsigned long size,
}
static void *unpack_raw_entry(struct object_entry *obj,
-
Re: 'git rebase' silently drops changes?
Johannes Sixt j...@kdbg.org writes: Am 09.02.2015 um 13:53 schrieb Sergey Organov: [...] If you want a version of --preserve-merges that does what *you* need, consider this commit: git://repo.or.cz/git/mingw/j6t.git rebase-p-first-parent Use it like this: git rebase -i -p --first-parent ... Thanks a lot, this sounds promising! I've read the message for this commit and it mentions no drawbacks. Are you aware of any? Beware, its implementation is incomplete: if the rebase is interrupted, then 'git rebase --continue' behaves as if --first-parent were not given. Just never did get round to it, or something more fundamental? To be useful for me, it also needs a support for 'git pull' to pass this flag to 'git rebase', but that I think I can easily fix myself. it is impossible for git rebase to decide to which rebased commit the amendement applies. It doesn't even try to guess. It's the responsibility of the user to apply the amendment to the correct commit. Yeah, this sounds reasonable, /except/ git even gives no warning when it drops amendments. Shouldn't 'git rebase' rather consider merge amendment a kind of conflict? There is work in progress where a merge is computed entirely in-memory (without relying on files in the worktree). It could be used to detect whether there are any changes beyond the automatic merge results, and they could be warned about. Nice to hear there are chances to improve this in the future. Thanks again! -- Sergey. -- To unsubscribe from this list: send the line unsubscribe 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: reduce memory footprint a bit
I'm having trouble getting this new patch to apply. Are you working
on a branch that I can track?
On Tue, Feb 10, 2015 at 3:30 AM, Duy Nguyen pclo...@gmail.com wrote:
On Mon, Feb 09, 2015 at 11:27:21AM -0800, Junio C Hamano wrote:
On a 3.4M object repo that's about 53MB. The saving is less impressive
compared to index-pack total memory use (about 400MB before delta
resolving, so the saving is just 13%)
Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
I'm not sure if this patch is worth pursuing. It makes the code a
little bit harder to read. I was just wondering how much memory could
be saved..
(text reordered)
I do not find the result all that harder to read. I however think
that the change would make it a lot harder to maintain, especially
because the name object-entry-extra does not have any direct link
to show-stat to hint us that this must be allocated when show-stat
is in use and must never be looked at when show-stat is not in use.
Noted. To be fixed.
I would say 13% is already impressive ;-).
The second patch makes the total saving 119MB, close to 30% (again on
x86-64, 32-bit platform number may be different). If we only compare
with the size of objects[] and deltas[], the saving percentage is 37%
(only for clone case) for this repo. Now it looks impressive to me :-D
The patch is larger than the previous one, but not really complex. And
the final index-pack.c is not hard to read either, probably becase we
already handle ofs-delta and ref-delta separately.
-- 8 --
Subject: [PATCH 2/2] index-pack: kill union delta_base to save memory
Once we know the number of objects in the input pack, we allocate an
array of nr_objects of struct delta_entry. On x86-64, this struct is
32 bytes long. The union delta_base, which is part of struct
delta_entry, provides enough space to store either ofs-delta (8 bytes)
or ref-delta (20 bytes).
Notice that with recent Git versions, ofs-delta objects are
preferred over ref-delta objects and ref-delta objects have no reason
to be present in a clone pack. So in clone case we waste
(20-8) * nr_objects bytes because of this union. That's about 38MB out
of 100MB for deltas[] with 3.4M objects, or 38%. deltas[] would be
around 62MB without the waste.
This patch attempts to eliminate that. deltas[] array is split into
two: one for ofs-delta and one for ref-delta. Many functions are also
duplicated because of this split. With this patch, ofs_delta_entry[]
array takes 38MB. ref_deltas[] should remain unallocated in clone case
(0 bytes). This array grows as we see ref-delta. We save more than
half in clone case, or 25% of total book keeping.
The saving is more than the calculation above because padding is
removed by __attribute__((packed)) on ofs_delta_entry. This attribute
should be ok to use, as we used to have it in our code base for some
time. The last use was removed because it may lead to incorrect
behavior when the struct is not packed, which is not the case in
index-pack.
A note about ofs_deltas allocation. We could use ref_deltas memory
allocation strategy for ofs_deltas. But that probably just adds more
overhead on top. ofs-deltas are generally the majority (1/2 to 2/3) in
any pack. Incremental realloc may lead to too many memcpy. And if we
preallocate, say 1/2 or 2/3 of nr_objects initially, the growth rate
of ALLOC_GROW() could make this array larger than nr_objects, wasting
more memory.
Brought-up-by: Matthew Sporleder msporle...@gmail.com
Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
builtin/index-pack.c | 260
+++
1 file changed, 160 insertions(+), 100 deletions(-)
diff --git a/builtin/index-pack.c b/builtin/index-pack.c
index 07b2c0c..27e3c8b 100644
--- a/builtin/index-pack.c
+++ b/builtin/index-pack.c
@@ -28,11 +28,6 @@ struct object_stat {
int base_object_no;
};
-union delta_base {
- unsigned char sha1[20];
- off_t offset;
-};
-
struct base_data {
struct base_data *base;
struct base_data *child;
@@ -52,26 +47,28 @@ struct thread_local {
int pack_fd;
};
-/*
- * Even if sizeof(union delta_base) == 24 on 64-bit archs, we really want
- * to memcmp() only the first 20 bytes.
- */
-#define UNION_BASE_SZ 20
-
#define FLAG_LINK (1u20)
#define FLAG_CHECKED (1u21)
-struct delta_entry {
- union delta_base base;
+struct ofs_delta_entry {
+ off_t offset;
+ int obj_no;
+} __attribute__((packed));
+
+struct ref_delta_entry {
+ unsigned char sha1[20];
int obj_no;
};
static struct object_entry *objects;
static struct object_stat *obj_stat;
-static struct delta_entry *deltas;
+static struct ofs_delta_entry *ofs_deltas;
+static struct ref_delta_entry *ref_deltas;
static struct thread_local nothread_data;
static int nr_objects;
-static int nr_deltas;
+static int nr_ofs_deltas;
+static int
git-p4 is not cloning perforce code properly
Dear All, I am trying to clone perforce branch from git to my local drive, but it's skipping too many files and change list while fetching it from perforce. it'll be very helpful if anyone can suggest me about how to git rid with this issue. Thanks in Advance, Sandeep -- View this message in context: http://git.661346.n2.nabble.com/git-p4-is-not-cloning-perforce-code-properly-tp7625215.html Sent from the git mailing list archive at Nabble.com. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: implement a stable 'Last updated' in Documentation
On Tue, Feb 10, 2015 at 04:17:47PM +0100, Olaf Hering wrote: On Fri, Jan 30, Jeff King wrote: I have 8.6.9-3 installed (it is part of Debian testing/unstable now), and confirmed that: diff --git a/Documentation/asciidoc.conf b/Documentation/asciidoc.conf index 2c16c53..10c777e 100644 --- a/Documentation/asciidoc.conf +++ b/Documentation/asciidoc.conf @@ -21,6 +21,7 @@ tilde=#126; apostrophe=#39; backtick=#96; litdd=#45;#45; +footer-style=none ifdef::backend-docbook[] [linkgit-inlinemacro] drops the last-updated footer. This does not remove Last updated from files like using-merge-subtree.html for me, using asciidoc-8.6.8. Yes, the feature is part of 8.6.9-3. -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] fast-import: avoid running end_packfile recursively
Jeff King p...@peff.net writes: When an import has finished, we run end_packfile() to finalize the data and move the packfile into place. If this process fails, we call die() and end up in our die_nicely() handler. Which unfortunately includes running end_packfile to save any progress we made. We enter the function again, and start operating on the pack_data struct while it is in an inconsistent state, leading to a segfault. ... This new problem is quite similar, except that we are worried about calling die() _during_ end_packfile, not right after. Ideally we would simply set pack_data to NULL as soon as we enter the function, and operate on a copy of the pointer. Nicely analyzed and well done. Unfortunately, it is not so easy. pack_data is a global, and end_packfile calls into other functions which operate on the global directly. We would have to teach each of these to take an argument, and there is no guarantee that we would catch all of the spots. Well, you can rename the global to something else to make sure ;-) But I think that the approach with a simple flag is better. If we were planning to do the global-to-parameter surgery for other reasons (perhaps need to make things reentrant?) then the equation might become different, but I do not think we are doing that right now, so... 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: 'git rebase' silently drops changes?
Am 10.02.2015 um 12:46 schrieb Sergey Organov: Johannes Sixt j...@kdbg.org writes: Am 09.02.2015 um 13:53 schrieb Sergey Organov: [...] If you want a version of --preserve-merges that does what *you* need, consider this commit: git://repo.or.cz/git/mingw/j6t.git rebase-p-first-parent Use it like this: git rebase -i -p --first-parent ... Thanks a lot, this sounds promising! I've read the message for this commit and it mentions no drawbacks. Are you aware of any? Except for this... Beware, its implementation is incomplete: if the rebase is interrupted, then 'git rebase --continue' behaves as if --first-parent were not given. ... not anything that I would care about. Of course, you can't rebase branchy history when this option is specified, but that's the whole point. Just never did get round to it, or something more fundamental? Nothing fundamental. Just has to store the option in the rebase state like all the other options. -- 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 06/11] commit: add tests of commit races
Stefan Beller sbel...@google.com writes: On Sun, Feb 8, 2015 at 8:14 AM, Michael Haggerty mhag...@alum.mit.edu wrote: +# Copyright (c) 2014 Michael Haggerty mhag...@alum.mit.edu What is the projects stance on copyright lines? I do not think we have a strong one. I've seen files (most of them from the beginning) having some copyright lines, other files (often introduced way later) not having them, because we're git and have history, so we know who did it. I personally agree with that statement. Also, a copyright notice per file is often added when a new file is added, but that ends up giving false sense of ownership to everybody else down the line even after the file has been extensively modified. It's not like Michael solely owns all lines in this file in later versions. And even if people added their name at the top every time they make any change, their names tend to stay even when their contributions are later completely rewritten or removed. In a sense, my agreement with your statement is stronger than Yes, Git can tell us who did what anyway. What we can find in the history is the sole source of truth, and in-file copyright notice is misleading. You do not even have to have one in the Berne signatory nations anyway. The tests themselves look fine. Is there a reason you did not append the tests in 7509 ? Hmph. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] contrib/completion: suppress stderror in bash
SZEDER Gábor sze...@ira.uka.de writes:
@@ -412,7 +412,7 @@ __git_refs_remotes ()
__git_remotes ()
{
local i IFS=$'\n' d=$(__gitdir)
- test -d $d/remotes ls -1 $d/remotes
+ test -d $d/remotes ls -1 $d/remotes 2/dev/null
for i in $(git --git-dir=$d config --get-regexp
'remote\..*\.url' 2/dev/null); do
i=${i#remote.}
echo ${i/.url*/}
Do I smell some bitrotting here?
This function just lists all the defined remotes, first by listing the
directories under refs/remotes to get the legacy remotes and then
loops over 'git config's output to get the modern ones. This
predates the arrival of the 'git remote' command in January 2007, so
it was really a long time ago.
We should just run 'git remote' instead, shouldn't we?
Perhaps. Is it sufficient to just make __git_remotes() a thin
wrapper around, i.e.
__git_remotes ()
{
git remotes
}
or do we need to munge its output further (I didn't look)?
--
To unsubscribe from this list: send the line unsubscribe 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: reduce memory footprint a bit
matthew sporleder msporle...@gmail.com writes: I'm having trouble getting this new patch to apply. Apply the first one, replace all object_entry_extra with object_stat, replace all objects_extra with obj_stat and amend the first one. Then apply this one. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] fast-import: avoid running end_packfile recursively
On Tue, Feb 10, 2015 at 10:45:20AM -0800, Junio C Hamano wrote:
Unfortunately, it is not so easy. pack_data is a global, and
end_packfile calls into other functions which operate on the
global directly. We would have to teach each of these to
take an argument, and there is no guarantee that we would
catch all of the spots.
Well, you can rename the global to something else to make sure ;-)
But I think that the approach with a simple flag is better.
:) True. The problems I had in mind were more:
1. One of the problems with that is that there are a whole bunch of
helper functions that use the variable. But only the ones that are
called as part of end_packfile need this treatment. So either we
need to touch all of them, or we need to figure out reliably which
ones are part of this code path.
2. Unless we get rid of the global completely, we open ourselves up to
end_packfile calling new functions, bringing the problem back. This
is probably not a huge concern, though, as this code has basically
not changed much since its inception.
I did work through it, though, and the result is not _too_ bad. Here it
is for reference, in case you want to change your mind. The remaining
references are only in start_packfile, and in gfi_load_object, both of
which are hopefully unlikely to be called from end_packfile.
---
diff --git a/fast-import.c b/fast-import.c
index d0bd285..e842386 100644
--- a/fast-import.c
+++ b/fast-import.c
@@ -883,7 +883,7 @@ static void start_packfile(void)
all_packs[pack_id] = p;
}
-static const char *create_index(void)
+static const char *create_index(struct packed_git *pack)
{
const char *tmpfile;
struct pack_idx_entry **idx, **c, **last;
@@ -901,18 +901,18 @@ static const char *create_index(void)
if (c != last)
die(internal consistency error creating the index);
- tmpfile = write_idx_file(NULL, idx, object_count, pack_idx_opts,
pack_data-sha1);
+ tmpfile = write_idx_file(NULL, idx, object_count, pack_idx_opts,
pack-sha1);
free(idx);
return tmpfile;
}
-static char *keep_pack(const char *curr_index_name)
+static char *keep_pack(struct packed_git *pack, const char *curr_index_name)
{
static char name[PATH_MAX];
static const char *keep_msg = fast-import;
int keep_fd;
- keep_fd = odb_pack_keep(name, sizeof(name), pack_data-sha1);
+ keep_fd = odb_pack_keep(name, sizeof(name), pack-sha1);
if (keep_fd 0)
die_errno(cannot create keep file);
write_or_die(keep_fd, keep_msg, strlen(keep_msg));
@@ -920,12 +920,12 @@ static char *keep_pack(const char *curr_index_name)
die_errno(failed to write keep file);
snprintf(name, sizeof(name), %s/pack/pack-%s.pack,
-get_object_directory(), sha1_to_hex(pack_data-sha1));
- if (move_temp_to_file(pack_data-pack_name, name))
+get_object_directory(), sha1_to_hex(pack-sha1));
+ if (move_temp_to_file(pack-pack_name, name))
die(cannot store pack file);
snprintf(name, sizeof(name), %s/pack/pack-%s.idx,
-get_object_directory(), sha1_to_hex(pack_data-sha1));
+get_object_directory(), sha1_to_hex(pack-sha1));
if (move_temp_to_file(curr_index_name, name))
die(cannot store index file);
free((void *)curr_index_name);
@@ -947,8 +947,11 @@ static void unkeep_all_packs(void)
static void end_packfile(void)
{
+ struct packed_git *old_p = pack_data;
+
if (!pack_data)
return;
+ pack_data = NULL;
clear_delta_base_cache();
if (object_count) {
@@ -959,13 +962,13 @@ static void end_packfile(void)
struct branch *b;
struct tag *t;
- close_pack_windows(pack_data);
+ close_pack_windows(old_p);
sha1close(pack_file, cur_pack_sha1, 0);
- fixup_pack_header_footer(pack_data-pack_fd, pack_data-sha1,
- pack_data-pack_name, object_count,
+ fixup_pack_header_footer(old_p-pack_fd, old_p-sha1,
+ old_p-pack_name, object_count,
cur_pack_sha1, pack_size);
- close(pack_data-pack_fd);
- idx_name = keep_pack(create_index());
+ close(old_p-pack_fd);
+ idx_name = keep_pack(old_p, create_index(old_p));
/* Register the packfile with core git's machinery. */
new_p = add_packed_git(idx_name, strlen(idx_name), 1);
@@ -994,11 +997,10 @@ static void end_packfile(void)
pack_id++;
}
else {
- close(pack_data-pack_fd);
- unlink_or_warn(pack_data-pack_name);
+ close(old_p-pack_fd);
+ unlink_or_warn(old_p-pack_name);
Re: [PATCH] contrib/completion: suppress stderror in bash
Hi,
Quoting Junio C Hamano gits...@pobox.com:
SZEDER Gábor sze...@ira.uka.de writes:
@@ -412,7 +412,7 @@ __git_refs_remotes ()
__git_remotes ()
{
local i IFS=$'\n' d=$(__gitdir)
- test -d $d/remotes ls -1 $d/remotes
+ test -d $d/remotes ls -1 $d/remotes 2/dev/null
for i in $(git --git-dir=$d config --get-regexp
'remote\..*\.url' 2/dev/null); do
i=${i#remote.}
echo ${i/.url*/}
Do I smell some bitrotting here?
This function just lists all the defined remotes, first by listing the
directories under refs/remotes to get the legacy remotes and then
loops over 'git config's output to get the modern ones. This
predates the arrival of the 'git remote' command in January 2007, so
it was really a long time ago.
We should just run 'git remote' instead, shouldn't we?
Perhaps. Is it sufficient to just make __git_remotes() a thin
wrapper around, i.e.
__git_remotes ()
{
git remotes
}
or do we need to munge its output further (I didn't look)?
Well, just like in other cases where we run git from the completion
script, we need a '--git-dir=$(__gitdir)' as well, because the user can
specify the path to a different repo via $GIT_DIR or on the command
line.
Other than that it seems we are OK. Docs say With no arguments,
shows a list of existing remotes. and as far as I can tell, on
MSysGit, it does so without any funny formatting.
But beware, I spent most of last year cycling to Mongolia and my git-fu
became a somewhat rusty... and I'm not quite up to speed yet.
Best,
Gábor
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Gmail Message rejection
On 10 February 2015 at 09:35, Matthieu Moy matthieu@grenoble-inp.fr wrote: Stefan Beller stefanbel...@gmail.com writes: If you want email to send via gmail, you can do so by enabling text only mode for sending mails. Unfortunately, it is also a known bug that the Android Gmail client doesn't have this option (or didn't last time I checked). Don't use it, then! We should not design our infrastructure to the lowest common denominator like that. Especially if it's just a question of supporting broken brand new implementations, those which couldn't have been bothered to adhere to the best practices. C. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] merge-file: correctly open files when in a subdir
Aleksander Boruch-Gruszecki aleksander.boruchgrusze...@gmail.com
writes:
run_setup_gently() is called before merge-file. This may result in changing
current working directory, which wasn't taken into account when opening a file
for writing.
Fix by prepending the passed prefix. Previous var is left so that error
messages keep refering to the file from the user's working directory
perspective.
Signed-off-by: Aleksander Boruch-Gruszecki
aleksander.boruchgrusze...@gmail.com
Please don't line wrap the footer.
---
builtin/merge-file.c | 3 ++-
t/t6023-merge-file.sh | 6 ++
2 files changed, 8 insertions(+), 1 deletion(-)
This patch does not apply.
diff --git a/builtin/merge-file.c b/builtin/merge-file.c
index 844f84f..232b768 100644
--- a/builtin/merge-file.c
+++ b/builtin/merge-file.c
@@ -90,7 +90,8 @@ int cmd_merge_file(int argc, const char **argv,
const char *prefix)
Please do not line-wrap the patch, either.
if (ret = 0) {
The original has a single tab at the beginning of this line to
indent, not four spaces.
const char *filename = argv[0];
-FILE *f = to_stdout ? stdout : fopen(filename, wb);
+const char *fpath = prefix_filename(prefix, prefixlen, argv[0]);
+FILE *f = to_stdout ? stdout : fopen(fpath, wb);
if (!f)
ret = error(Could not open %s for writing, filename);
diff --git a/t/t6023-merge-file.sh b/t/t6023-merge-file.sh
index 3758961..fdd104c 100755
--- a/t/t6023-merge-file.sh
+++ b/t/t6023-merge-file.sh
@@ -72,6 +72,12 @@ test_expect_success 'works in subdirectory' '
( cd dir git merge-file a.txt o.txt b.txt )
'
+mkdir -p dir/deep
+cp new1.txt orig.txt new2.txt dir/deep
+test_expect_success 'accounts for subdirectory when writing' '
+(cd dir git merge-file deep/new1.txt deep/orig.txt deep/new2.txt)
+'
Interesting. Makes us wonder why the one before this new one you
added did not catch the issue, doesn't it?
+
cp new1.txt test.txt
test_expect_success merge without conflict (--quiet) \
git merge-file --quiet test.txt orig.txt new2.txt
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] config: add show_err flag to git_config_parse_key()
`git_config_parse_key()` is used to sanitize the input key.
Some callers of the function like `git_config_set_multivar_in_file()`
get the per-sanitized key directly from the user so it becomes
necessary to raise an error specifying what went wrong when the entered
key is defective.
Other callers like `configset_find_element()` get their keys from
the git itself so a return value signifying error would be enough.
The error output shown to the user is useless and confusing in this
case so add a show_err flag to suppress errors in such cases.
Signed-off-by: Tanay Abhra tanay...@gmail.com
---
Hi,
I just saw your mail late in the night (I didn't had net for a week).
This patch just squelches the error message, I will take a better
look tomorrow morning.
-Tanay
builtin/config.c | 2 +-
cache.h | 2 +-
config.c | 19 ---
3 files changed, 14 insertions(+), 9 deletions(-)
diff --git a/builtin/config.c b/builtin/config.c
index 15a7bea..d5070d7 100644
--- a/builtin/config.c
+++ b/builtin/config.c
@@ -200,7 +200,7 @@ static int get_value(const char *key_, const char *regex_)
goto free_strings;
}
} else {
- if (git_config_parse_key(key_, key, NULL)) {
+ if (git_config_parse_key(key_, key, NULL, 1)) {
ret = CONFIG_INVALID_KEY;
goto free_strings;
}
diff --git a/cache.h b/cache.h
index f704af5..1c0914d 100644
--- a/cache.h
+++ b/cache.h
@@ -1358,7 +1358,7 @@ extern int git_config_string(const char **, const char *,
const char *);
extern int git_config_pathname(const char **, const char *, const char *);
extern int git_config_set_in_file(const char *, const char *, const char *);
extern int git_config_set(const char *, const char *);
-extern int git_config_parse_key(const char *, char **, int *);
+extern int git_config_parse_key(const char *, char **, int *, int);
extern int git_config_set_multivar(const char *, const char *, const char *,
int);
extern int git_config_set_multivar_in_file(const char *, const char *, const
char *, const char *, int);
extern int git_config_rename_section(const char *, const char *);
diff --git a/config.c b/config.c
index 752e2e2..074a671 100644
--- a/config.c
+++ b/config.c
@@ -1309,7 +1309,7 @@ static struct config_set_element
*configset_find_element(struct config_set *cs,
* `key` may come from the user, so normalize it before using it
* for querying entries from the hashmap.
*/
- ret = git_config_parse_key(key, normalized_key, NULL);
+ ret = git_config_parse_key(key, normalized_key, NULL, 0);
if (ret)
return NULL;
@@ -1842,8 +1842,9 @@ int git_config_set(const char *key, const char *value)
* lowercase section and variable name
* baselen - pointer to int which will hold the length of the
* section + subsection part, can be NULL
+ * show_err - toggle whether the function raises an error on a defective key
*/
-int git_config_parse_key(const char *key, char **store_key, int *baselen_)
+int git_config_parse_key(const char *key, char **store_key, int *baselen_, int
show_err)
{
int i, dot, baselen;
const char *last_dot = strrchr(key, '.');
@@ -1854,12 +1855,14 @@ int git_config_parse_key(const char *key, char
**store_key, int *baselen_)
*/
if (last_dot == NULL || last_dot == key) {
- error(key does not contain a section: %s, key);
+ if (show_err)
+ error(key does not contain a section: %s, key);
return -CONFIG_NO_SECTION_OR_NAME;
}
if (!last_dot[1]) {
- error(key does not contain variable name: %s, key);
+ if (show_err)
+ error(key does not contain variable name: %s, key);
return -CONFIG_NO_SECTION_OR_NAME;
}
@@ -1881,12 +1884,14 @@ int git_config_parse_key(const char *key, char
**store_key, int *baselen_)
if (!dot || i baselen) {
if (!iskeychar(c) ||
(i == baselen + 1 !isalpha(c))) {
- error(invalid key: %s, key);
+ if (show_err)
+ error(invalid key: %s, key);
goto out_free_ret_1;
}
c = tolower(c);
} else if (c == '\n') {
- error(invalid key (newline): %s, key);
+ if (show_err)
+ error(invalid key (newline): %s, key);
goto out_free_ret_1;
}
(*store_key)[i] = c;
@@ -1936,7 +1941,7 @@ int git_config_set_multivar_in_file(const char
*config_filename,
char *filename_buf = NULL;
/* parse-key returns negative; flip the sign
[PATCH v2] merge-file: correctly open files when in a subdir
run_setup_gently() is called before merge-file. This may result in changing
current working directory, which wasn't taken into account when opening a file
for writing.
Fix by prepending the passed prefix. Previous var is left so that error
messages keep refering to the file from the user's working directory
perspective.
Signed-off-by: Aleksander Boruch-Gruszecki
aleksander.boruchgrusze...@gmail.com
---
builtin/merge-file.c | 3 ++-
t/t6023-merge-file.sh | 6 ++
2 files changed, 8 insertions(+), 1 deletion(-)
diff --git a/builtin/merge-file.c b/builtin/merge-file.c
index 844f84f..232b768 100644
--- a/builtin/merge-file.c
+++ b/builtin/merge-file.c
@@ -90,7 +90,8 @@ int cmd_merge_file(int argc, const char **argv,
const char *prefix)
if (ret = 0) {
const char *filename = argv[0];
-FILE *f = to_stdout ? stdout : fopen(filename, wb);
+const char *fpath = prefix_filename(prefix, prefixlen, argv[0]);
+FILE *f = to_stdout ? stdout : fopen(fpath, wb);
if (!f)
ret = error(Could not open %s for writing, filename);
diff --git a/t/t6023-merge-file.sh b/t/t6023-merge-file.sh
index 3758961..fdd104c 100755
--- a/t/t6023-merge-file.sh
+++ b/t/t6023-merge-file.sh
@@ -72,6 +72,12 @@ test_expect_success 'works in subdirectory' '
( cd dir git merge-file a.txt o.txt b.txt )
'
+mkdir -p dir/deep
+cp new1.txt orig.txt new2.txt dir/deep
+test_expect_success 'accounts for subdirectory when writing' '
+(cd dir git merge-file deep/new1.txt deep/orig.txt deep/new2.txt)
+'
+
cp new1.txt test.txt
test_expect_success merge without conflict (--quiet) \
git merge-file --quiet test.txt orig.txt new2.txt
--
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 v4 0/4] git apply safety
Git tracks symbolic links; e.g. you can remove files that have been tracked in a directory dir/file* and then creates a symbolic link at dir to point elsewhere, express such a change as a patchset and then apply it to the original tree. Consequently, applying a patch to update dir/file, when you have dir as a symbolic link pointing somewhere, must fail, just like a patch whose preimage does not match what you have in tree you are trying to apply the patch to gets rejected. Also, we fundamentally do not like to touch a path that contains .git as a path component. This round uses cache_file_exists() in the last patch to cope with case insensitive filesystems better. The previous round begins here: http://thread.gmane.org/gmane.comp.version-control.git/263341 Junio C Hamano (4): apply: reject input that touches outside the working area apply: do not read from the filesystem under --index apply: do not read from beyond a symbolic link apply: do not touch a file beyond a symbolic link Documentation/git-apply.txt | 12 +++- builtin/apply.c | 142 +++- t/t4122-apply-symlink-inside.sh | 106 ++ t/t4139-apply-escape.sh | 141 +++ 4 files changed, 399 insertions(+), 2 deletions(-) create mode 100755 t/t4139-apply-escape.sh -- 2.3.0-185-g073f588 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v4 2/4] apply: do not read from the filesystem under --index
We currently read the preimage to apply a patch from the index only
when the --cached option is given. Do so also when the command is
running under the --index option. With --index, the index entry and
the working tree file for a path that is involved in a patch must be
identical, so this should not affect the result, but by reading from
the index, we will get the protection to avoid reading an unintended
path beyond a symbolic link automatically.
Signed-off-by: Junio C Hamano gits...@pobox.com
---
builtin/apply.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/builtin/apply.c b/builtin/apply.c
index 8561236..21e45a0 100644
--- a/builtin/apply.c
+++ b/builtin/apply.c
@@ -3136,7 +3136,7 @@ static int load_patch_target(struct strbuf *buf,
const char *name,
unsigned expected_mode)
{
- if (cached) {
+ if (cached || check_index) {
if (read_file_or_gitlink(ce, buf))
return error(_(read of %s failed), name);
} else if (name) {
--
2.3.0-185-g073f588
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v4 1/4] apply: reject input that touches outside the working area
By default, a patch that affects outside the working area (either a
Git controlled working tree, or the current working directory when
git apply is used as a replacement of GNU patch) is rejected as a
mistake (or a mischief). Git itself does not create such a patch,
unless the user bends over backwards and specifies a non-standard
prefix to git diff and friends.
When `git apply` is used as a better GNU patch, the user can pass
the `--unsafe-paths` option to override this safety check. This
option has no effect when `--index` or `--cached` is in use.
The new test was stolen from Jeff King with slight enhancements.
Note that a few new tests for touching outside the working area by
following a symbolic link are still expected to fail at this step,
but will be fixed in later steps.
Signed-off-by: Junio C Hamano gits...@pobox.com
---
Documentation/git-apply.txt | 12 +++-
builtin/apply.c | 26
t/t4139-apply-escape.sh | 141
3 files changed, 178 insertions(+), 1 deletion(-)
create mode 100755 t/t4139-apply-escape.sh
diff --git a/Documentation/git-apply.txt b/Documentation/git-apply.txt
index f605327..9489664 100644
--- a/Documentation/git-apply.txt
+++ b/Documentation/git-apply.txt
@@ -16,7 +16,7 @@ SYNOPSIS
[--ignore-space-change | --ignore-whitespace ]
[--whitespace=(nowarn|warn|fix|error|error-all)]
[--exclude=path] [--include=path] [--directory=root]
- [--verbose] [patch...]
+ [--verbose] [--unsafe-paths] [patch...]
DESCRIPTION
---
@@ -229,6 +229,16 @@ For example, a patch that talks about updating
`a/git-gui.sh` to `b/git-gui.sh`
can be applied to the file in the working tree `modules/git-gui/git-gui.sh` by
running `git apply --directory=modules/git-gui`.
+--unsafe-paths::
+ By default, a patch that affects outside the working area
+ (either a Git controlled working tree, or the current working
+ directory when git apply is used as a replacement of GNU
+ patch) is rejected as a mistake (or a mischief).
++
+When `git apply` is used as a better GNU patch, the user can pass
+the `--unsafe-paths` option to override this safety check. This option
+has no effect when `--index` or `--cached` is in use.
+
Configuration
-
diff --git a/builtin/apply.c b/builtin/apply.c
index ef32e4f..8561236 100644
--- a/builtin/apply.c
+++ b/builtin/apply.c
@@ -50,6 +50,7 @@ static int apply_verbosely;
static int allow_overlap;
static int no_add;
static int threeway;
+static int unsafe_paths;
static const char *fake_ancestor;
static int line_termination = '\n';
static unsigned int p_context = UINT_MAX;
@@ -3483,6 +3484,23 @@ static int check_to_create(const char *new_name, int
ok_if_exists)
return 0;
}
+static void die_on_unsafe_path(struct patch *patch)
+{
+ const char *old_name = NULL;
+ const char *new_name = NULL;
+ if (patch-is_delete)
+ old_name = patch-old_name;
+ else if (!patch-is_new !patch-is_copy)
+ old_name = patch-old_name;
+ if (!patch-is_delete)
+ new_name = patch-new_name;
+
+ if (old_name !verify_path(old_name))
+ die(_(invalid path '%s'), old_name);
+ if (new_name !verify_path(new_name))
+ die(_(invalid path '%s'), new_name);
+}
+
/*
* Check and apply the patch in-core; leave the result in patch-result
* for the caller to write it out to the final destination.
@@ -3570,6 +3588,9 @@ static int check_patch(struct patch *patch)
}
}
+ if (!unsafe_paths)
+ die_on_unsafe_path(patch);
+
if (apply_data(patch, st, ce) 0)
return error(_(%s: patch does not apply), name);
patch-rejected = 0;
@@ -4379,6 +4400,8 @@ int cmd_apply(int argc, const char **argv, const char
*prefix_)
N_(make sure the patch is applicable to the current
index)),
OPT_BOOL(0, cached, cached,
N_(apply a patch without touching the working tree)),
+ OPT_BOOL(0, unsafe-paths, unsafe_paths,
+ N_(accept a patch that touches outside the working
area)),
OPT_BOOL(0, apply, force_apply,
N_(also apply the patch (use with
--stat/--summary/--check))),
OPT_BOOL('3', 3way, threeway,
@@ -4451,6 +4474,9 @@ int cmd_apply(int argc, const char **argv, const char
*prefix_)
die(_(--cached outside a repository));
check_index = 1;
}
+ if (check_index)
+ unsafe_paths = 0;
+
for (i = 0; i argc; i++) {
const char *arg = argv[i];
int fd;
diff --git a/t/t4139-apply-escape.sh b/t/t4139-apply-escape.sh
new file mode 100755
index 000..02b97b3
--- /dev/null
+++ b/t/t4139-apply-escape.sh
@@ -0,0 +1,141 @@
+#!/bin/sh
+
[PATCH v4 4/4] apply: do not touch a file beyond a symbolic link
Because Git tracks symbolic links as symbolic links, a path that
has a symbolic link in its leading part (e.g. path/to/dir/file,
where path/to/dir is a symbolic link to somewhere else, be it
inside or outside the working tree) can never appear in a patch
that validly applies, unless the same patch first removes the
symbolic link to allow a directory to be created there.
Detect and reject such a patch.
Things to note:
- Unfortunately, we cannot reuse the has_symlink_leading_path()
from dir.c, as that is only about the working tree, but git
apply can be told to apply the patch only to the index or to
both the index and to the working tree.
- We cannot directly use has_symlink_leading_path() even when we
are applying only to the working tree, as an early patch of a
valid input may remove a symbolic link path/to/dir and then a
later patch of the input may create a path path/to/dir/file, but
git apply first checks the input without touching either the
index or the working tree. The leading symbolic link check must
be done on the interim result we compute in-core (i.e. after the
first patch, there is no path/to/dir symbolic link and it is
perfectly valid to create path/to/dir/file).
Similarly, when an input creates a symbolic link path/to/dir and
then creates a file path/to/dir/file, we need to flag it as an
error without actually creating path/to/dir symbolic link in the
filesystem.
Instead, for any patch in the input that leaves a path (i.e. a non
deletion) in the result, we check all leading paths against the
resulting tree that the patch would create by inspecting all the
patches in the input and then the target of patch application
(either the index or the working tree).
This way, we catch a mischief or a mistake to add a symbolic link
path/to/dir and a file path/to/dir/file at the same time, while
allowing a valid patch that removes a symbolic link path/to/dir and
then adds a file path/to/dir/file.
Signed-off-by: Junio C Hamano gits...@pobox.com
---
builtin/apply.c | 112
t/t4122-apply-symlink-inside.sh | 87 +++
t/t4139-apply-escape.sh | 8 +--
3 files changed, 203 insertions(+), 4 deletions(-)
diff --git a/builtin/apply.c b/builtin/apply.c
index 422e4ce..c2c6fda 100644
--- a/builtin/apply.c
+++ b/builtin/apply.c
@@ -3486,6 +3486,104 @@ static int check_to_create(const char *new_name, int
ok_if_exists)
return 0;
}
+/*
+ * We need to keep track of how symlinks in the preimage are
+ * manipulated by the patches. A patch to add a/b/c where a/b
+ * is a symlink should not be allowed to affect the directory
+ * the symlink points at, but if the same patch removes a/b,
+ * it is perfectly fine, as the patch removes a/b to make room
+ * to create a directory a/b so that a/b/c can be created.
+ */
+static struct string_list symlink_changes;
+#define SYMLINK_GOES_AWAY 01
+#define SYMLINK_IN_RESULT 02
+
+static uintptr_t register_symlink_changes(const char *path, uintptr_t what)
+{
+ struct string_list_item *ent;
+
+ ent = string_list_lookup(symlink_changes, path);
+ if (!ent) {
+ ent = string_list_insert(symlink_changes, path);
+ ent-util = (void *)0;
+ }
+ ent-util = (void *)(what | ((uintptr_t)ent-util));
+ return (uintptr_t)ent-util;
+}
+
+static uintptr_t check_symlink_changes(const char *path)
+{
+ struct string_list_item *ent;
+
+ ent = string_list_lookup(symlink_changes, path);
+ if (!ent)
+ return 0;
+ return (uintptr_t)ent-util;
+}
+
+static void prepare_symlink_changes(struct patch *patch)
+{
+ for ( ; patch; patch = patch-next) {
+ if ((patch-old_name S_ISLNK(patch-old_mode))
+ (patch-is_rename || patch-is_delete))
+ /* the symlink at patch-old_name is removed */
+ register_symlink_changes(patch-old_name,
SYMLINK_GOES_AWAY);
+
+ if (patch-new_name S_ISLNK(patch-new_mode))
+ /* the symlink at patch-new_name is created or remains
*/
+ register_symlink_changes(patch-new_name,
SYMLINK_IN_RESULT);
+ }
+}
+
+static int path_is_beyond_symlink_1(struct strbuf *name)
+{
+ do {
+ unsigned int change;
+
+ while (--name-len name-buf[name-len] != '/')
+ ; /* scan backwards */
+ if (!name-len)
+ break;
+ name-buf[name-len] = '\0';
+ change = check_symlink_changes(name-buf);
+ if (change SYMLINK_IN_RESULT)
+ return 1;
+ if (change SYMLINK_GOES_AWAY)
+ /*
+* This cannot be return 0, because we may
+* see a new one created at a higher level.
+
[PATCH v4 3/4] apply: do not read from beyond a symbolic link
We should reject a patch, whether it renames/copies dir/file to
elsewhere with or without modificiation, or updates dir/file in
place, if dir/ part is actually a symbolic link to elsewhere,
by making sure that the code to read the preimage does not read
from a path that is beyond a symbolic link.
Signed-off-by: Junio C Hamano gits...@pobox.com
---
builtin/apply.c | 2 ++
t/t4122-apply-symlink-inside.sh | 19 +++
2 files changed, 21 insertions(+)
diff --git a/builtin/apply.c b/builtin/apply.c
index 21e45a0..422e4ce 100644
--- a/builtin/apply.c
+++ b/builtin/apply.c
@@ -3145,6 +3145,8 @@ static int load_patch_target(struct strbuf *buf,
return read_file_or_gitlink(ce, buf);
else
return SUBMODULE_PATCH_WITHOUT_INDEX;
+ } else if (has_symlink_leading_path(name, strlen(name))) {
+ return error(_(reading from '%s' beyond a symbolic
link), name);
} else {
if (read_old_data(st, name, buf))
return error(_(read of %s failed), name);
diff --git a/t/t4122-apply-symlink-inside.sh b/t/t4122-apply-symlink-inside.sh
index 70b3a06..035c080 100755
--- a/t/t4122-apply-symlink-inside.sh
+++ b/t/t4122-apply-symlink-inside.sh
@@ -52,4 +52,23 @@ test_expect_success 'check result' '
'
+test_expect_success SYMLINKS 'do not read from beyond symbolic link' '
+ git reset --hard
+ mkdir -p arch/x86_64/dir
+ arch/x86_64/dir/file
+ git add arch/x86_64/dir/file
+ echo line arch/x86_64/dir/file
+ git diff patch
+ git reset --hard
+
+ mkdir arch/i386/dir
+ arch/i386/dir/file
+ ln -s ../i386/dir arch/x86_64/dir
+
+ test_must_fail git apply patch
+ test_must_fail git apply --cached patch
+ test_must_fail git apply --index patch
+
+'
+
test_done
--
2.3.0-185-g073f588
--
To unsubscribe from this list: send the line 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] refs.c: get rid of force_write flag
Signed-off-by: Stefan Beller sbel...@google.com
---
Notes:
When this patch series is applied, you only have 3 occurences of
force_write.
1. In the struct as an undocumented int.
2. In lock_ref_sha1_basic:
if ((flags REF_NODEREF) (type REF_ISSYMREF))
lock-force_write = 1;
3: In ref_transaction_commit:
/* Perform updates first so live commits remain referenced */
for (i = 0; i n; i++) {
struct ref_update *update = updates[i];
if (!is_null_sha1(update-new_sha1)) {
if (!update-lock-force_write
!hashcmp(update-lock-old_sha1, update-new_sha1))
{
unlock_ref(update-lock);
update-lock = NULL;
} else if (write_ref_sha1(update-lock,
update-new_sha1,
update-msg)) {
update-lock = NULL; /* freed by write_ref_sha1
*/
strbuf_addf(err, Cannot update the ref '%s'.,
update-refname);
ret = TRANSACTION_GENERIC_ERROR;
goto cleanup;
} else {
/* freed by write_ref_sha1(): */
update-lock = NULL;
}
}
}
So maybe we can solve it even more elegant by omiting the first 2
occurences and
directly check the type and flags in ref_transaction_commit.
Maybe this makes sense to go on top of that series?
Thanks,
Stefan
refs.c | 9 -
1 file changed, 4 insertions(+), 5 deletions(-)
diff --git a/refs.c b/refs.c
index 3fcf342..ae24502 100644
--- a/refs.c
+++ b/refs.c
@@ -12,7 +12,6 @@ struct ref_lock {
struct lock_file *lk;
unsigned char old_sha1[20];
int lock_fd;
- int force_write;
};
/*
@@ -2319,8 +2318,6 @@ static struct ref_lock *lock_ref_sha1_basic(const char
*refname,
lock-ref_name = xstrdup(refname);
lock-orig_ref_name = xstrdup(orig_refname);
ref_file = git_path(%s, refname);
- if ((flags REF_NODEREF) (type REF_ISSYMREF))
- lock-force_write = 1;
retry:
switch (safe_create_leading_directories(ref_file)) {
@@ -3788,8 +3785,10 @@ int ref_transaction_commit(struct ref_transaction
*transaction,
struct ref_update *update = updates[i];
if (!is_null_sha1(update-new_sha1)) {
- if (!update-lock-force_write
- !hashcmp(update-lock-old_sha1, update-new_sha1))
{
+ /* Ignore symbolic links when told not to dereference */
+ if (!((update-type REF_ISSYMREF)
+ (update-flags REF_NODEREF))
+!hashcmp(update-lock-old_sha1,
update-new_sha1)) {
unlock_ref(update-lock);
update-lock = NULL;
} else if (write_ref_sha1(update-lock,
update-new_sha1,
--
2.2.1.62.g3f15098
--
To unsubscribe from this list: send the line unsubscribe 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/3] trailer: add a trailer.trimEmpty config option
Christian Couder christian.cou...@gmail.com writes: I think that very few new features are now needed to make it possible to use the code in other commands like commit, format-patch, am, etc, but this patch implements one of the needed features. - do trailer stuff by calling a central helper that does trailer stuff a pointer to the middle, trailers, struct. - when the trailer becomes a reusable subsystem, this central helper will become the primary entry point to the API. - trailer stuff will include adding new ones, removing existing ones, and rewriting lines. What you do in the current process_command_line_args() and process_trailers_lists() [*1*] would come here. - write out the result, given the outermost struct. This will become another entry point in the API. Structured that way, callers will supply that outermost structure, and can trust that the trailers subsystem will not molest message_proper or lines_after_trailers part. I don't think it is a big improvement because it is easy to see that the current code doesn't molest the part before and after the trailers. You force the callers that want only the trailer thing to happen to: - pass first and last around. - keep each line of the message body in separate strbuf and have it in the same array as the trailers Neither of which is necessary. I recall that during the review of the previous rounds your own code had to work this around by first concatenating lines (each of which are unnecessarily in separate struct strbufs) into a single strbuf, only to call a helper that takes a single strbuf to count what to ignore in it, and then iterate over the array of strbuf to add up the lengths of them, which would have been unnecessary if the underlying data structure were saner. -- To unsubscribe from this list: send the line unsubscribe 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/3] trailer: add a trailer.trimEmpty config option
Christian Couder christian.cou...@gmail.com writes: On Sat, Feb 7, 2015 at 9:20 PM, Junio C Hamano gits...@pobox.com wrote: Another problem I have with filter out during the output phase comes from the semantics/correctness in the resulting code, and I suspect that it would need to be done a lot earlier, before you allow the logic such as if I have X, do this, but if there is no X, do this other thing. If you have an X that is empty in the input, trimming that before such logic kicks in and trimming that in the final output phase would give different results, and I think the latter would give a less intuitive result. I think it is much better in the output phase because it is very natural for some projects to have a template message with empty trailers like this: That is exactly my point. With empty trailers in the input, Add this iff there is no existing one will be made useless. I am *not* saying that we must not have a filter at the output phrase. If anything, it would allow people to be more sloppy and hide the problem under the rug. Your code may have a bug or design problem that adds an empty one somewhere even when the user told you that she does not want an empty one in the result. The user may be sloppy and say Add this key-value unconditionally, instead of having to do What is the value I want to add? Oh, it is not empty, so I'll ask the trailer machiner to add this key-value there. I am saying that not filtering the input and whatever intermediate result you produce [*1*] will make the end result much less useful. [Footnote] *1* Filtering intermediate result of course can be done by making sure you do not add an empty one in the first place. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [msysGit] Missing inversion in Makefile (ee9be06)
Another go at this XY-Problem...
From: Philip Oakley philipoak...@iee.org
Sent: Saturday, December 27, 2014 8:17 PM
From: Johannes Sixt j...@kdbg.org
Am 27.12.2014 um 19:49 schrieb Philip Oakley:
Hi,
In ee9be06 (perl: detect new files in MakeMaker builds, 2012-07-27)
there is a step to detect if there has been an update to the PM.*
files,
however it appears that the logic is inverted in the comparison.
I need some extra eye's on this to be sure I have it right (I'm
trying
to debug an old Windows breakage...).
The resultant output of a make dry run included (on my m/c)..:
find perl -type f -name '*.pm' | sort perl/PM.stamp+ \
{ cmp perl/PM.stamp+ perl/PM.stamp /dev/null 2/dev/null || mv
perl/PM.stamp+ perl/PM.stamp; } \
rm -f perl/PM.stamp+
make -C perl PERL_PATH='/usr/bin/perl' prefix='/c/Documents and
Settings/Philip' perl.mak
Shouldn't it be `{ ! cmp ` so that when the files are not identical,
the
move is performed?
https://github.com/git/git/blob/ee9be06770223238c6a22430eb874754dd22dfb0/Makefile#L2097
The existing code looks correct to me. cmp succeeds when the files
are
identical and fails when they are different: When it succeeds (files
are
equal), the mv is not executed. When it fails, either because a file
does not exist or they are different, the mv is executed.
Thanks. The inverse logic had me confused.
It's like 7400's again, for those that remember;-)
Here's where the real problem starts...
I was getting errors from
`cd $git_dir make -n MSVC=1 V=1 2MakeDryErrs.txt 1MakeDry.txt`
(borrowed from 'msvc-build') which reported the PM.stamp as a problem,
with the quoted code being the last part of the MakeDry.txt (and no
PM.stamp seen).
Now that I've been poking and investigating the error's stopped! It's
all getting rather frustrating. Time to go again on a clean and
rebuild..
I'm trying to get the msysgit msvc-build script[1], which essentially
implements the Git 'compat/vcbuild/README', to work again in terms of
creating a Visual Studio [2008] project file (.sln).
If I run the code (find perl -type f -name '*.pm' ...) manually then the
PM.stamp file is created allowing future dry-runs to succeed - hence
some of my confusion.
The script uses git's 'contrib/buildsystems/engine.pl' to parse the
output of:
`make -n MSVC=1 V=1 2\dev\null` [2]
This appears to no longer work because the -n (dry-run) option fails to
run the required 'perl/PM.stamp' during the dry-run. At least that's now
my understanding.
The
https://www.gnu.org/software/make/manual/html_node/Instead-of-Execution.html
page indicates that adding a + to the right rule would be needed to also
run the PM.stamp process during dry-run.
At the moment I'm getting (on my old WinXP machine, using Msysgit 1.9.5
as a basis)
$ make -n MSVC=1 V=1 1makedry.txt
make[1]: *** No rule to make target `PM.stamp', needed by `perl.mak'.
Stop.
make: *** [perl/perl.mak] Error 2
i.e. PM.stamp was not created so can't be the target of the dry-run make
rule.
The makedry.txt file generated ends with the
find perl -type f -name '*.pm' | sort perl/PM.stamp+ \
{ cmp perl/PM.stamp+ perl/PM.stamp /dev/null 2/dev/null || mv
perl/PM.stamp+ perl/PM.stamp; } \
rm -f perl/PM.stamp+
make -C perl PERL_PATH='/usr/bin/perl' prefix='/c/Documents and
Settings/Philip' perl.mak
make[1]: Entering directory `/c/msysgit195/git/perl'
make -C .. GIT-CFLAGS
make[2]: Entering directory `/c/msysgit195/git'
FLAGS='compat/vcbuild/scripts/clink.pl: -Imsvcgit/32bits/include [...]
if test x$FLAGS != x`cat GIT-CFLAGS 2/dev/null` ; then \
echo 2 * new build flags; \
echo $FLAGS GIT-CFLAGS; \
fi
make[2]: Leaving directory `/c/msysgit195/git'
make[1]: Leaving directory `/c/msysgit195/git/perl'
i.e. the commands for the PM.stamp process are listed, rather than
executed as may have been hoped.
I've tried hacking the plus(+) prefix onto the perl/PM.stamp: FORCE
rule, but it gave the same error.
What would be the right way of making a dry-run produce a suitable
complete output? Make files are not something I normally delve into.
--
Philip
[1] https://github.com/msysgit/msysgit/blob/master/bin/msvc-build
[2]
https://github.com/git/git/blob/master/contrib/buildsystems/engine.pl#L75
--
To unsubscribe from this list: send the line unsubscribe 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/8] write_ref_sha1(): remove check for lock == NULL
On Mon, Feb 9, 2015 at 1:12 AM, Michael Haggerty mhag...@alum.mit.edu wrote:
None of the callers pass NULL to this function, and there doesn't seem
to be any usefulness to allowing them to do so.
Usually I'd oppose this change, as it seems to be a good defensive
measure. (I cannot assume future me or anybody knows what they're
doing), but as this function (write_ref_sha1) is not widely exposed
any more since aae383db8 (Apr 28, refs.c: make write_ref_sha1 static),
I think it's safe to assume changes affecting this call are well
understood in the future.
so
Reviewed-by: Stefan Beller sbel...@google.com
Signed-off-by: Michael Haggerty mhag...@alum.mit.edu
---
refs.c | 4
1 file changed, 4 deletions(-)
diff --git a/refs.c b/refs.c
index c5fa709..d1130e2 100644
--- a/refs.c
+++ b/refs.c
@@ -3080,10 +3080,6 @@ static int write_ref_sha1(struct ref_lock *lock,
static char term = '\n';
struct object *o;
- if (!lock) {
- errno = EINVAL;
- return -1;
- }
if (!lock-force_write !hashcmp(lock-old_sha1, sha1)) {
unlock_ref(lock);
return 0;
--
2.1.4
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] git-gui: sort entries in tclIndex
Ping? On Mon, Jan 26, Olaf Hering wrote: ALL_LIBFILES uses wildcard, which provides the result in directory order. This order depends on the underlying filesystem on the buildhost. To get reproducible builds it is required to sort such list before using them. Signed-off-by: Olaf Hering o...@aepfle.de --- git-gui/Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/git-gui/Makefile b/git-gui/Makefile index cde8b2e..7564a18 100644 --- a/git-gui/Makefile +++ b/git-gui/Makefile @@ -258,7 +258,7 @@ lib/tclIndex: $(ALL_LIBFILES) GIT-GUI-VARS rm -f $@ ; \ echo '# Autogenerated by git-gui Makefile' $@ \ echo $@ \ - $(foreach p,$(PRELOAD_FILES) $(ALL_LIBFILES),echo '$(subst lib/,,$p)' $@ ) \ + $(foreach p,$(PRELOAD_FILES) $(sort $(ALL_LIBFILES)),echo '$(subst lib/,,$p)' $@ ) \ echo $@ ; \ fi -- To unsubscribe from this list: send the line unsubscribe 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] config: add show_err flag to git_config_parse_key()
On Wed, Feb 11, 2015 at 01:15:05AM +0530, Tanay Abhra wrote: I just saw your mail late in the night (I didn't had net for a week). This patch just squelches the error message, I will take a better look tomorrow morning. Thanks, this is probably a good first step. We can worry about making the config look actually _work_ as the next step (which does not even have to happen right now; it is not like it hasn't been this way since the very beginning of git). Another option for this first step would be to actually make git_config_parse_key permissive, rather than just squelching the error. That is, to actually look up pager.under_score rather than silently erroring out with an invalid key whenever we are reading (whereas on the writing side, we _do_ want to make sure we enforce syntactic validity). I doubt it matters, much, though. Such a lookup would never succeed, because the config file parser will also not allow it. So assuming the syntactic rules here match what the config file parser does, they are at worst redundant. builtin/config.c | 2 +- cache.h | 2 +- config.c | 19 --- 3 files changed, 14 insertions(+), 9 deletions(-) Here's a test that can be squashed in: diff --git a/t/t7006-pager.sh b/t/t7006-pager.sh index da958a8..a28a2fd 100755 --- a/t/t7006-pager.sh +++ b/t/t7006-pager.sh @@ -447,4 +447,14 @@ test_expect_success TTY 'external command pagers override sub-commands' ' test_cmp expect actual ' +test_expect_success 'command with underscores does not complain' ' + write_script git-under_score -\EOF + echo ok + EOF + git --exec-path=. under_score actual 21 + echo ok expect + test_cmp expect actual +' + + test_done I was tempted to also add something like: test_expect_failure TTY 'command with underscores can override pager' ' test_config pager.under_score sed s/^/paged:// git --exec-path=. under_score actual echo paged:ok expect test_cmp expect actual ' but I am not sure it is worth adding the test, even as a placeholder. Unless we are planning to relax the config syntax, the correct spelling is more like pager.under_score.command. It's probably better to just add the test along with the code when we know what the final form will look like. -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 3/8] lock_ref_sha1_basic(): do not set force_write for missing references
On Mon, Feb 9, 2015 at 1:12 AM, Michael Haggerty mhag...@alum.mit.edu wrote:
If a reference is missing, its SHA-1 will be null_sha1, which can't
possibly match a new value that ref_transaction_commit() is trying to
update it to. So there is no need to set force_write in this scenario.
This commit reverts half the lines of 5bdd8d4a3062a (2008-11, do not
force write of packed refs). And reading both commit messages, they
seem to contradict each other. (Both agree on If a reference is
missing, its SHA-1 will be null_sha1 as provided by resolve_ref, but
the conclusion seems to be different.)
On the other hand, there is more than 6 years difference, so I guess
the meaning and implications of some variables and functions may have
slightly changed.
Signed-off-by: Michael Haggerty mhag...@alum.mit.edu
---
refs.c | 15 ++-
1 file changed, 6 insertions(+), 9 deletions(-)
diff --git a/refs.c b/refs.c
index 651e37e..b083858 100644
--- a/refs.c
+++ b/refs.c
@@ -2259,7 +2259,6 @@ static struct ref_lock *lock_ref_sha1_basic(const char
*refname,
int type, lflags;
int mustexist = (old_sha1 !is_null_sha1(old_sha1));
int resolve_flags = 0;
- int missing = 0;
int attempts_remaining = 3;
lock = xcalloc(1, sizeof(struct ref_lock));
@@ -2298,13 +2297,13 @@ static struct ref_lock *lock_ref_sha1_basic(const
char *refname,
orig_refname, strerror(errno));
goto error_return;
}
- missing = is_null_sha1(lock-old_sha1);
- /* When the ref did not exist and we are creating it,
-* make sure there is no existing ref that is packed
-* whose name begins with our refname, nor a ref whose
-* name is a proper prefix of our refname.
+ /*
+* When the ref did not exist and we are creating it, make
+* sure there is no existing packed ref whose name begins with
+* our refname, nor a packed ref whose name is a proper prefix
+* of our refname.
*/
- if (missing
+ if (is_null_sha1(lock-old_sha1)
!is_refname_available(refname, skip,
get_packed_refs(ref_cache))) {
last_errno = ENOTDIR;
goto error_return;
@@ -2320,8 +2319,6 @@ static struct ref_lock *lock_ref_sha1_basic(const char
*refname,
lock-ref_name = xstrdup(refname);
lock-orig_ref_name = xstrdup(orig_refname);
ref_file = git_path(%s, refname);
- if (missing)
- lock-force_write = 1;
if ((flags REF_NODEREF) (type REF_ISSYMREF))
lock-force_write = 1;
--
2.1.4
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 4/8] reflog: fix documentation
On Mon, Feb 9, 2015 at 1:12 AM, Michael Haggerty mhag...@alum.mit.edu wrote: Update the git reflog usage documentation in the manpage and the command help to match the current reality. Signed-off-by: Michael Haggerty mhag...@alum.mit.edu Reviewed-by: Stefan Beller sbel...@google.com --- -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/8] lock_ref_sha1_basic(): do not set force_write for missing references
On Tue, Feb 10, 2015 at 03:24:47PM -0800, Stefan Beller wrote:
On Mon, Feb 9, 2015 at 1:12 AM, Michael Haggerty mhag...@alum.mit.edu wrote:
If a reference is missing, its SHA-1 will be null_sha1, which can't
possibly match a new value that ref_transaction_commit() is trying to
update it to. So there is no need to set force_write in this scenario.
This commit reverts half the lines of 5bdd8d4a3062a (2008-11, do not
force write of packed refs). And reading both commit messages, they
seem to contradict each other. (Both agree on If a reference is
missing, its SHA-1 will be null_sha1 as provided by resolve_ref, but
the conclusion seems to be different.)
Most of the lines of 5bdd8d4a3062a that are being reverted here are
caching the is_null_sha1() check in the missing variable. And that's
a cleanup in this patch that is not strictly necessary (missing would
only be used once, so it becomes noise).
The interesting thing in the earlier commit was to use the null sha1 to
cause a force-write, rather than lstat()ing the filesystem. And here we
are saying the force-write is not necessary at all, no matter what
storage scheme is used. So I don't think there is any contradiction
between the two.
Is this patch correct that the force-write is not necessary? I think so.
The force-write flag comes from:
commit 732232a123e1e61e38babb1c572722bb8a189ba3
Author: Shawn Pearce spea...@spearce.org
Date: Fri May 19 03:29:05 2006 -0400
Force writing ref if it doesn't exist.
Normally we try to skip writing a ref if its value hasn't changed
but in the special case that the ref doesn't exist but the new
value is going to be 0{40} then force writing the ref anyway.
but I am not sure that logic still holds (if it ever did). We do not ever write
0{40} into a ref value.
-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 5/8] reflog: rearrange the manpage
On Mon, Feb 9, 2015 at 1:12 AM, Michael Haggerty mhag...@alum.mit.edu wrote: ---all:: - Instead of listing refs explicitly, prune all refs. +--stale-fix:: + This revamps the logic -- the definition of broken commit + becomes: a commit that is not reachable from any of the refs and + there is a missing object among the commit, tree, or blob + objects reachable from it that is not reachable from any of the + refs. --stale-fix becomes more and more irrelevant over time, so why not put in at the very end even after --all ? Thinking out loud: (--expire=,--expire-unreachable= and --stale-fix) look like a group and (--updateref --rewrite --verbose and --all) also feel like a group, so you wanted to keep --stale-fix after --expire-unreachable= ? While talking about this man page, we should also add --dry-run? -- To unsubscribe from this list: send the line unsubscribe 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/8] write_ref_sha1(): remove check for lock == NULL
On Tue, Feb 10, 2015 at 02:52:23PM -0800, Stefan Beller wrote: On Mon, Feb 9, 2015 at 1:12 AM, Michael Haggerty mhag...@alum.mit.edu wrote: None of the callers pass NULL to this function, and there doesn't seem to be any usefulness to allowing them to do so. Usually I'd oppose this change, as it seems to be a good defensive measure. (I cannot assume future me or anybody knows what they're doing), but as this function (write_ref_sha1) is not widely exposed any more since aae383db8 (Apr 28, refs.c: make write_ref_sha1 static), I think it's safe to assume changes affecting this call are well understood in the future. Thanks, I was iffy on this change for the same reason, but after your explanation, I too think it is reasonable. -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 3/8] lock_ref_sha1_basic(): do not set force_write for missing references
On Tue, Feb 10, 2015 at 4:05 PM, Jeff King p...@peff.net wrote:
On Tue, Feb 10, 2015 at 03:24:47PM -0800, Stefan Beller wrote:
On Mon, Feb 9, 2015 at 1:12 AM, Michael Haggerty mhag...@alum.mit.edu
wrote:
If a reference is missing, its SHA-1 will be null_sha1, which can't
possibly match a new value that ref_transaction_commit() is trying to
update it to. So there is no need to set force_write in this scenario.
This commit reverts half the lines of 5bdd8d4a3062a (2008-11, do not
force write of packed refs). And reading both commit messages, they
seem to contradict each other. (Both agree on If a reference is
missing, its SHA-1 will be null_sha1 as provided by resolve_ref, but
the conclusion seems to be different.)
Most of the lines of 5bdd8d4a3062a that are being reverted here are
caching the is_null_sha1() check in the missing variable. And that's
a cleanup in this patch that is not strictly necessary (missing would
only be used once, so it becomes noise).
The interesting thing in the earlier commit was to use the null sha1 to
cause a force-write, rather than lstat()ing the filesystem. And here we
are saying the force-write is not necessary at all, no matter what
storage scheme is used. So I don't think there is any contradiction
between the two.
Is this patch correct that the force-write is not necessary? I think so.
The force-write flag comes from:
commit 732232a123e1e61e38babb1c572722bb8a189ba3
Author: Shawn Pearce spea...@spearce.org
Date: Fri May 19 03:29:05 2006 -0400
Force writing ref if it doesn't exist.
Normally we try to skip writing a ref if its value hasn't changed
but in the special case that the ref doesn't exist but the new
value is going to be 0{40} then force writing the ref anyway.
but I am not sure that logic still holds (if it ever did). We do not ever
write
0{40} into a ref value.
-Peff
Makes sense.
--
To unsubscribe from this list: send the line unsubscribe 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 6/8] reflog_expire(): ignore --updateref for symbolic references
On Mon, Feb 9, 2015 at 1:12 AM, Michael Haggerty mhag...@alum.mit.edu wrote:
If we are expiring reflog entries for a symbolic reference, then how
should --updateref be handled if the newest reflog entry is expired?
Option 1: Update the referred-to reference. (This is what the current
code does.) This doesn't make sense, because the referred-to reference
has its own reflog, which hasn't been rewritten.
Option 2: Update the symbolic reference itself (as in, REF_NODEREF).
This would convert the symbolic reference into a non-symbolic
reference (e.g., detaching HEAD), which is surely not what a user
would expect.
Option 3: Error out. This is plausible, but it would make the
following usage impossible:
git reflog expire ... --updateref --all
Option 4: Ignore --updateref for symbolic references.
Ok let me ask a question first about the symbolic refs.
We used to use symbolic links for that, but because of
portability issues we decided to not make it a link, but rather
a standard file containing the pointing link (The content of
.git/HEAD is ref: refs/heads/master\n except when detached)
So this is the only distinction? Or is there also a concept of
symbolic links/pointers for the reflog handling?
We choose to implement option 4.
You're only saying why the other options are insane, not why this
is sane.
Also I'd rather tend for option 3 than 4, as it is a safety measure
(assuming we give a hint to the user what the problem is, and
how it is circumventable)
Note: there are still other problems in this code that will be fixed
in a moment.
Signed-off-by: Michael Haggerty mhag...@alum.mit.edu
---
Documentation/git-reflog.txt | 3 ++-
refs.c | 15 ---
2 files changed, 14 insertions(+), 4 deletions(-)
diff --git a/Documentation/git-reflog.txt b/Documentation/git-reflog.txt
index f15a48e..9b87b46 100644
--- a/Documentation/git-reflog.txt
+++ b/Documentation/git-reflog.txt
@@ -85,7 +85,8 @@ them.
--updateref::
Update the ref with the sha1 of the top reflog entry (i.e.
- ref@\{0\}) after expiring or deleting.
+ ref@\{0\}) after expiring or deleting. (This option is
+ ignored for symbolic references.)
--rewrite::
While expiring or deleting, adjust each reflog entry to ensure
diff --git a/refs.c b/refs.c
index b083858..c0001da 100644
--- a/refs.c
+++ b/refs.c
@@ -4025,6 +4025,7 @@ int reflog_expire(const char *refname, const unsigned
char *sha1,
struct ref_lock *lock;
char *log_file;
int status = 0;
+ int type;
memset(cb, 0, sizeof(cb));
cb.flags = flags;
@@ -4036,7 +4037,7 @@ int reflog_expire(const char *refname, const unsigned
char *sha1,
* reference itself, plus we might need to update the
* reference if --updateref was specified:
*/
- lock = lock_ref_sha1_basic(refname, sha1, NULL, 0, NULL);
+ lock = lock_ref_sha1_basic(refname, sha1, NULL, 0, type);
if (!lock)
return error(cannot lock ref '%s', refname);
if (!reflog_exists(refname)) {
@@ -4073,10 +4074,18 @@ int reflog_expire(const char *refname, const unsigned
char *sha1,
(*cleanup_fn)(cb.policy_cb);
if (!(flags EXPIRE_REFLOGS_DRY_RUN)) {
+ /*
+* It doesn't make sense to adjust a reference pointed
+* to by a symbolic ref based on expiring entries in
+* the symbolic reference's reflog.
+*/
+ int update = (flags EXPIRE_REFLOGS_UPDATE_REF)
+ ~(type REF_ISSYMREF);
+
if (close_lock_file(reflog_lock)) {
status |= error(couldn't write %s: %s, log_file,
strerror(errno));
- } else if ((flags EXPIRE_REFLOGS_UPDATE_REF)
+ } else if (update
(write_in_full(lock-lock_fd,
sha1_to_hex(cb.last_kept_sha1), 40) != 40 ||
write_str_in_full(lock-lock_fd, \n) != 1 ||
@@ -4087,7 +4096,7 @@ int reflog_expire(const char *refname, const unsigned
char *sha1,
} else if (commit_lock_file(reflog_lock)) {
status |= error(unable to commit reflog '%s' (%s),
log_file, strerror(errno));
- } else if ((flags EXPIRE_REFLOGS_UPDATE_REF)
commit_ref(lock)) {
+ } else if (update commit_ref(lock)) {
status |= error(couldn't set %s, lock-ref_name);
}
}
--
2.1.4
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 8/8] reflog_expire(): lock symbolic refs themselves, not their referent
On Mon, Feb 9, 2015 at 1:12 AM, Michael Haggerty mhag...@alum.mit.edu wrote:
When processing the reflog of a symbolic ref, hold the lock on the
symbolic reference itself, not on the reference that it points to.
I am not sure if that makes sense.
So when expiring HEAD, you want to have a .git/HEAD.lock file
instead of a .git/refs/heads/master.lock file?
What would happen if there is a concurrent modification
to the master branch?
Signed-off-by: Michael Haggerty mhag...@alum.mit.edu
---
refs.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/refs.c b/refs.c
index 1b2a497..3fcf342 100644
--- a/refs.c
+++ b/refs.c
@@ -4037,7 +4037,7 @@ int reflog_expire(const char *refname, const unsigned
char *sha1,
* reference itself, plus we might need to update the
* reference if --updateref was specified:
*/
- lock = lock_ref_sha1_basic(refname, sha1, NULL, 0, type);
+ lock = lock_ref_sha1_basic(refname, sha1, NULL, REF_NODEREF, type);
if (!lock)
return error(cannot lock ref '%s', refname);
if (!reflog_exists(refname)) {
--
2.1.4
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] show-branch --upstream: add upstream branches to the list of branches to display
`git show-branch` is a useful tool to display topics, but when you have
several local topic branches based on different upstream branches, it
can get cumbersome to use the right upstream branch with the right set
of topic branches.
The --upstream flag automatically adds the upstream branch for every
topic branch given, such that:
`git show-branch --upstream` is equivalent to `git show-branch
$(git for-each-ref refs/heads --format '%(refname:short)')
$(git for-each-ref refs/heads --format '%(upstream:short)')`
`git show-branch --upstream foo bar` is equivalent to `git show-branch
foo bar $(git for-each-ref refs/heads/foo refs/heads/bar
--format '%(upstream:short)')`
Furthermore, the --topics argument only takes one upstream ref. However,
when combined with --upstream, all the upstream branches are considered,
and show-branch only shows commits that are NOT on ANY of those upstream
branches.
Signed-off-by: Mike Hommey m...@glandium.org
---
Refreshed against current next.
Documentation/git-show-branch.txt | 6 ++
builtin/show-branch.c | 44 ---
2 files changed, 47 insertions(+), 3 deletions(-)
diff --git a/Documentation/git-show-branch.txt
b/Documentation/git-show-branch.txt
index b91d4e5..fd29c8d 100644
--- a/Documentation/git-show-branch.txt
+++ b/Documentation/git-show-branch.txt
@@ -53,6 +53,10 @@ OPTIONS
branch to the list of revs to be shown when it is not
given on the command line.
+--upstream::
+ With this option, the command includes the upstream
+ branch of each rev to be shown.
+
--topo-order::
By default, the branches and their commits are shown in
reverse chronological order. This option makes them
@@ -102,6 +106,8 @@ OPTIONS
--topics::
Shows only commits that are NOT on the first branch given.
+ When used with `--upstream`, shows only commits that are NOT
+ on any upstream branch.
This helps track topic branches by hiding any commit that
is already in the main line of development. When given
git show-branch --topics master topic1 topic2, this
diff --git a/builtin/show-branch.c b/builtin/show-branch.c
index f3fb5fb..140e88c 100644
--- a/builtin/show-branch.c
+++ b/builtin/show-branch.c
@@ -4,11 +4,12 @@
#include builtin.h
#include color.h
#include parse-options.h
+#include remote.h
static const char* show_branch_usage[] = {
N_(git show-branch [-a | --all] [-r | --remotes] [--topo-order |
--date-order]\n
- [--current] [--color[=when] | --no-color]
[--sparse]\n
- [--more=n | --list | --independent | --merge-base]\n
+ [--current] [--upstream] [--color[=when] |
--no-color]\n
+ [--sparse] [--more=n | --list | --independent |
--merge-base]\n
[--no-name | --sha1-name] [--topics] [(rev |
glob)...]),
N_(git show-branch (-g | --reflog)[=n[,base]] [--list] [ref]),
NULL
@@ -643,6 +644,7 @@ int cmd_show_branch(int ac, const char **av, const char
*prefix)
int sha1_name = 0;
int shown_merge_point = 0;
int with_current_branch = 0;
+ int with_upstream_branches = 0;
int head_at = -1;
int topics = 0;
int dense = 1;
@@ -661,6 +663,8 @@ int cmd_show_branch(int ac, const char **av, const char
*prefix)
OPT_BOOL(0, no-name, no_name, N_(suppress naming strings)),
OPT_BOOL(0, current, with_current_branch,
N_(include the current branch)),
+ OPT_BOOL(0, upstream, with_upstream_branches,
+N_(include upstream branches)),
OPT_BOOL(0, sha1-name, sha1_name,
N_(name commits with their object names)),
OPT_BOOL(0, merge-base, merge_base,
@@ -851,7 +855,41 @@ int cmd_show_branch(int ac, const char **av, const char
*prefix)
if (commit-object.flags == flag)
commit_list_insert_by_date(commit, list);
rev[num_rev] = commit;
+
+ if (with_upstream_branches) {
+ unsigned char branch_sha1[20];
+ struct branch *branch;
+ int current_ref_name_cnt = ref_name_cnt;
+
+ /* If this ref is already marked as an upstream, skip */
+ if (topics flag)
+ continue;
+
+ branch = branch_get(ref_name[num_rev]);
+
+ if (!branch || !branch-merge || !branch-merge[0] ||
+ !branch-merge[0]-dst)
+ continue;
+ if (get_sha1(branch-merge[0]-dst, branch_sha1))
+ continue;
+ append_remote_ref(branch-merge[0]-dst, branch_sha1,
0, 0);
+ /* If append_remote_ref didn't add a
