Re: [RFC/PATCH] avoid SIGPIPE warnings for aliases
Am 04.01.2013 13:47, schrieb Jeff King: I have two reservations with this patch: 1. We are ignoring SIGPIPE all the time. For an alias that is calling log, that is fine. But if pack-objects dies on the server side, seeing that it died from SIGPIPE is useful data, and we are squelching that. Maybe callers of run-command should have to pass an ignore SIGPIPE flag? I am of two minds. On the one hand, losing useful debugging information is not something we should do lightly. On the other hand, the message is really noise most of the time, even on servers: when pack-objects dies on the server side, it is most likely due to a connection that breaks (voluntarily or involuntarily) half-way during a transfer, and is presumably a frequent event, and as such not worth noting most of the time. 2. The die_errno in handle_alias is definitely wrong. Even if we want to print a message for signal death, showing errno is bogus unless the return value was -1. But is it the right thing to just pass the negative value straight to exit()? It works, but it is depending on the fact that (unsigned char)(ret 0xff) behaves in a certain way (i.e., that we are on a twos-complement platform, and -13 becomes 141). That is not strictly portable, but it is probably fine in practice. I'd worry more about exit() doing something weird on Windows. It did something weird on Windows until we added this line to compat/mingw.h: #define exit(code) exit((code) 0xff) -- 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: What's cooking in git.git (Jan 2013, #02; Thu, 3)
On Thu, Jan 3, 2013 at 7:17 PM, Junio C Hamano gits...@pobox.com wrote: * as/check-ignore (2012-12-28) 19 commits - Add git-check-ignore sub-command - setup.c: document get_pathspec() - pathspec.c: extract new validate_path() for reuse - pathspec.c: move reusable code from builtin/add.c - add.c: remove unused argument from validate_pathspec() - add.c: refactor treat_gitlinks() - dir.c: provide clear_directory() for reclaiming dir_struct memory - dir.c: keep track of where patterns came from - dir.c: use a single struct exclude_list per source of excludes - dir.c: rename free_excludes() to clear_exclude_list() - dir.c: refactor is_path_excluded() - dir.c: refactor is_excluded() - dir.c: refactor is_excluded_from_list() - dir.c: rename excluded() to is_excluded() - dir.c: rename excluded_from_list() to is_excluded_from_list() - dir.c: rename path_excluded() to is_path_excluded() - dir.c: rename cryptic 'which' variable to more consistent name - Improve documentation and comments regarding directory traversal API - api-directory-listing.txt: update to match code Rerolled. The early parts looked mostly fine; we may want to split this into two topics and have the early half graduate sooner. Sounds good to me. As already mentioned, I have the v4 series ready and it addresses all issues already voiced in v3, but I have postponed submitting it as per your request. Please let me know when and how to proceed, 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: Lockless Refs? (Was [PATCH] refs: do not use cached refs in repack_without_ref)
From: Martin Fick Sent: Thursday, January 03, 2013 6:53 PM Any thoughts on this idea? Is it flawed? I am trying to write it up in a more formal generalized manner and was hoping to get at least one it seems sane before I do. If you are assuming that atomic renames, etc. are available, then you should identify a test case and a degrade operation path when it is not available. Thanks, -Martin On Monday, December 31, 2012 03:30:53 am Martin Fick wrote: On Thursday, December 27, 2012 04:11:51 pm Martin Fick wrote: It concerns me that git uses any locking at all, even for refs since it has the potential to leave around stale locks. ... [a previous not so great attempt to fix this] ... I may have finally figured out a working loose ref update mechanism which I think can avoid stale locks. Unfortunately it requires atomic directory renames and universally unique identifiers (uuids). These may be no-go criteria? But I figure it is worth at least exploring this idea because of the potential benefits? The general approach is to setup a transaction and either commit or abort it. A transaction can be setup by renaming an appropriately setup directory to the ref.lock name. If the rename succeeds, the transaction is begun. Any actor can abort the transaction (up until it is committed) by simply deleting the ref.lock directory, so it is not at risk of going stale. However, once the actor who sets up the transaction commits it, deleting the ref.lock directory simply aids in cleaning it up for the next transaction (instead of aborting it). One important piece of the transaction is the use of uuids. The uuids provide a mechanism to tie the atomic commit pieces to the transactions and thus to prevent long sleeping process from inadvertently performing actions which could be out of date when they wake finally up. In each case, the atomic commit piece is the renaming of a file. For the create and update pieces, a file is renamed from the ref.lock dir to the ref file resulting in an update to the sha for the ref. However, in the delete case, the ref file is instead renamed to end up in the ref.lock directory resulting in a delete of the ref. This scheme does not affect the way refs are read today, To prepare for a transaction, an actor first generates a uuid (an exercise I will delay for now). Next, a tmp directory named after the uuid is generated in the parent directory for the ref to be updated, perhaps something like: .lock_uuid. In this directory is places either a file or a directory named after the uuid, something like: .lock_uuid/,uuid. In the case of a create or an update, the new sha is written to this file. In the case of a delete, it is a directory. Once the tmp directory is setup, the initiating actor attempts to start the transaction by renaming the tmp directory to ref.lock. If the rename fails, the update fails. If the rename succeeds, the actor can then attempt to commit the transaction (before another actor aborts it). In the case of a create, the actor verifies that ref does not currently exist, and then renames the now named ref.lock/uuid file to ref. On success, the ref was created. In the case of an update, the actor verifies that ref currently contains the old sha, and then also renames the now named ref.lock/uuid file to ref. On success, the ref was updated. In the case of a delete, the actor may verify that ref currently contains the sha to prune if it needs to, and then renames the ref file to ref.lock/uuid/delete. On success, the ref was deleted. Whether successful or not, the actor may now simply delete the ref.lock directory, clearing the way for a new transaction. Any other actor may delete this directory at any time also, likely either on conflict (if they are attempting to initiate a transaction), or after a grace period just to cleanup the FS. Any actor may also safely cleanup the tmp directories, preferably also after a grace period. One neat part about this scheme is that I believe it would be backwards compatible with the current locking mechanism since the transaction directory will simply appear to be a lock to older clients. And the old lock file should continue to lock out these newer transactions. Due to this backwards compatibility, I believe that this could be incrementally employed today without affecting very much. It could be deployed in place of any updates which only hold ref.locks to update the loose ref. So for example I think it could replace step 4a below from Michael Haggerty's description of today's loose ref pruning during ref packing: * Pack references: ... 4. prune_refs(): for each ref in the ref_to_prune list, call prune_ref(): a. Lock the reference using lock_ref_sha1(), verifying that the recorded
Re: Lockless Refs? (Was [PATCH] refs: do not use cached refs in repack_without_ref)
On Friday, January 04, 2013 10:52:43 am Pyeron, Jason J CTR (US) wrote: From: Martin Fick Sent: Thursday, January 03, 2013 6:53 PM Any thoughts on this idea? Is it flawed? I am trying to write it up in a more formal generalized manner and was hoping to get at least one it seems sane before I do. If you are assuming that atomic renames, etc. are available, then you should identify a test case and a degrade operation path when it is not available. Thanks, sound reasonable. Where you thinking a runtime test case that would be run before every transaction? I was anticipating a per repo config option called something like core.locks = recoverable that would be needed to turn them on? I was thinking that this was something that server sites could test in advance on their repos and then enable it for them. Maybe a git- lock tool with a --test-recoverable option? -Martin On Monday, December 31, 2012 03:30:53 am Martin Fick wrote: On Thursday, December 27, 2012 04:11:51 pm Martin Fick wrote: It concerns me that git uses any locking at all, even for refs since it has the potential to leave around stale locks. ... [a previous not so great attempt to fix this] ... I may have finally figured out a working loose ref update mechanism which I think can avoid stale locks. Unfortunately it requires atomic directory renames and universally unique identifiers (uuids). These may be no-go criteria? But I figure it is worth at least exploring this idea because of the potential benefits? The general approach is to setup a transaction and either commit or abort it. A transaction can be setup by renaming an appropriately setup directory to the ref.lock name. If the rename succeeds, the transaction is begun. Any actor can abort the transaction (up until it is committed) by simply deleting the ref.lock directory, so it is not at risk of going stale. However, once the actor who sets up the transaction commits it, deleting the ref.lock directory simply aids in cleaning it up for the next transaction (instead of aborting it). One important piece of the transaction is the use of uuids. The uuids provide a mechanism to tie the atomic commit pieces to the transactions and thus to prevent long sleeping process from inadvertently performing actions which could be out of date when they wake finally up. In each case, the atomic commit piece is the renaming of a file. For the create and update pieces, a file is renamed from the ref.lock dir to the ref file resulting in an update to the sha for the ref. However, in the delete case, the ref file is instead renamed to end up in the ref.lock directory resulting in a delete of the ref. This scheme does not affect the way refs are read today, To prepare for a transaction, an actor first generates a uuid (an exercise I will delay for now). Next, a tmp directory named after the uuid is generated in the parent directory for the ref to be updated, perhaps something like: .lock_uuid. In this directory is places either a file or a directory named after the uuid, something like: .lock_uuid/,uuid. In the case of a create or an update, the new sha is written to this file. In the case of a delete, it is a directory. Once the tmp directory is setup, the initiating actor attempts to start the transaction by renaming the tmp directory to ref.lock. If the rename fails, the update fails. If the rename succeeds, the actor can then attempt to commit the transaction (before another actor aborts it). In the case of a create, the actor verifies that ref does not currently exist, and then renames the now named ref.lock/uuid file to ref. On success, the ref was created. In the case of an update, the actor verifies that ref currently contains the old sha, and then also renames the now named ref.lock/uuid file to ref. On success, the ref was updated. In the case of a delete, the actor may verify that ref currently contains the sha to prune if it needs to, and then renames the ref file to ref.lock/uuid/delete. On success, the ref was deleted. Whether successful or not, the actor may now simply delete the ref.lock directory, clearing the way for a new transaction. Any other actor may delete this directory at any time also, likely either on conflict (if they are attempting to initiate a transaction), or after a grace period just to cleanup the FS. Any actor may also safely cleanup the tmp directories, preferably also after a grace period. One neat part about this scheme is that I believe it would be backwards compatible with the current locking mechanism since the transaction directory will simply appear to be a lock to older clients. And the old
Re: Proposal for git stash rename
Greg Hewgill greg at hewgill.com writes: On Sun, Jun 20, 2010 at 10:54:43AM +, ??var Arnfj??r?? Bjarmason wrote: It's good to post a WIP PATCH even if it needs cleanup, just as a point for further discussion. Thanks, point taken. WIP patch follows. This patch implements a git stash rename using a new git reflog update command that updates the message associated with a reflog entry. --- [--snip--] Hi, I note that this proposal is now two years old. A work in progress patch was requested, however, after one was given this thread ended. I'm also finding a need for this feature; Not to try and bump an old thread, but what's the best way to land this? – Micheil Smith @miksago -- To unsubscribe from this list: send the line 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] wincred: improve compatibility with windows versions
On WinXP, the windows credential helper doesn't work at all (due to missing
Cred[Un]PackAuthenticationBuffer APIs). On Win7, the credential format used
by wincred is incompatible with native Windows tools (such as the control
panel applet or 'cmdkey.exe /generic'). These Windows tools only set the
TargetName, UserName and CredentialBlob members of the CREDENTIAL
structure (where CredentialBlob is the UTF-16-encoded password).
Remove the unnecessary packing / unpacking of the password, along with the
related API definitions, for compatibility with Windows XP.
Don't use CREDENTIAL_ATTRIBUTEs to identify credentials for compatibility
with Windows credential manager tools. Parse the protocol, username, host
and path fields from the credential's target name instead.
While we're at it, optionally accept CRLF (instead of LF only) to simplify
debugging in bash / cmd.
Signed-off-by: Karsten Blees bl...@dcon.de
---
Hi,
I tried the windows credential helper yesterday and found that it doesn't work
on XP at all, and doesn't work properly in combination with Win7 credential
manager tools. So here's a patch that reduces it to the functionality provided
/ expected by Windows.
@Erik, Jeff: Please let me know if I'm missing something and the use of
Cred[Un]PackAuthenticationBuffer or CREDENTIAL_ATTRIBUTEs actually served a
useful purpose.
Cheers,
Karsten
Also available here:
https://github.com/kblees/git/tree/kb/improve-wincred-compatibility
git pull git://github.com/kblees/git.git kb/improve-wincred-compatibility
---
.../credential/wincred/git-credential-wincred.c| 179 ++---
1 file changed, 53 insertions(+), 126 deletions(-)
diff --git a/contrib/credential/wincred/git-credential-wincred.c
b/contrib/credential/wincred/git-credential-wincred.c
index cbaec5f..3464080 100644
--- a/contrib/credential/wincred/git-credential-wincred.c
+++ b/contrib/credential/wincred/git-credential-wincred.c
@@ -9,6 +9,8 @@
/* common helpers */
+#define ARRAY_SIZE(x) (sizeof(x)/sizeof(x[0]))
+
static void die(const char *err, ...)
{
char msg[4096];
@@ -30,14 +32,6 @@ static void *xmalloc(size_t size)
return ret;
}
-static char *xstrdup(const char *str)
-{
- char *ret = strdup(str);
- if (!ret)
- die(Out of memory);
- return ret;
-}
-
/* MinGW doesn't have wincred.h, so we need to define stuff */
typedef struct _CREDENTIAL_ATTRIBUTEW {
@@ -67,20 +61,14 @@ typedef struct _CREDENTIALW {
#define CRED_MAX_ATTRIBUTES 64
typedef BOOL (WINAPI *CredWriteWT)(PCREDENTIALW, DWORD);
-typedef BOOL (WINAPI *CredUnPackAuthenticationBufferWT)(DWORD, PVOID, DWORD,
-LPWSTR, DWORD *, LPWSTR, DWORD *, LPWSTR, DWORD *);
typedef BOOL (WINAPI *CredEnumerateWT)(LPCWSTR, DWORD, DWORD *,
PCREDENTIALW **);
-typedef BOOL (WINAPI *CredPackAuthenticationBufferWT)(DWORD, LPWSTR, LPWSTR,
-PBYTE, DWORD *);
typedef VOID (WINAPI *CredFreeT)(PVOID);
typedef BOOL (WINAPI *CredDeleteWT)(LPCWSTR, DWORD, DWORD);
static HMODULE advapi, credui;
static CredWriteWT CredWriteW;
-static CredUnPackAuthenticationBufferWT CredUnPackAuthenticationBufferW;
static CredEnumerateWT CredEnumerateW;
-static CredPackAuthenticationBufferWT CredPackAuthenticationBufferW;
static CredFreeT CredFree;
static CredDeleteWT CredDeleteW;
@@ -88,74 +76,64 @@ static void load_cred_funcs(void)
{
/* load DLLs */
advapi = LoadLibrary(advapi32.dll);
- credui = LoadLibrary(credui.dll);
- if (!advapi || !credui)
+ if (!advapi)
die(failed to load DLLs);
/* get function pointers */
CredWriteW = (CredWriteWT)GetProcAddress(advapi, CredWriteW);
- CredUnPackAuthenticationBufferW = (CredUnPackAuthenticationBufferWT)
- GetProcAddress(credui, CredUnPackAuthenticationBufferW);
CredEnumerateW = (CredEnumerateWT)GetProcAddress(advapi,
CredEnumerateW);
- CredPackAuthenticationBufferW = (CredPackAuthenticationBufferWT)
- GetProcAddress(credui, CredPackAuthenticationBufferW);
CredFree = (CredFreeT)GetProcAddress(advapi, CredFree);
CredDeleteW = (CredDeleteWT)GetProcAddress(advapi, CredDeleteW);
- if (!CredWriteW || !CredUnPackAuthenticationBufferW ||
- !CredEnumerateW || !CredPackAuthenticationBufferW || !CredFree ||
- !CredDeleteW)
+ if (!CredWriteW || !CredEnumerateW || !CredFree || !CredDeleteW)
die(failed to load functions);
}
-static char target_buf[1024];
-static char *protocol, *host, *path, *username;
-static WCHAR *wusername, *password, *target;
+static WCHAR *wusername, *password, *protocol, *host, *path, target[1024];
-static void write_item(const char *what, WCHAR *wbuf)
+static void write_item(const char *what, LPCWSTR wbuf, int wlen)
{
char *buf;
- int len = WideCharToMultiByte(CP_UTF8, 0, wbuf, -1, NULL, 0, NULL,
+ int len = WideCharToMultiByte(CP_UTF8, 0, wbuf, wlen, NULL, 0, NULL,
[BUG] git submodule update is not fail safe
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 Hi. My network connection at times is rather unstable, so I can experience all sort of network problems. Today I tried to clone the qemu repository, and then to update all submodules. I'm using git from a recent master (790c83 - 14 December). This is what happened: $ git submodule update --init pixman Submodule 'pixman' (git://anongit.freedesktop.org/pixman) registered for path 'pixman' Cloning into 'pixman'... fatal: Unable to look up anongit.freedesktop.org (port 9418) (Name or service not known) Clone of 'git://anongit.freedesktop.org/pixman' into submodule path 'pixman' failed $ git submodule update --init Submodule 'roms/SLOF' (git://git.qemu.org/SLOF.git) registered for path 'roms/SLOF' Submodule 'roms/ipxe' (git://git.qemu.org/ipxe.git) registered for path 'roms/ipxe' Submodule 'roms/openbios' (git://git.qemu.org/openbios.git) registered for path 'roms/openbios' Submodule 'roms/qemu-palcode' (git://repo.or.cz/qemu-palcode.git) registered for path 'roms/qemu-palcode' Submodule 'roms/seabios' (git://git.qemu.org/seabios.git/) registered for path 'roms/seabios' Submodule 'roms/sgabios' (git://git.qemu.org/sgabios.git) registered for path 'roms/sgabios' Submodule 'roms/vgabios' (git://git.qemu.org/vgabios.git/) registered for path 'roms/vgabios' fatal: unable to connect to anongit.freedesktop.org: anongit.freedesktop.org[0: 131.252.210.161]: errno=Connection timed out Unable to fetch in submodule path 'pixman' $ git submodule update --init fatal: Needed a single revision Unable to find current revision in submodule path 'pixman' The problem is easy to solve: manually remove the pixman directory; however IMHO git submodule update should not fail this way since it may confuse the user. I'm sorry for not sending a patch. Regards Manlio -BEGIN PGP SIGNATURE- Version: GnuPG v1.4.10 (GNU/Linux) Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/ iEYEARECAAYFAlDnQUUACgkQscQJ24LbaUSgVACglJjFxB51VINOCe9Z39/XEEUH 6+QAnAwdQerBSjVSS1/3eNXSBfnR0yba =eOJT -END PGP SIGNATURE- -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 11/19] dir.c: use a single struct exclude_list per source of excludes
Adam Spiers g...@adamspiers.org writes:
diff --git a/builtin/clean.c b/builtin/clean.c
index 0c7b3d0..bd18b88 100644
--- a/builtin/clean.c
+++ b/builtin/clean.c
@@ -97,9 +97,10 @@ int cmd_clean(int argc, const char **argv, const char
*prefix)
if (!ignored)
setup_standard_excludes(dir);
+ add_exclude_list(dir, EXC_CMDL);
for (i = 0; i exclude_list.nr; i++)
add_exclude(exclude_list.items[i].string, , 0,
- dir.exclude_list[EXC_CMDL]);
+ dir.exclude_list_groups[EXC_CMDL].ary[0]);
This looks somewhat ugly for two reasons.
* The abstraction add_exclude() offers to its callers is just to
let them add one pattern to the list of patterns for the kind
(here, EXC_CMDL); why should they care about .ary[0] part? Are
there cases any sane caller (not the implementation of the
exclude_list_group machinery e.g. add_excludes_from_... function)
may want to call it with .ary[1]? I do not think of any.
Shouldn't the public API function add_exclude() take a pointer to
the list group itself?
* When naming an array of things, we tend to prefer naming it
type thing[count]
so that the second element can be called thing[2] and not
things[2]. dir.exclude_list_group[EXC_CMDL] reads beter.
diff --git a/builtin/ls-files.c b/builtin/ls-files.c
index ef7f99a..c448e06 100644
--- a/builtin/ls-files.c
+++ b/builtin/ls-files.c
@@ -420,10 +420,10 @@ static int option_parse_z(const struct option *opt,
static int option_parse_exclude(const struct option *opt,
const char *arg, int unset)
{
- struct exclude_list *list = opt-value;
+ struct exclude_list_group *group = opt-value;
exc_given = 1;
- add_exclude(arg, , 0, list);
+ add_exclude(arg, , 0, group-ary[0]);
This is another example where the caller would wish to be able to say
add_exclude(arg, , 0, group);
instead.
--
To unsubscribe from this list: send the line 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: as/check-ignore (was Re: What's cooking in git.git (Jan 2013, #02; Thu, 3))
Adam Spiers g...@adamspiers.org writes: On Thu, Jan 3, 2013 at 7:17 PM, Junio C Hamano gits...@pobox.com wrote: * as/check-ignore (2012-12-28) 19 commits - Add git-check-ignore sub-command - setup.c: document get_pathspec() - pathspec.c: extract new validate_path() for reuse - pathspec.c: move reusable code from builtin/add.c - add.c: remove unused argument from validate_pathspec() - add.c: refactor treat_gitlinks() - dir.c: provide clear_directory() for reclaiming dir_struct memory - dir.c: keep track of where patterns came from - dir.c: use a single struct exclude_list per source of excludes - dir.c: rename free_excludes() to clear_exclude_list() - dir.c: refactor is_path_excluded() - dir.c: refactor is_excluded() - dir.c: refactor is_excluded_from_list() - dir.c: rename excluded() to is_excluded() - dir.c: rename excluded_from_list() to is_excluded_from_list() - dir.c: rename path_excluded() to is_path_excluded() - dir.c: rename cryptic 'which' variable to more consistent name - Improve documentation and comments regarding directory traversal API - api-directory-listing.txt: update to match code Rerolled. The early parts looked mostly fine; we may want to split this into two topics and have the early half graduate sooner. Sounds good to me. As already mentioned, I have the v4 series ready and it addresses all issues already voiced in v3, but I have postponed submitting it as per your request. Please let me know when and how to proceed, thanks! I was planning to add a new as/dir-c-cleanup topic that leads to f619881 (dir.c: rename free_excludes() to clear_exclude_list(), 2012-12-27), and leave the remainder in this series. I think the earlier parts of this series up to that point should go 'next' now. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH v4] git-completion.bash: add support for path completion
-Original Message-
From: git-ow...@vger.kernel.org
[mailto:git-ow...@vger.kernel.org] On Behalf Of Manlio Perillo
Sent: Friday, December 21, 2012 11:55 AM
To: git@vger.kernel.org
Cc: sze...@ira.uka.de; felipe.contre...@gmail.com; Manlio Perillo
Subject: [PATCH v4] git-completion.bash: add support for path
completion
The git-completion.bash script did not implemented full, git aware,
support to complete paths, for git commands that operate on
files within
the current working directory or the index.
I think this is a great improvement! Very nice.
I've been playing with it but I'm not getting the expected
behavior when I cd to a sub-directory. Instead I get all files
in the repo as proposals. I'm trying it in the git git-repository
itself. Here is a sample:
git status
# On branch pu
nothing to commit, working directory clean
source contrib/completion/git-completion.bash
touch contrib/test1
touch contrib/test2
git status
# On branch pu
# Untracked files:
# (use git add file... to include in what will be committed)
#
# contrib/test1
# contrib/test2
nothing added to commit but untracked files present (use git add to track)
git add TAB # this works as expected and I get: contrib/test1
contrib/test2
cd contrib/
git add TAB
Display all 387 possibilities? (y or n) # That is not right. Shouldn't I get
# the same two files only?
Maybe I mis-understood what should happen?
Besides that, without looking at the patch in detail, I put just a couple
of minor points below.
As an example:
git add TAB
will suggest all files in the current working directory, including
ignored files and files that have not been modified.
Support path completion, for git commands where the
non-option arguments
always refer to paths within the current working directory or
the index, as the follow:
s/as the follow/as follows/
* the path completion for the git rm and git ls-files
commands will suggest all cached files.
* the path completion for the git add command will suggest all
untracked and modified files. Ignored files are excluded.
* the path completion for the git clean command will suggest all
untracked files. Ignored files are excluded.
* the path completion for the git mv command will suggest all cached
files when expanding the first argument, and all untracked
and cached
files for subsequent arguments. In the latter case, empty
directories
are included and ignored files are excluded.
* the path completion for the git commit command will suggest all
files that have been modified from the HEAD, if HEAD
exists, otherwise
it will suggest all cached files.
For all affected commands, completion will always stop at directory
boundary. Only standard ignored files are excluded, using the
--exclude-standard option of the ls-files command.
Signed-off-by: Manlio Perillo manlio.peri...@gmail.com
---
Changes from version 3:
* Fixed quoting issues
* Fixed default parameters handling
* Fixed a typo in the commit message: the affected
command was ls-files,
not ls-tree.
* Fixed incorrect behavior when expanding a path in git commit
command, for a newly created repository (when HEAD does not
exists).
* Make sure to always execute git commands with stderr
redirected to
/dev/null.
* Improved path completion for the git mv command.
This required a small refactorization of the __git_index_files
function, in order to support multiple options for ls-files.
contrib/completion/git-completion.bash | 140
+
1 file changed, 124 insertions(+), 16 deletions(-)
diff --git a/contrib/completion/git-completion.bash
b/contrib/completion/git-completion.bash
index 0b77eb1..c8c6464 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -13,6 +13,7 @@
#*) .git/remotes file names
#*) git 'subcommands'
#*) tree paths within 'ref:path/to/file' expressions
+#*) file paths within current working directory and index
#*) common --long-options
#
# To use these routines:
@@ -233,6 +234,62 @@ __gitcomp_nl ()
COMPREPLY=($(compgen -P ${2-} -S ${4- } -W $1 --
${3-$cur}))
}
+# Process path list returned by ls-files and diff-index
--name-only
+# commands, in order to list only file names relative to a specified
+# directory, and append a slash to directory names.
+# It accepts 1 optional argument: a directory path. The
path must have
+# a trailing slash.
+__git_index_file_list_filter ()
+{
+ local pfx=${1-} offset=${#pfx} path
+
+ while read -r path; do
+ path=${path:$offset}
+
+ case $path in
+ ?*/*) echo ${path%%/*}/ ;;
+ *) echo $path ;;
+ esac
+ done
+}
+
+#
Re: [RFC/PATCH] avoid SIGPIPE warnings for aliases
On Fri, Jan 04, 2013 at 05:55:18PM +0100, Johannes Sixt wrote: Am 04.01.2013 13:47, schrieb Jeff King: I have two reservations with this patch: 1. We are ignoring SIGPIPE all the time. For an alias that is calling log, that is fine. But if pack-objects dies on the server side, seeing that it died from SIGPIPE is useful data, and we are squelching that. Maybe callers of run-command should have to pass an ignore SIGPIPE flag? I am of two minds. On the one hand, losing useful debugging information is not something we should do lightly. On the other hand, the message is really noise most of the time, even on servers: when pack-objects dies on the server side, it is most likely due to a connection that breaks (voluntarily or involuntarily) half-way during a transfer, and is presumably a frequent event, and as such not worth noting most of the time. Yeah. I'd mostly be worried about a case where pack-objects prints nothing (because it dies due to pipe), and then the outer process is not sufficiently verbose (it just says something like pack-objects died abnormally, and the user is left scratching their head. I.e., it _is_ uninteresting, but because we are too silent, the user does not even know it is uninteresting. Pack-objects is already careful to check all of its writes. I really think it would be fine to just ignore SIGPIPE, and then it would produce a useful error message on EPIPE. The downside is that if we accidentally have an unchecked call, we won't notice the error (we'll probably notice it later, but we might continue uselessly spewing data in the meantime). Perhaps we should catch SIGPIPE in such programs and print an error message. 2. The die_errno in handle_alias is definitely wrong. Even if we want to print a message for signal death, showing errno is bogus unless the return value was -1. But is it the right thing to just pass the negative value straight to exit()? It works, but it is depending on the fact that (unsigned char)(ret 0xff) behaves in a certain way (i.e., that we are on a twos-complement platform, and -13 becomes 141). That is not strictly portable, but it is probably fine in practice. I'd worry more about exit() doing something weird on Windows. It did something weird on Windows until we added this line to compat/mingw.h: #define exit(code) exit((code) 0xff) Ah, makes sense. I think that hunk of my patch is probably good, then. -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: Lockless Refs?
Martin Fick mf...@codeaurora.org writes: Any thoughts on this idea? Is it flawed? I am trying to write it up in a more formal generalized manner and was hoping to get at least one it seems sane before I do. The general impression I have been getting was that this isn't even worth the effort and the resulting complexity of the code, given Peff's observations earlier in the thread that ref update conflicts and leftover locks are reasonably rare in practice. But perhaps I has been mis-reading the discussion. I also have this suspicion that if you really want to shoot for multi-repository transactions in an massively scaled repository hosting environment, you would rather want to not rely on hacks based on filesystem semantics, but instead want to RPC with a dedicated ref management service that knows the transaction semantics you want, but that could become a much larger change. I dunno. -- To unsubscribe from this list: send the line 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: Proposal for git stash rename
Micheil Smith mich...@brandedcode.com writes: This patch implements a git stash rename using a new git reflog update command that updates the message associated with a reflog entry. ... I note that this proposal is now two years old. A work in progress patch was requested, however, after one was given this thread ended. I'm also finding a need for this feature; The whole point of reflog is that it is a mechanism to let users to go safely back to the previous state, by using a file that is pretty much append-only. It feels that a mechanism to rewrite one goes completely against that principle, at least to me. I have a feeling that need in need for this feature is a misspelt want, that occasional misspelling of the stash message may give users awkward feelings when viewing git stash list output but not severe enough to make them unable to identify which stash entry holds which change, and that it is sufficient to pop and then restash if a user *really* cares. -- To unsubscribe from this list: send the line 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] SubmittingPatches: Document how to request a patch review tag
Jason Holden jason.k.holden.sw...@gmail.com writes: A very similiar question was asked previously in: http://thread.gmane.org/gmane.comp.version-control.git/185564/focus=185570 Reviewed-by is for those who are familiar with the part of the system being touched to say I reviewed this patch, it looks good, and Michael indeed was involved in recent updates to the refs.c infrastructure, so as he said in his message it looks like I should, it was the right thing to do. I do not think Michael was asking if that was the standard _thing_ to do; I think the question was if there was a standard _way_ (perhaps a tool) to send such a Reviewed-by: line. This will apply on top of your last tweak to SubmittingPatches Please add my reviewed-by to the rest of the patches in this series. I do not think you own anyting in SubmittingPatches document, though; at least not yet. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [BUG] git submodule update is not fail safe
Manlio Perillo manlio.peri...@gmail.com writes: $ git submodule update --init ... Submodule 'roms/vgabios' (git://git.qemu.org/vgabios.git/) registered for path 'roms/vgabios' fatal: unable to connect to anongit.freedesktop.org: anongit.freedesktop.org[0: 131.252.210.161]: errno=Connection timed out Unable to fetch in submodule path 'pixman' $ git submodule update --init fatal: Needed a single revision Unable to find current revision in submodule path 'pixman' The problem is easy to solve: manually remove the pixman directory; however IMHO git submodule update should not fail this way since it may confuse the user. Sounds like a reasonable observation. Jens, Heiko, comments? -- To unsubscribe from this list: send the line 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] wincred: improve compatibility with windows versions
On Fri, Jan 4, 2013 at 9:28 PM, Karsten Blees karsten.bl...@gmail.com wrote:
On WinXP, the windows credential helper doesn't work at all (due to missing
Cred[Un]PackAuthenticationBuffer APIs). On Win7, the credential format used
by wincred is incompatible with native Windows tools (such as the control
panel applet or 'cmdkey.exe /generic'). These Windows tools only set the
TargetName, UserName and CredentialBlob members of the CREDENTIAL
structure (where CredentialBlob is the UTF-16-encoded password).
Remove the unnecessary packing / unpacking of the password, along with the
related API definitions, for compatibility with Windows XP.
Don't use CREDENTIAL_ATTRIBUTEs to identify credentials for compatibility
with Windows credential manager tools. Parse the protocol, username, host
and path fields from the credential's target name instead.
While we're at it, optionally accept CRLF (instead of LF only) to simplify
debugging in bash / cmd.
Signed-off-by: Karsten Blees bl...@dcon.de
---
Hi,
I tried the windows credential helper yesterday and found that it doesn't
work on XP at all, and doesn't work properly in combination with Win7
credential manager tools. So here's a patch that reduces it to the
functionality provided / expected by Windows.
@Erik, Jeff: Please let me know if I'm missing something and the use of
Cred[Un]PackAuthenticationBuffer or CREDENTIAL_ATTRIBUTEs actually served a
useful purpose.
The only reason why I used Cred[Un]PackAuthenticationBuffer, were that
I wasn't aware that it was possible any other way. I didn't even know
there was a Windows Credential Manager in Windows XP.
The credential attributes were because they were convenient, and I'm
not sure I understand what you mean about the Win7 credential manager
tools. I did test my code with it - in fact, it was a very useful tool
for debugging the helper.
Are you referring to the credentials not *looking* like normal
HTTP-credentials? If so, I simply didn't see that as a goal. Why would
it be? Compatibility with IE? We already lose that with our git:
prefix in the target, no? Perhaps we can lose the git:-prefix, and
gain IE-compatibility when the protocol matches?
But, if we do any of these changes, does this mean I will lose my
existing credentials? It's probably not a big deal, but it's worth a
mention, isn't it?
@@ -67,20 +61,14 @@ typedef struct _CREDENTIALW {
#define CRED_MAX_ATTRIBUTES 64
typedef BOOL (WINAPI *CredWriteWT)(PCREDENTIALW, DWORD);
-typedef BOOL (WINAPI *CredUnPackAuthenticationBufferWT)(DWORD, PVOID, DWORD,
-LPWSTR, DWORD *, LPWSTR, DWORD *, LPWSTR, DWORD *);
typedef BOOL (WINAPI *CredEnumerateWT)(LPCWSTR, DWORD, DWORD *,
PCREDENTIALW **);
-typedef BOOL (WINAPI *CredPackAuthenticationBufferWT)(DWORD, LPWSTR, LPWSTR,
-PBYTE, DWORD *);
typedef VOID (WINAPI *CredFreeT)(PVOID);
typedef BOOL (WINAPI *CredDeleteWT)(LPCWSTR, DWORD, DWORD);
static HMODULE advapi, credui;
Perhaps get rid of credui also?
static CredWriteWT CredWriteW;
-static CredUnPackAuthenticationBufferWT CredUnPackAuthenticationBufferW;
static CredEnumerateWT CredEnumerateW;
-static CredPackAuthenticationBufferWT CredPackAuthenticationBufferW;
static CredFreeT CredFree;
static CredDeleteWT CredDeleteW;
@@ -88,74 +76,64 @@ static void load_cred_funcs(void)
{
/* load DLLs */
advapi = LoadLibrary(advapi32.dll);
- credui = LoadLibrary(credui.dll);
- if (!advapi || !credui)
+ if (!advapi)
die(failed to load DLLs);
It's not multiple DLLs any more, so perhaps failed to load
advapi32.dll instead?
-static char target_buf[1024];
-static char *protocol, *host, *path, *username;
-static WCHAR *wusername, *password, *target;
+static WCHAR *wusername, *password, *protocol, *host, *path, target[1024];
-static void write_item(const char *what, WCHAR *wbuf)
+static void write_item(const char *what, LPCWSTR wbuf, int wlen)
{
char *buf;
- int len = WideCharToMultiByte(CP_UTF8, 0, wbuf, -1, NULL, 0, NULL,
+ int len = WideCharToMultiByte(CP_UTF8, 0, wbuf, wlen, NULL, 0, NULL,
FALSE);
buf = xmalloc(len);
- if (!WideCharToMultiByte(CP_UTF8, 0, wbuf, -1, buf, len, NULL, FALSE))
+ if (!WideCharToMultiByte(CP_UTF8, 0, wbuf, wlen, buf, len, NULL,
FALSE))
die(WideCharToMultiByte failed!);
printf(%s=, what);
- fwrite(buf, 1, len - 1, stdout);
+ fwrite(buf, 1, len, stdout);
putchar('\n');
free(buf);
}
When the password-blob is simply UTF-16 encoded, are the
zero-termination not included? That's the reason for this change,
right?
-static int match_attr(const CREDENTIALW *cred, const WCHAR *keyword,
-const char *want)
+static int match_part(LPCWSTR *ptarget, LPCWSTR want, LPCWSTR delim)
{
- int i;
- if (!want)
- return 1;
-
- for (i = 0; i cred-AttributeCount; ++i)
-
Re: [RFC/PATCH] avoid SIGPIPE warnings for aliases
Jeff King p...@peff.net writes: I have two reservations with this patch: 1. We are ignoring SIGPIPE all the time. For an alias that is calling log, that is fine. But if pack-objects dies on the server side, seeing that it died from SIGPIPE is useful data, and we are squelching that. Maybe callers of run-command should have to pass an ignore SIGPIPE flag? What should this do: GIT_PAGER='head -n 1' git -p -c alias.o='!cat longfile' o Should it behave just like cat longfile | head -n 1 or should it behave differently? I am having a feeling that whatever external command given as the value of alias.$cmd should choose what error status it wants to be reported. 2. The die_errno in handle_alias is definitely wrong. Even if we want to print a message for signal death, showing errno is bogus unless the return value was -1. But is it the right thing to just pass the negative value straight to exit()? It works, but it is depending on the fact that (unsigned char)(ret 0xff) behaves in a certain way (i.e., that we are on a twos-complement platform, and -13 becomes 141). Isn't that what POSIX.1 guarantees us, though? The value of status may be 0, EXIT_SUCCESS, EXIT_FAILURE, or any other value, though only the least significant 8 bits (that is, status 0377) shall be available to a waiting parent process. -- To unsubscribe from this list: send the line 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.wiki.kernel.org spam ...
2012/12/31 Johannes Schindelin johannes.schinde...@gmx.de: Hi Rupert, On Sat, 29 Dec 2012, rupert THURNER wrote: ich hab gesehen, du bist ober-meister des kernle.org git wikis. da gibt es ganz schön viel neue user und spam derzeit, zb: https://git.wiki.kernel.org/index.php?title=User_talk:Bridgetevans0521redirect=no möchtest du das erzeugen von user accounts erschweren, wie zb hier: http://kiwix.org/index.php/Special:UserLogin?type=signupreturnto=Main_Page/en falls du noch leute als admin haben willst, ich melde mich freiwillig, ThurnerRupert ist mein account. Since my trustworthiness was questioned, I have stopped completely with the maintenance of the Wiki. The mailing list (Cc:ed) may have additional comments. there are 3 admins: * https://git.wiki.kernel.org/index.php/Special:Contributions/KorgWikiSysop, last contribution january 2010 * https://git.wiki.kernel.org/index.php/Special:Contributions/Warthog9, last contribution august 2010 * https://git.wiki.kernel.org/index.php/Special:Contributions/Dscho, last contribution august 2010 and you were (by far) the most active. this leaves me a little confused. who would be then be responsible? who would be responsible for upgrading / installing anything at the wiki? rupert. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4] git-completion.bash: add support for path completion
Marc Khouzam marc.khou...@ericsson.com writes: I've been playing with it but I'm not getting the expected behavior when I cd to a sub-directory. Thanks for testing. Manlio? -- To unsubscribe from this list: send the line 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.wiki.kernel.org spam ...
Hi Rupert, On Sat, 5 Jan 2013, rupert THURNER wrote: 2012/12/31 Johannes Schindelin johannes.schinde...@gmx.de: there are 3 admins: * https://git.wiki.kernel.org/index.php/Special:Contributions/KorgWikiSysop, last contribution january 2010 * https://git.wiki.kernel.org/index.php/Special:Contributions/Warthog9, last contribution august 2010 * https://git.wiki.kernel.org/index.php/Special:Contributions/Dscho, last contribution august 2010 and you were (by far) the most active. I was. John Hawley trusted me when I asked for admin privileges to keep the spam at bay, but a very vocal voice on the mailing list tried to discredit my work, and in the wake of the ensuing mailing list thread I got the impression that that feeling was universal, so I abided and stopped. this leaves me a little confused. who would be then be responsible? who would be responsible for upgrading / installing anything at the wiki? That would be John Hawley. Ciao, Dscho -- To unsubscribe from this list: send the line 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] gitk: Display the date of a tag in a human friendly way.
By selecting a tag within gitk you can display information about it.
This information is output by using the command
'git cat-file tag tagid'
This outputs the *raw* information from the tag, amongst which is the
time - in seconds since the epoch. As useful as that value is, I find it
a lot easier to read and process time which it is something like:
Mon Dec 31 14:26:11 2012 -0800
This change will modify the display of tags in gitk like so:
@@ -1,7 +1,7 @@
object 5d417842efeafb6e109db7574196901c4e95d273
type commit
tag v1.8.1
-tagger Junio C Hamano gits...@pobox.com 1356992771 -0800
+tagger Junio C Hamano gits...@pobox.com Mon Dec 31 14:26:11 2012 -0800
Git 1.8.1
-BEGIN PGP SIGNATURE-
Signed-off-by: Anand Kumria wildf...@progsoc.org
---
gitk-git/gitk |2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/gitk-git/gitk b/gitk-git/gitk
index d93bd99..aae1c58 100755
--- a/gitk-git/gitk
+++ b/gitk-git/gitk
@@ -10675,7 +10675,7 @@ proc showtag {tag isnew} {
set linknum 0
if {![info exists cached_tagcontent($tag)]} {
catch {
- set cached_tagcontent($tag) [exec git cat-file tag $tag]
+ set cached_tagcontent($tag) [exec git cat-file -p $tag]
}
}
if {[info exists cached_tagcontent($tag)]} {
--
1.7.9.5
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: git.wiki.kernel.org spam ...
On Sat, Jan 05, 2013 at 12:27:12AM +0100, Johannes Schindelin wrote: I was. John Hawley trusted me when I asked for admin privileges to keep the spam at bay, but a very vocal voice on the mailing list tried to discredit my work, and in the wake of the ensuing mailing list thread I got the impression that that feeling was universal, so I abided and stopped. this leaves me a little confused. who would be then be responsible? who would be responsible for upgrading / installing anything at the wiki? That would be John Hawley. John is one of the Linux Foundation staff members that are responsible for the system administration of wiki.kernel.org (and kernel.org, and bugzilla.kernel.org, etc.) They are *not* responsible for the contents of the *.wiki.kernel.org; someone from the project has to be the wiki maintainer. (Note: the *.wiki.kernel.org infrastructure was originally set up at my request, and the first such hosted wiki was ext4.wiki.kernel.org; the second was rt.wiki.kernel.org, for which I was also the primary wiki administrator initially. I'm confident the policy on this hasn't changed since those early days because LF sysadmins (e.g., John and Konstantin) do *not* have time to police the various wikis for spam) - Ted -- To unsubscribe from this list: send the line 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] gitk: Display the date of a tag in a human friendly way.
Anand Kumria wildf...@progsoc.org writes:
By selecting a tag within gitk you can display information about it.
This information is output by using the command
'git cat-file tag tagid'
This outputs the *raw* information from the tag, amongst which is the
time - in seconds since the epoch. As useful as that value is, I find it
a lot easier to read and process time which it is something like:
Mon Dec 31 14:26:11 2012 -0800
This change will modify the display of tags in gitk like so:
@@ -1,7 +1,7 @@
object 5d417842efeafb6e109db7574196901c4e95d273
type commit
tag v1.8.1
-tagger Junio C Hamano gits...@pobox.com 1356992771 -0800
+tagger Junio C Hamano gits...@pobox.com Mon Dec 31 14:26:11 2012 -0800
Git 1.8.1
-BEGIN PGP SIGNATURE-
Signed-off-by: Anand Kumria wildf...@progsoc.org
---
Sounds like a sensible thing to do but I didn't check how else
(other than purely for displaying) this string is used.
Paul, the patch is not made against your tree, so if you choose to
take it you would need to strip the leading directory at the top.
Thanks.
PS. I haven't received a pull request from you for a while; are
there accumulated changes I should be pulling in before -rc0 of the
next release we are working on?
gitk-git/gitk |2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/gitk-git/gitk b/gitk-git/gitk
index d93bd99..aae1c58 100755
--- a/gitk-git/gitk
+++ b/gitk-git/gitk
@@ -10675,7 +10675,7 @@ proc showtag {tag isnew} {
set linknum 0
if {![info exists cached_tagcontent($tag)]} {
catch {
- set cached_tagcontent($tag) [exec git cat-file tag $tag]
+ set cached_tagcontent($tag) [exec git cat-file -p $tag]
}
}
if {[info exists cached_tagcontent($tag)]} {
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[BUG/PATCH] setup: Copy an environment variable to avoid overwrites
It is possible for this pointer of the GIT_DIR environment variable to
survive unduplicated until further getenv calls are made. The standards
allow for subsequent calls of getenv to overwrite the string located at
its returned pointer, and this can result in broken git operations on
certain platforms.
Signed-off-by: David Michael fedora@gmail.com
---
I have encountered an issue with consecutive calls to getenv
overwriting earlier values. Most notably, it prevents a plain git
clone from working.
Long story short: This value of GIT_DIR gets passed around setup.c
until it reaches check_repository_format_gently. This function calls
git_config_early, which eventually runs getenv(HOME). When it
returns back to check_repository_format_gently, the gitdir variable
contains my home directory path. The end result is that I wind up
with ~/objects/ etc. and a failed repository clone. (Simply adding a
bare getenv(GIT_DIR) afterwards to reset the pointer also corrects
the problem.)
Since other platforms are apparently working, yet this getenv behavior
is supported by the standards, I am left wondering if this could be a
symptom of something else being broken on my platform (z/OS). Can
anyone more familiar with this part of git identify any condition that
obviously should not be occurring?
Thanks.
setup.c | 8 ++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/setup.c b/setup.c
index f108c4b..64fb160 100644
--- a/setup.c
+++ b/setup.c
@@ -675,8 +675,12 @@ static const char
*setup_git_directory_gently_1(int *nongit_ok)
* validation.
*/
gitdirenv = getenv(GIT_DIR_ENVIRONMENT);
-if (gitdirenv)
-return setup_explicit_git_dir(gitdirenv, cwd, len, nongit_ok);
+if (gitdirenv) {
+gitdirenv = xstrdup(gitdirenv);
+ret = setup_explicit_git_dir(gitdirenv, cwd, len, nongit_ok);
+free(gitdirenv);
+return ret;
+}
if (env_ceiling_dirs) {
string_list_split(ceiling_dirs, env_ceiling_dirs, PATH_SEP, -1);
--
1.7.11.7
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [BUG/PATCH] setup: Copy an environment variable to avoid overwrites
David Michael fedora@gmail.com writes:
I have encountered an issue with consecutive calls to getenv
overwriting earlier values. Most notably, it prevents a plain git
clone from working.
Long story short: This value of GIT_DIR gets passed around setup.c
until it reaches check_repository_format_gently. This function calls
git_config_early, which eventually runs getenv(HOME). When it
returns back to check_repository_format_gently, the gitdir variable
contains my home directory path. The end result is that I wind up
with ~/objects/ etc. and a failed repository clone. (Simply adding a
bare getenv(GIT_DIR) afterwards to reset the pointer also corrects
the problem.)
Since other platforms are apparently working, yet this getenv behavior
is supported by the standards, I am left wondering if this could be a
symptom of something else being broken on my platform (z/OS).
The execve(2) function
int execve(const char *filename, char *const argv[],
char *const envp[]);
takes a NULL terminated array of NUL terminated strings of form
VAR=VAL in envp[], and this is kept in:
extern char **environ;
of the new image that runs.
The most naive and straight-forward way to implement getenv(3) is to
iterate over this environ[] array to look for an element that begins
with GIT_DIR=, and return the pointer pointing at the location one
byte past that equal sign. So even if the standard allowed the
returned value to be volatile across calls to getenv(3), it will
take *more* work for implementations if they want to break our use
pattern. They have to deliberately return a string that they will
overwrite in subsequent calls to getenv(3).
Also the natural way to implement putenv(3) and setenv(3) is to
replace the pointer in the environ[] array (not overwrite the
existing string in environ[] that holds the VAR=VAL string that
represents the current value, which might be shorter than the new
value of the enviornment variable), hence even calling these
functions is unlikely to invalidate the result you previously
received from getenv(3).
I am not at all surprised that nobody from other platforms has seen
this breakage.
In fact,
http://pubs.opengroup.org/onlinepubs/9699919799/functions/getenv.html
says that only setenv(), unsetenv() and putenv() may invalidate
previous return values. Note that getenv() is not listed as a
function that is allowed to break return values from a previous call
to getenv().
So in short, your platform's getenv(3) emulation may be broken. We
have other calls to getenv() where we rely on the result of it being
stable, and you might discover these places also break.
Having said that, we do have codepaths to update a handful of
environment variables ourselves (GIT_DIR is among them), so I think
your patch is a good safety measure in general.
setup.c | 8 ++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/setup.c b/setup.c
index f108c4b..64fb160 100644
--- a/setup.c
+++ b/setup.c
@@ -675,8 +675,12 @@ static const char
*setup_git_directory_gently_1(int *nongit_ok)
* validation.
*/
gitdirenv = getenv(GIT_DIR_ENVIRONMENT);
-if (gitdirenv)
-return setup_explicit_git_dir(gitdirenv, cwd, len, nongit_ok);
+if (gitdirenv) {
+gitdirenv = xstrdup(gitdirenv);
+ret = setup_explicit_git_dir(gitdirenv, cwd, len, nongit_ok);
+free(gitdirenv);
+return ret;
+}
if (env_ceiling_dirs) {
string_list_split(ceiling_dirs, env_ceiling_dirs, PATH_SEP, -1);
--
1.7.11.7
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [BUG/PATCH] setup: Copy an environment variable to avoid overwrites
On Sat, Jan 5, 2013 at 7:35 AM, David Michael fedora@gmail.com wrote:
-if (gitdirenv)
-return setup_explicit_git_dir(gitdirenv, cwd, len, nongit_ok);
+if (gitdirenv) {
+gitdirenv = xstrdup(gitdirenv);
+ret = setup_explicit_git_dir(gitdirenv, cwd, len, nongit_ok);
+free(gitdirenv);
+return ret;
+}
Maybe we could all this into a wrapper? If getenv() here has a
problem, many other places may have the same problem too. This
simplifies the change. But one has to check that getenv() must not be
used in threaded code.
char *git_getenv(const char *env)
{
static int bufno;
static char *buf[4];
bufno = (bufno + 1) % 4;
free(buf[bufno]);
buf[bufno] = xstrdup(getenv(env));
return buf[bufno];
}
#define getenv(x) git_getenv(x)
--
Duy
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [BUG/PATCH] setup: Copy an environment variable to avoid overwrites
Junio C Hamano gits...@pobox.com writes: ... So even if the standard allowed the returned value to be volatile across calls to getenv(3),... ... In fact, http://pubs.opengroup.org/onlinepubs/9699919799/functions/getenv.html says that only ... Apparently I wasn't even reading what I was quoting carefully enough. The above does include getenv() as one of the functions that are allowed to invalidate earlier return values. Sorry about that. I'll go back to bed (I am a bit under the weather and OOO today). The conclusion in my original message is still valid. Having said that, we do have codepaths to update a handful of environment variables ourselves (GIT_DIR is among them), so I think your patch is a good safety measure in general. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [BUG/PATCH] setup: Copy an environment variable to avoid overwrites
Duy Nguyen pclo...@gmail.com writes: Maybe we could all this into a wrapper? If getenv() here has a problem, many other places may have the same problem too. This simplifies the change. But one has to check that getenv() must not be used in threaded code. That needs to be done regardless, if we care; POSIX explicitly says getenv() need not be thread-safe. I personally do not think a wrapper with limited slots is a healthy direction to go. Most places we use getenv() do not let the return value live across their scope, and those that do should explicitly copy the value away. It's between validating that there is _no_ *env() calls in the codepath between a getenv() call and the use of its return value, and validating that there is at most 4 such calls there. The former is much easier to verify and maintain, I think. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [BUG/PATCH] setup: Copy an environment variable to avoid overwrites
On Sat, Jan 5, 2013 at 11:38 AM, Junio C Hamano gits...@pobox.com wrote: I personally do not think a wrapper with limited slots is a healthy direction to go. Most places we use getenv() do not let the return value live across their scope, and those that do should explicitly copy the value away. It's between validating that there is _no_ *env() calls in the codepath between a getenv() call and the use of its return value, and validating that there is at most 4 such calls there. The former is much easier to verify and maintain, I think. I did not look carefully and was scared of 143 getenv calls. But with about 4 calls, yes it's best to do without the wrapper. -- Duy -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4] git-completion.bash: add support for path completion
Junio C Hamano gits...@pobox.com writes:
Marc Khouzam marc.khou...@ericsson.com writes:
I've been playing with it but I'm not getting the expected
behavior when I cd to a sub-directory.
Thanks for testing. Manlio?
Can you try the attached patch?
As I am not familiar with the completion machinery, take this with a
large grain of salt. Here is my explanation of what is going on in
this how about this fixup:
* Giving --git-dir from the command line (or GIT_DIR environment)
without specifying GIT_WORK_TREE is to signal Git that you are at
the top of the working tree. git ls-files will then show the
full tree even outside the real $cwd because you are lying to
Git.
* git diff-index could be told to show only the $cwd and its
subdirectory with the --relative option, but it alone is not
sufficient if you throw --git-dir at it; again, you end up lying
that you are at the top.
* As far as I can tell, there is no reason you would want to pass
--git-dir to these invocations of ls-files and diff-index. If
the previous call to __gitdir could figure out where it is, the
command should be able to figure it out the same way.
There seem to be millions of other existing git --git-dir=$there
in this script. As I already said, I am not familiar with the
completion machinery, and I do not know what they are for in the
first place. Perhaps people put them there for a reason, but I do
not know what that reason is.
I think the ones for for-each-ref, config and stash should be
harmless. They are commands that do not care about the working
tree.
There is one given to ls-tree used in __git_complete_revlist_file;
I do not know if this was intended, what it is for, and if that is
working as intended, though.
I've been CC'ing two people who touched this script rather heavily,
are expected to know the completion machinery, and should be able to
help polishing this topic and moving it forward. Perhaps one of
them can shed light on this.
-- 8 --
Subject: completion: do not pass harmful --git-dir=$there
The recently added __git_index_files and __git_diff_index_files
helper functions invoke ls-files and diff_index while explicitly
passing --git-dir=$there, to tell them that the invocation is done
at the top of the working tree, which may not be the case when the
user is in a subdirectory. Remove the harmful use of this option,
Tell diff-index to show only the paths in the $cwd and show them
relative to the $cwd by passing --relative. The ls-files does
not need this, as that is already its default mode of operation.
---
contrib/completion/git-completion.bash | 12 ++--
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/contrib/completion/git-completion.bash
b/contrib/completion/git-completion.bash
index c8c6464..f4bd548 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -267,9 +267,9 @@ __git_index_files ()
if [ -d $dir ]; then
# NOTE: $1 is not quoted in order to support multiple options
- git --git-dir=$dir ls-files --exclude-standard $1 ${2+$2}
2/dev/null |
- __git_index_file_list_filter ${2+$2} |
- uniq
+ git ls-files --exclude-standard $1 ${2+$2} 2/dev/null |
+ __git_index_file_list_filter ${2+$2} |
+ uniq
fi
}
@@ -284,9 +284,9 @@ __git_diff_index_files ()
local dir=$(__gitdir)
if [ -d $dir ]; then
- git --git-dir=$dir diff-index --name-only $1 2/dev/null |
- __git_index_file_list_filter ${2+$2} |
- uniq
+ git diff-index --name-only --relative $1 2/dev/null |
+ __git_index_file_list_filter ${2+$2} |
+ uniq
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: [BUG/PATCH] setup: Copy an environment variable to avoid overwrites
Duy Nguyen pclo...@gmail.com writes: On Sat, Jan 5, 2013 at 11:38 AM, Junio C Hamano gits...@pobox.com wrote: I personally do not think a wrapper with limited slots is a healthy direction to go. Most places we use getenv() do not let the return value live across their scope, and those that do should explicitly copy the value away. It's between validating that there is _no_ *env() calls in the codepath between a getenv() call and the use of its return value, and validating that there is at most 4 such calls there. The former is much easier to verify and maintain, I think. I did not look carefully and was scared of 143 getenv calls. But with about 4 calls, yes it's best to do without the wrapper. Just to make sure you did not misunderstand, the 4 (four) in my message is not about 4 calls among 143 are unsafe. It was referring to the number of rotating slots your patch defined, which means val = getenv(FOO); ... random other code ... use(val) is safe only if random other code makes less than 4 getenv() calls. I didn't verify all of the call sites. It needs to be done with or without your wrapper patch. Without your wrapper, the validation needs to make sure random other code above does not make any getenv() call. With your wrapper, the validation needs to make sure random other code above does not make more than 4 such calls. My point was that the effort needed for both are about the same, so your wrapper does not buy us much. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 00/10] push: switch default from matching to simple
We promised to change the behaviour of lazy git push [there] that does not say what to push on the command line from matching to simple in Git 2.0. Even though the top-level RelNotes symbolic link points at 1.8.2, we can change our mind to tag 2.0 at the end of this cycle. This carries out the threat ;-) Mmany tests have assumed that the long established default will never change and relied on the convenience of the matching behaviour. They need to be updated to either specify what they want from the command line explicitly, or configure the default behaviour they would expect. And then finally we can flip the default. Junio C Hamano (10): t5404: do not assume the matching push is the default t5505: do not assume the matching push is the default t5516: do not assume the matching push is the default t5517: do not assume the matching push is the default t5519: do not assume the matching push is the default t5531: do not assume the matching push is the default t7406: do not assume the matching push is the default t9400: do not assume the matching push is the default t9401: do not assume the matching push is the default push: switch default from matching to simple Documentation/config.txt| 6 +++--- builtin/push.c | 31 +++ t/t5404-tracking-branches.sh| 2 +- t/t5505-remote.sh | 2 +- t/t5516-fetch-push.sh | 10 +- t/t5517-push-mirror.sh | 2 +- t/t5519-push-alternates.sh | 12 ++-- t/t5531-deep-submodule-push.sh | 1 + t/t7406-submodule-update.sh | 4 ++-- t/t9400-git-cvsserver-server.sh | 1 + t/t9401-git-cvsserver-crlf.sh | 1 + 11 files changed, 29 insertions(+), 43 deletions(-) -- 1.8.1.299.gc73b41f -- To unsubscribe from this list: send the line 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 01/10] t5404: do not assume the matching push is the default
Signed-off-by: Junio C Hamano gits...@pobox.com --- t/t5404-tracking-branches.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/t/t5404-tracking-branches.sh b/t/t5404-tracking-branches.sh index c240035..2b8c0ba 100755 --- a/t/t5404-tracking-branches.sh +++ b/t/t5404-tracking-branches.sh @@ -36,7 +36,7 @@ test_expect_success 'prepare pushable branches' ' ' test_expect_success 'mixed-success push returns error' ' - test_must_fail git push + test_must_fail git push origin : ' test_expect_success 'check tracking branches updated correctly after push' ' -- 1.8.1.299.gc73b41f -- To unsubscribe from this list: send the line 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 02/10] t5505: do not assume the matching push is the default
Signed-off-by: Junio C Hamano gits...@pobox.com --- t/t5505-remote.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/t/t5505-remote.sh b/t/t5505-remote.sh index ccc55eb..6579a86 100755 --- a/t/t5505-remote.sh +++ b/t/t5505-remote.sh @@ -345,7 +345,7 @@ test_expect_success 'fetch mirrors do not act as mirrors during push' ' ) (cd mirror-fetch/child git branch -m renamed renamed2 -git push parent +git push parent : ) (cd mirror-fetch/parent git rev-parse --verify renamed -- 1.8.1.299.gc73b41f -- To unsubscribe from this list: send the line 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 03/10] t5516: do not assume the matching push is the default
Signed-off-by: Junio C Hamano gits...@pobox.com --- t/t5516-fetch-push.sh | 10 +- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh index b5417cc..1a8753d 100755 --- a/t/t5516-fetch-push.sh +++ b/t/t5516-fetch-push.sh @@ -247,7 +247,7 @@ test_expect_success 'push with pushInsteadOf and explicit pushurl (pushInsteadOf test_expect_success 'push with matching heads' ' mk_test heads/master - git push testrepo + git push testrepo : check_push_result $the_commit heads/master ' @@ -276,7 +276,7 @@ test_expect_success 'push --force with matching heads' ' mk_test heads/master git push testrepo : git commit --amend -massaged - git push --force testrepo + git push --force testrepo : ! check_push_result $the_commit heads/master git reset --hard $the_commit @@ -507,7 +507,7 @@ test_expect_success 'push with config remote.*.pushurl' ' git checkout master git config remote.there.url test2repo git config remote.there.pushurl testrepo - git push there + git push there : check_push_result $the_commit heads/master ' @@ -521,7 +521,7 @@ test_expect_success 'push with dry-run' ' cd testrepo old_commit=$(git show-ref -s --verify refs/heads/master) ) - git push --dry-run testrepo + git push --dry-run testrepo : check_push_result $old_commit heads/master ' @@ -981,7 +981,7 @@ test_expect_success 'push --porcelain --dry-run rejected' ' test_expect_success 'push --prune' ' mk_test heads/master heads/second heads/foo heads/bar - git push --prune testrepo + git push --prune testrepo : check_push_result $the_commit heads/master check_push_result $the_first_commit heads/second ! check_push_result $the_first_commit heads/foo heads/bar -- 1.8.1.299.gc73b41f -- To unsubscribe from this list: send the line 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 04/10] t5517: do not assume the matching push is the default
Signed-off-by: Junio C Hamano gits...@pobox.com --- t/t5517-push-mirror.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/t/t5517-push-mirror.sh b/t/t5517-push-mirror.sh index e2ad260..12a5dfb 100755 --- a/t/t5517-push-mirror.sh +++ b/t/t5517-push-mirror.sh @@ -256,7 +256,7 @@ test_expect_success 'remote.foo.mirror=no has no effect' ' git branch keep master git push --mirror up git branch -D keep - git push up + git push up : ) ( cd mirror -- 1.8.1.299.gc73b41f -- To unsubscribe from this list: send the line 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 05/10] t5519: do not assume the matching push is the default
Signed-off-by: Junio C Hamano gits...@pobox.com --- t/t5519-push-alternates.sh | 12 ++-- 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/t/t5519-push-alternates.sh b/t/t5519-push-alternates.sh index c00c9b0..11fcd37 100755 --- a/t/t5519-push-alternates.sh +++ b/t/t5519-push-alternates.sh @@ -40,7 +40,7 @@ test_expect_success 'alice works and pushes' ' cd alice-work echo more file git commit -a -m second - git push ../alice-pub + git push ../alice-pub : ) ' @@ -57,7 +57,7 @@ test_expect_success 'bob fetches from alice, works and pushes' ' git pull ../alice-pub master echo more bob file git commit -a -m third - git push ../bob-pub + git push ../bob-pub : ) # Check that the second commit by Alice is not sent @@ -86,7 +86,7 @@ test_expect_success 'alice works and pushes again' ' cd alice-work echo more alice file git commit -a -m fourth - git push ../alice-pub + git push ../alice-pub : ) ' @@ -99,7 +99,7 @@ test_expect_success 'bob works and pushes' ' cd bob-work echo yet more bob file git commit -a -m fifth - git push ../bob-pub + git push ../bob-pub : ) ' @@ -115,7 +115,7 @@ test_expect_success 'alice works and pushes yet again' ' git commit -a -m sixth.2 echo more and more alice file git commit -a -m sixth.3 - git push ../alice-pub + git push ../alice-pub : ) ' @@ -136,7 +136,7 @@ test_expect_success 'bob works and pushes again' ' git hash-object -t commit -w commit echo even more bob file git commit -a -m seventh - git push ../bob-pub + git push ../bob-pub : ) ' -- 1.8.1.299.gc73b41f -- To unsubscribe from this list: send the line 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 06/10] t5531: do not assume the matching push is the default
Signed-off-by: Junio C Hamano gits...@pobox.com --- t/t5531-deep-submodule-push.sh | 1 + 1 file changed, 1 insertion(+) diff --git a/t/t5531-deep-submodule-push.sh b/t/t5531-deep-submodule-push.sh index 1947c28..8c16e04 100755 --- a/t/t5531-deep-submodule-push.sh +++ b/t/t5531-deep-submodule-push.sh @@ -16,6 +16,7 @@ test_expect_success setup ' ( cd gar/bage git init + git config push.default matching junk git add junk git commit -m Initial junk -- 1.8.1.299.gc73b41f -- To unsubscribe from this list: send the line 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 07/10] t7406: do not assume the matching push is the default
Signed-off-by: Junio C Hamano gits...@pobox.com --- t/t7406-submodule-update.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/t/t7406-submodule-update.sh b/t/t7406-submodule-update.sh index feaec6c..c675ce6 100755 --- a/t/t7406-submodule-update.sh +++ b/t/t7406-submodule-update.sh @@ -565,14 +565,14 @@ test_expect_success 'submodule add places git-dir in superprojects git-dir recur git log ../../../expected ) git commit -m added subsubmodule - git push + git push origin : ) (cd .git/modules/deeper/submodule/modules/subsubmodule git log ../../../../../actual ) git add deeper/submodule git commit -m update submodule -git push +git push origin : test_cmp actual expected ) ' -- 1.8.1.299.gc73b41f -- To unsubscribe from this list: send the line 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 10/10] push: switch default from matching to simple
We promised to change the behaviour of lazy git push [there] that
does not say what to push on the command line from matching to
simple in Git 2.0.
This finally flips that bit.
Signed-off-by: Junio C Hamano gits...@pobox.com
---
Documentation/config.txt | 6 +++---
builtin/push.c | 31 +++
2 files changed, 10 insertions(+), 27 deletions(-)
diff --git a/Documentation/config.txt b/Documentation/config.txt
index bf8f911..770eefe 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -1750,15 +1750,15 @@ push.default::
since locally stalled branches will attempt a non-fast forward push
if other users updated the branch.
+
- This is currently the default, but Git 2.0 will change the default
- to `simple`.
+ This used to be the default, and stale web sites may still say so,
+ but Git 2.0 has changed the default to `simple`.
* `upstream` - push the current branch to its upstream branch.
With this, `git push` will update the same remote ref as the one which
is merged by `git pull`, making `push` and `pull` symmetrical.
See branch.name.merge for how to configure the upstream branch.
* `simple` - like `upstream`, but refuses to push if the upstream
branch's name is different from the local one. This is the safest
- option and is well-suited for beginners. It will become the default
+ option and is well-suited for beginners. It has become the default
in Git 2.0.
* `current` - push the current branch to a branch of the same name.
--
diff --git a/builtin/push.c b/builtin/push.c
index db9ba30..9f7c252 100644
--- a/builtin/push.c
+++ b/builtin/push.c
@@ -24,7 +24,6 @@ static int progress = -1;
static const char **refspec;
static int refspec_nr;
static int refspec_alloc;
-static int default_matching_used;
static void add_refspec(const char *ref)
{
@@ -148,9 +147,9 @@ static void setup_push_upstream(struct remote *remote, int
simple)
}
static char warn_unspecified_push_default_msg[] =
-N_(push.default is unset; its implicit value is changing in\n
+N_(push.default is unset; its implicit value has changed in\n
Git 2.0 from 'matching' to 'simple'. To squelch this message\n
- and maintain the current behavior after the default changes, use:\n
+ and maintain the traditional behavior, use:\n
\n
git config --global push.default matching\n
\n
@@ -175,14 +174,14 @@ static void setup_default_push_refspecs(struct remote
*remote)
{
switch (push_default) {
default:
- case PUSH_DEFAULT_UNSPECIFIED:
- default_matching_used = 1;
- warn_unspecified_push_default_configuration();
- /* fallthru */
case PUSH_DEFAULT_MATCHING:
add_refspec(:);
break;
+ case PUSH_DEFAULT_UNSPECIFIED:
+ warn_unspecified_push_default_configuration();
+ /* fallthru */
+
case PUSH_DEFAULT_SIMPLE:
setup_push_upstream(remote, 1);
break;
@@ -208,12 +207,6 @@ static const char message_advice_pull_before_push[] =
before pushing again.\n
See the 'Note about fast-forwards' in 'git push --help' for
details.);
-static const char message_advice_use_upstream[] =
- N_(Updates were rejected because a pushed branch tip is behind its
remote\n
- counterpart. If you did not intend to push that branch, you may
want to\n
- specify branches to push or set the 'push.default' configuration
variable\n
- to 'simple', 'current' or 'upstream' to push only the current
branch.);
-
static const char message_advice_checkout_pull_push[] =
N_(Updates were rejected because a pushed branch tip is behind its
remote\n
counterpart. Check out this branch and merge the remote changes\n
@@ -227,13 +220,6 @@ static void advise_pull_before_push(void)
advise(_(message_advice_pull_before_push));
}
-static void advise_use_upstream(void)
-{
- if (!advice_push_non_ff_default || !advice_push_nonfastforward)
- return;
- advise(_(message_advice_use_upstream));
-}
-
static void advise_checkout_pull_push(void)
{
if (!advice_push_non_ff_matching || !advice_push_nonfastforward)
@@ -272,10 +258,7 @@ static int push_with_options(struct transport *transport,
int flags)
advise_pull_before_push();
break;
case NON_FF_OTHER:
- if (default_matching_used)
- advise_use_upstream();
- else
- advise_checkout_pull_push();
+ advise_checkout_pull_push();
break;
}
--
1.8.1.299.gc73b41f
--
To unsubscribe from this list: send the line 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 09/10] t9401: do not assume the matching push is the default
Signed-off-by: Junio C Hamano gits...@pobox.com --- t/t9401-git-cvsserver-crlf.sh | 1 + 1 file changed, 1 insertion(+) diff --git a/t/t9401-git-cvsserver-crlf.sh b/t/t9401-git-cvsserver-crlf.sh index 1c5bc84..8c3db76 100755 --- a/t/t9401-git-cvsserver-crlf.sh +++ b/t/t9401-git-cvsserver-crlf.sh @@ -84,6 +84,7 @@ export CVSROOT CVS_SERVER rm -rf $CVSWORK $SERVERDIR test_expect_success 'setup' ' +git config push.default matching echo Simple text file textfile.c echo File with embedded NUL: Q - there | q_to_nul binfile.bin mkdir subdir -- 1.8.1.299.gc73b41f -- To unsubscribe from this list: send the line 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 08/10] t9400: do not assume the matching push is the default
Signed-off-by: Junio C Hamano gits...@pobox.com --- t/t9400-git-cvsserver-server.sh | 1 + 1 file changed, 1 insertion(+) diff --git a/t/t9400-git-cvsserver-server.sh b/t/t9400-git-cvsserver-server.sh index 9502f24..0431386 100755 --- a/t/t9400-git-cvsserver-server.sh +++ b/t/t9400-git-cvsserver-server.sh @@ -36,6 +36,7 @@ export CVSROOT CVS_SERVER rm -rf $CVSWORK $SERVERDIR test_expect_success 'setup' ' + git config push.default matching echo empty git add empty git commit -q -m First Commit -- 1.8.1.299.gc73b41f -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 11/19] dir.c: use a single struct exclude_list per source of excludes
Junio C Hamano gits...@pobox.com writes:
Adam Spiers g...@adamspiers.org writes:
diff --git a/builtin/clean.c b/builtin/clean.c
index 0c7b3d0..bd18b88 100644
--- a/builtin/clean.c
+++ b/builtin/clean.c
@@ -97,9 +97,10 @@ int cmd_clean(int argc, const char **argv, const char
*prefix)
if (!ignored)
setup_standard_excludes(dir);
+add_exclude_list(dir, EXC_CMDL);
for (i = 0; i exclude_list.nr; i++)
add_exclude(exclude_list.items[i].string, , 0,
-dir.exclude_list[EXC_CMDL]);
+dir.exclude_list_groups[EXC_CMDL].ary[0]);
This looks somewhat ugly for two reasons.
* The abstraction add_exclude() offers to its callers is just to
let them add one pattern to the list of patterns for the kind
(here, EXC_CMDL); why should they care about .ary[0] part? Are
there cases any sane caller (not the implementation of the
exclude_list_group machinery e.g. add_excludes_from_... function)
may want to call it with .ary[1]? I do not think of any.
Shouldn't the public API function add_exclude() take a pointer to
the list group itself?
* When naming an array of things, we tend to prefer naming it
type thing[count]
so that the second element can be called thing[2] and not
things[2]. dir.exclude_list_group[EXC_CMDL] reads better.
Also, ary[] is a bad name, even as an implementation detail, for
two reasons: it is naming it after its type (being an array) not
after what it is (if it holds the patterns from the same information
source, e.g. file, togeter, src might be a better name), and it
uses rather unusual abbreviation (I haven't seen array shortened
to ary anywhere else).
diff --git a/builtin/ls-files.c b/builtin/ls-files.c
index ef7f99a..c448e06 100644
--- a/builtin/ls-files.c
+++ b/builtin/ls-files.c
@@ -420,10 +420,10 @@ static int option_parse_z(const struct option *opt,
static int option_parse_exclude(const struct option *opt,
const char *arg, int unset)
{
-struct exclude_list *list = opt-value;
+struct exclude_list_group *group = opt-value;
exc_given = 1;
-add_exclude(arg, , 0, list);
+add_exclude(arg, , 0, group-ary[0]);
This is another example where the caller would wish to be able to say
add_exclude(arg, , 0, group);
instead.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
