* Re: [RFD] Rewriting safety - warn before/when rewriting published history
From: Jakub Narebski @ 2012-02-11 13:45 UTC (permalink / raw)
To: Johan Herland; +Cc: Philip Oakley, git
In-Reply-To: <CALKQrgdWOgG3y2HzM694zDykGJWa4QDetsEVXf0AGpf=FNFaVg@mail.gmail.com>
On Sat, 11 Feb 2012, Johan Herland wrote:
> On Fri, Feb 10, 2012 at 20:38, Jakub Narebski <jnareb@gmail.com> wrote:
> > On Tue, 7 Feb 2012, Johan Herland wrote:
[...]
> > > I am unsure whether the 'secret'-ness of a commit should follow across
> > > the push, but if you do (assuming we store the 'secret' flag using
> > > git-notes) this is simply a matter of synchronizing the
> > > refs/notes/secret to the same remote.
> >
> > I think it should, so that 'secret' commit would not escape by accident
> > via a group secret repository.
> >
> > What makes it hard (I think) is that we would prefer to transfer
> > 'secret'-ness only for pushed commits. That might be problem for notes
> > based implementation of 'secret' annotation and 'secret'-ness transfer...
> > though I guess knowing that there exist 'secret' commit with given SHA1
> > which we do not have and should not have is not much breach of
> > confidentiality. Still...
>
> If you don't want to transfer all of refs/notes/secret, you would
> probably have to extend the git protocol with a per-commit 'secret'
> flag (which would then be applied to the receiving repo's
> refs/notes/secret).
Or create per-remote custom notes ref, for example for 'foo' remote it
would be 'refs/remotes/foo/notes/secret'... assuming that canonalization
of remote-tracking refs goes in (refs/remotes/foo/{heads,tags,notes,replace})
This would be updated with 'secret'-ness state of comits being pushed
before actual push, and secretness notes ref would be pushed together
with actual push.
> Still, this is all specific to the 'secret' feature, which IMHO is
> much less important then the 'public' feature. Implementing the
> barebones 'public' feature (i.e. refuse rewrite of commits reachable
> from upstream) is much less work, and would be enough for 90% of git
> users, I believe.
Hmmm... I thought that is 'public' feature that is more work, especially
in full incarnation. But perhaps bare bones (no 'pre-push' or 'pre-rewrite'
hooks, no traits info in "git log" output, per remote tracking of 'public'
status only, no support for bundles or send-email, etc.) could be as easy
or easier.
As to what is more important for git users... perhaps short survey would
help here? I wonder if asking Mercurial users about their use of "phases"
on their mailing would help us...
--
Jakub Narebski
Poland
^ permalink raw reply
* Re: [RFD] Rewriting safety - warn before/when rewriting published history
From: Johan Herland @ 2012-02-11 13:10 UTC (permalink / raw)
To: Jakub Narebski; +Cc: Philip Oakley, git
In-Reply-To: <201202102038.55710.jnareb@gmail.com>
On Fri, Feb 10, 2012 at 20:38, Jakub Narebski <jnareb@gmail.com> wrote:
> On Tue, 7 Feb 2012, Johan Herland wrote:
>> On Tue, Feb 7, 2012 at 15:31, Jakub Narebski <jnareb@gmail.com> wrote:
>> I am unsure whether the 'secret'-ness of a commit should follow across
>> the push, but if you do (assuming we store the 'secret' flag using
>> git-notes) this is simply a matter of synchronizing the
>> refs/notes/secret to the same remote.
>
> I think it should, so that 'secret' commit would not escape by accident
> via a group secret repository.
>
> What makes it hard (I think) is that we would prefer to transfer
> 'secret'-ness only for pushed commits. That might be problem for notes
> based implementation of 'secret' annotation and 'secret'-ness transfer...
> though I guess knowing that there exist 'secret' commit with given SHA1
> which we do not have and should not have is not much breach of
> confidentiality. Still...
If you don't want to transfer all of refs/notes/secret, you would
probably have to extend the git protocol with a per-commit 'secret'
flag (which would then be applied to the receiving repo's
refs/notes/secret).
Still, this is all specific to the 'secret' feature, which IMHO is
much less important then the 'public' feature. Implementing the
barebones 'public' feature (i.e. refuse rewrite of commits reachable
from upstream) is much less work, and would be enough for 90% of git
users, I believe.
...Johan
--
Johan Herland, <johan@herland.net>
www.herland.net
^ permalink raw reply
* [PATCH] gitweb: Silence stderr in parse_commit*() subroutines
From: Jakub Narebski @ 2012-02-11 13:02 UTC (permalink / raw)
To: rajesh boyapati; +Cc: git
In-Reply-To: <201202092114.40832.jnareb@gmail.com>
On Thu, 9 Feb 2012, Jakub Narebski wrote:
> On Wed, 8 Feb 2012, rajesh boyapati wrote:
> > 2012/2/8 Jakub Narebski <jnareb@gmail.com>
>
> [...]
> > > Does the following patch help, and does it fix the issue?
> > >
> > > (Nb. you can try to simply change filename, and apply it with fuzz
> > > against index.cgi file).
> > > -- >8 -- ----- ----- ----- ----- ----- -- >8 --
> > > From: Jakub Narebski <jnareb@gmail.com>
> > > Subject: [PATCH] gitweb: Harden parse_commit and parse_commits
> [...]
> > When I applied the above patch and also the patch from your previous
> > e-mail, I am getting this error
> > >>>>>>>>>>>>>
> > [2012-02-08 14:09:58,396] ERROR
> > com.google.gerrit.httpd.gitweb.GitWebServlet : CGI: fatal: bad revision
> > 'HEAD'
> > [2012-02-08 14:10:06,732] ERROR
> > com.google.gerrit.httpd.gitweb.GitWebServlet : CGI: fatal: bad revision
> > 'HEAD'
> > [2012-02-08 14:10:11,404] ERROR
> > com.google.gerrit.httpd.gitweb.GitWebServlet : CGI: fatal: bad revision
> > 'HEAD'
> > [2012-02-08 14:10:15,270] ERROR
> > com.google.gerrit.httpd.gitweb.GitWebServlet : CGI: fatal: Not a valid
> > object name HEAD
> > <<<<<<<<<<<<<<
> > With these patches, the previous errors at line numbers are gone.
>
> Thanks for information.
>
>
> This final issue will be a bit harder to fix. This error message
>
> fatal: bad revision 'HEAD'
>
> comes from git (I think from "git rev-list" command), and not from gitweb.
> It is printed on STDERR of git command. What has to be done to fix it is
> to capture stderr of a process, or silence it.
>
> Unfortunately it is not that easy. We use list form of open, which avoids
> using a shell interpreter to run command, and is safer wrt. shell escaping.
>
> The only place where gitweb cares about redirecting standard error from git
> command is git_object(). It is a bit hacky, and might be not entirely safe.
> To fix this issue we would have to do the same in parse_commit*() as in
> git_object(), or provide some kind of wrapper like IPC::Run provides
> for redirecting stderr of called command.
>
> Note that this issue was not considered very important, because this message
> doesn't goes into web server logs when running gitweb via mod_cgi with
> Apache... and probably also with other web servers. Gerrit (or rather
> whatever it uses for serving CGI scripts) might be exception here.
Anyway, here is the patch that should fix those "CGI: fatal: Not a valid
object name HEAD" errors for you.
I'll resend the all the patches as single patch series for inclusion in
git, but I am not sure if this latest patch will be accepted because of
drawbacks of its implementation.
-- >8 ---- ----- ----- ----- >8 --
From: Jakub Narebski <jnareb@gmail.com>
Subject: [PATCH] gitweb: Silence stderr in parse_commit*() subroutines
git-rev-list command in parse_commit() and parse_commits() can fail
because $commit_id doesn't point to a valid commit; for example if we
are on unborn branch HEAD doesn't point to a valid commit.
In this case git-rev-list prints
fatal: bad revision 'HEAD'
on its standard error. This commit silences this warning, at the cost
of using shell to redirect it to /dev/null, and relying on
quote_command() to protect against code injection.
Note however that such error message from git (from extrenal command)
usually but not always does not appear in web server logs.
Reported-by: rajesh boyapati <boyapatisrajesh@gmail.com>
Signed-off-by: Jakub Narebski <jnareb@gmail.com>
---
gitweb/gitweb.perl | 10 ++++++----
1 files changed, 6 insertions(+), 4 deletions(-)
diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 1181aeb..081ac45 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -3338,12 +3338,13 @@ sub parse_commit {
local $/ = "\0";
- open my $fd, "-|", git_cmd(), "rev-list",
+ open my $fd, "-|", quote_command(
+ git_cmd(), "rev-list",
"--parents",
"--header",
"--max-count=1",
$commit_id,
- "--",
+ "--") . ' 2>/dev/null',
or die_error(500, "Open git-rev-list failed");
my $commit_text = <$fd>;
%co = parse_commit_text($commit_text, 1)
@@ -3363,7 +3364,8 @@ sub parse_commits {
local $/ = "\0";
- open my $fd, "-|", git_cmd(), "rev-list",
+ open my $fd, "-|", quote_command(
+ git_cmd(), "rev-list",
"--header",
@args,
("--max-count=" . $maxcount),
@@ -3371,7 +3373,7 @@ sub parse_commits {
@extra_options,
$commit_id,
"--",
- ($filename ? ($filename) : ())
+ ($filename ? ($filename) : ())) . ' 2>/dev/null'
or die_error(500, "Open git-rev-list failed");
while (my $line = <$fd>) {
my %co = parse_commit_text($line);
--
1.7.9
^ permalink raw reply related
* Re: [PATCH] pack-refs: remove all empty dirs under .git/{refs,logs/refs}
From: Thomas Adam @ 2012-02-11 11:27 UTC (permalink / raw)
To: Nguyễn Thái Ngọc Duy; +Cc: git, Junio C Hamano, Jeff King
In-Reply-To: <1328958484-4202-1-git-send-email-pclouds@gmail.com>
2012/2/11 Nguyễn Thái Ngọc Duy <pclouds@gmail.com>:
> static void prune_refs(struct ref_to_prune *r)
> {
> + struct string_list *list = get_empty_ref_directories();;
Double ";;" at end of line.
-- Thomas Adam
^ permalink raw reply
* [PATCH] pack-refs: remove all empty dirs under .git/{refs,logs/refs}
From: Nguyễn Thái Ngọc Duy @ 2012-02-11 11:08 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Jeff King, Nguyễn Thái Ngọc Duy
In-Reply-To: <1328946907-31650-1-git-send-email-pclouds@gmail.com>
"git pack-refs" tries to remove directory that becomes empty but it
does not try to do so hard enough. Only empty directories created
because a ref is packed are considered.
This patch introduces a global switch, which instructs ref machinery
to collect all empty directories (or ones containing only empty
directories) in removable order. "git pack-refs" uses this information
to clean $GIT_DIR/refs and $GIT_DIR/logs/refs.
Some directories are kept by this patch even if they are empty: refs,
refs/heads and refs/tags. The first one is one of git repository
signature. The rest is created by init-db, one may expect them to always
be there.
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
v3, no second look at $GIT_DIR/refs and also clean
$GIT_DIR/logs/refs. Not really fond of the global switch, but it does
not look very intrusive to refs.c
builtin/pack-refs.c | 2 ++
pack-refs.c | 10 ++++++++++
refs.c | 35 +++++++++++++++++++++++++++++++----
refs.h | 4 ++++
t/t3210-pack-refs.sh | 10 ++++++++++
5 files changed, 57 insertions(+), 4 deletions(-)
diff --git a/builtin/pack-refs.c b/builtin/pack-refs.c
index 39a9d89..044ae8f 100644
--- a/builtin/pack-refs.c
+++ b/builtin/pack-refs.c
@@ -1,6 +1,7 @@
#include "builtin.h"
#include "parse-options.h"
#include "pack-refs.h"
+#include "refs.h"
static char const * const pack_refs_usage[] = {
"git pack-refs [options]",
@@ -15,6 +16,7 @@ int cmd_pack_refs(int argc, const char **argv, const char *prefix)
OPT_BIT(0, "prune", &flags, "prune loose refs (default)", PACK_REFS_PRUNE),
OPT_END(),
};
+ save_empty_ref_directories = 1;
if (parse_options(argc, argv, prefix, opts, pack_refs_usage, 0))
usage_with_options(pack_refs_usage, opts);
return pack_refs(flags);
diff --git a/pack-refs.c b/pack-refs.c
index f09a054..76d3408 100644
--- a/pack-refs.c
+++ b/pack-refs.c
@@ -2,6 +2,7 @@
#include "refs.h"
#include "tag.h"
#include "pack-refs.h"
+#include "string-list.h"
struct ref_to_prune {
struct ref_to_prune *next;
@@ -105,10 +106,19 @@ static void prune_ref(struct ref_to_prune *r)
static void prune_refs(struct ref_to_prune *r)
{
+ struct string_list *list = get_empty_ref_directories();;
+ int i;
+
while (r) {
prune_ref(r);
r = r->next;
}
+
+ for (i = 0; i < list->nr; i++) {
+ const char *s = list->items[i].string;
+ rmdir(git_path("%s", s));
+ rmdir(git_path("logs/%s", s));
+ }
}
static struct lock_file packed;
diff --git a/refs.c b/refs.c
index b8843bb..7e9a250 100644
--- a/refs.c
+++ b/refs.c
@@ -3,6 +3,7 @@
#include "object.h"
#include "tag.h"
#include "dir.h"
+#include "string-list.h"
/* ISSYMREF=0x01, ISPACKED=0x02 and ISBROKEN=0x04 are public interfaces */
#define REF_KNOWS_PEELED 0x10
@@ -29,6 +30,8 @@ struct ref_array {
struct ref_entry **refs;
};
+int save_empty_ref_directories;
+
/*
* Parse one line from a packed-refs file. Write the SHA1 to sha1.
* Return a pointer to the refname within the line (null-terminated),
@@ -177,6 +180,7 @@ static struct ref_cache {
char did_packed;
struct ref_array loose;
struct ref_array packed;
+ struct string_list empty_dirs;
/* The submodule name, or "" for the main repo. */
char name[FLEX_ARRAY];
} *ref_cache;
@@ -326,7 +330,7 @@ void add_packed_ref(const char *refname, const unsigned char *sha1)
}
static void get_ref_dir(struct ref_cache *refs, const char *base,
- struct ref_array *array)
+ struct ref_array *array, int *would_be_empty)
{
DIR *dir;
const char *path;
@@ -343,6 +347,7 @@ static void get_ref_dir(struct ref_cache *refs, const char *base,
struct dirent *de;
int baselen = strlen(base);
char *refname = xmalloc(baselen + 257);
+ int nr = 0;
memcpy(refname, base, baselen);
if (baselen && base[baselen-1] != '/')
@@ -355,8 +360,13 @@ static void get_ref_dir(struct ref_cache *refs, const char *base,
int namelen;
const char *refdir;
- if (de->d_name[0] == '.')
+ if (de->d_name[0] == '.') {
+ if (strcmp(de->d_name, "..") &&
+ strcmp(de->d_name, "."))
+ nr++;
continue;
+ }
+ nr++;
namelen = strlen(de->d_name);
if (namelen > 255)
continue;
@@ -369,7 +379,10 @@ static void get_ref_dir(struct ref_cache *refs, const char *base,
if (stat(refdir, &st) < 0)
continue;
if (S_ISDIR(st.st_mode)) {
- get_ref_dir(refs, refname, array);
+ int empty = 0;
+ get_ref_dir(refs, refname, array, &empty);
+ if (empty)
+ nr--;
continue;
}
if (*refs->name) {
@@ -387,6 +400,15 @@ static void get_ref_dir(struct ref_cache *refs, const char *base,
}
free(refname);
closedir(dir);
+ if (save_empty_ref_directories &&
+ nr == 0 &&
+ strcmp(base, "refs") &&
+ strcmp(base, "refs/heads") &&
+ strcmp(base, "refs/tags")) {
+ string_list_append(&refs->empty_dirs, xstrdup(base));
+ if (would_be_empty)
+ *would_be_empty = 1;
+ }
}
}
@@ -427,12 +449,17 @@ void warn_dangling_symref(FILE *fp, const char *msg_fmt, const char *refname)
static struct ref_array *get_loose_refs(struct ref_cache *refs)
{
if (!refs->did_loose) {
- get_ref_dir(refs, "refs", &refs->loose);
+ get_ref_dir(refs, "refs", &refs->loose, NULL);
refs->did_loose = 1;
}
return &refs->loose;
}
+struct string_list *get_empty_ref_directories()
+{
+ return &get_ref_cache(NULL)->empty_dirs;
+}
+
/* We allow "recursive" symbolic refs. Only within reason, though */
#define MAXDEPTH 5
#define MAXREFLEN (1024)
diff --git a/refs.h b/refs.h
index 00ba1e2..21a2a00 100644
--- a/refs.h
+++ b/refs.h
@@ -14,6 +14,10 @@ struct ref_lock {
#define REF_ISPACKED 0x02
#define REF_ISBROKEN 0x04
+struct string_list;
+extern int save_empty_ref_directories;
+extern struct string_list *get_empty_ref_directories();
+
/*
* Calls the specified function for each ref file until it returns nonzero,
* and returns the value
diff --git a/t/t3210-pack-refs.sh b/t/t3210-pack-refs.sh
index cd04361..40fcd54 100755
--- a/t/t3210-pack-refs.sh
+++ b/t/t3210-pack-refs.sh
@@ -66,6 +66,16 @@ test_expect_success 'see if git pack-refs --prune removes empty dirs' '
! test -e .git/refs/heads/r
'
+test_expect_success 'pack-refs --prune removes all empty dirs in refs and logs' '
+ mkdir -p .git/refs/empty/outside/heads &&
+ mkdir -p .git/refs/heads/empty/dir/ectory &&
+ mkdir -p .git/logs/refs/heads/empty/dir/ectory &&
+ git pack-refs --all --prune &&
+ ! test -e .git/refs/empty &&
+ ! test -e .git/refs/heads/empty &&
+ ! test -e .git/logs/refs/heads/empty
+'
+
test_expect_success \
'git branch g should work when git branch g/h has been deleted' \
'git branch g/h &&
--
1.7.8.36.g69ee2
^ permalink raw reply related
* Re: [PATCH 2/3] help.c: make term_columns() cached and export it
From: Zbigniew Jędrzejewski-Szmek @ 2012-02-11 10:49 UTC (permalink / raw)
To: Nguyen Thai Ngoc Duy; +Cc: git, gitster, Michael J Gruber, Ramsay Jones
In-Reply-To: <CACsJy8AjqqEpJr64DJUqVumw1sB2g3pvuz-g4DxhmS-ZbGhY3w@mail.gmail.com>
On 02/11/2012 05:36 AM, Nguyen Thai Ngoc Duy wrote:
> 2012/2/10 Zbigniew Jędrzejewski-Szmek<zbyszek@in.waw.pl>:
>> Since term_columns() will usually fail, when a pager is installed,
>> the cache is primed before the pager is installed. If a pager is not
>> installed, then the cache will be set on first use.
>
> Conflict alert. term_columns() is also moved out of help.c in
> nd/columns series on pu, commit cb0850f (Save terminal width before
> setting up pager - 2012-02-04)
Thanks for the heads-up. I think that the two patches should be
merged, especially because there's an error in cb0850f (a variable is
read-only, never written).
Tweaks to cb0850f (Save terminal width before setting up pager -
2012-02-04):
[This actually was done on top of today's pu, so it will not apply
cleanly to cb0850f].
- term_columns() lives in pager.c so it should be declared in
cache.h like other public functions in pager.c. It has nothing to do
with columns.h.
- simplify logic to use a static variable instead of two global
variables (cb0850f actually doesn't work at all because
spawned_pager wasn't ever set).
- check the cache first, then do getenv(COLUMNS) + atoi, then do
ioctl(). The behaviour is equivalent to checking COLUMNS first, but
is slightly more efficient.
- document the function
- remove #include "column.h" added in 88c9754c4097 column: Fix some
compiler and sparse warnings (Wed Feb 8 2012).
Junio suggested that "a new file, term.c or something, be a lot more
suitable home for the function you will be reusing from diff and other
parts of the system". Nevertheless, I think that adding two files (.c
and .h) to hold one function isn't worth it. It can live in pager.c.
Terminal size is logically connected to paging after all.
---
cache.h | 1 +
column.h | 1 -
pager.c | 64 +++++++++++++++++++++++-----------------
3 files changed, 38 insertions(+), 28 deletions(-)
diff --git a/cache.h b/cache.h
index 6c70dbc..b4422d4 100644
--- a/cache.h
+++ b/cache.h
@@ -1196,6 +1196,7 @@ extern void setup_pager(void);
extern const char *pager_program;
extern int pager_in_use(void);
extern int pager_use_color;
+extern int term_columns(void);
extern const char *editor_program;
extern const char *askpass_program;
diff --git a/column.h b/column.h
index b9dec64..142299e 100644
--- a/column.h
+++ b/column.h
@@ -17,7 +17,6 @@ struct column_options {
const char *nl;
};
-extern int term_columns(void);
extern void print_columns(const struct string_list *list,
unsigned int mode,
struct column_options *opts);
diff --git a/pager.c b/pager.c
index fe203a7..d105761 100644
--- a/pager.c
+++ b/pager.c
@@ -7,21 +7,6 @@
#define DEFAULT_PAGER "less"
#endif
-static int spawned_pager;
-static int max_columns;
-
-static int retrieve_terminal_width(void)
-{
-#ifdef TIOCGWINSZ
- struct winsize ws;
- if (ioctl(1, TIOCGWINSZ, &ws)) /* e.g., ENOSYS */
- return 0;
- return ws.ws_col;
-#else
- return 0;
-#endif
-}
-
/*
* This is split up from the rest of git so that we can do
* something different on Windows.
@@ -88,16 +73,15 @@ const char *git_pager(int stdout_is_tty)
void setup_pager(void)
{
const char *pager = git_pager(isatty(1));
- int width;
if (!pager || pager_in_use())
return;
- setenv("GIT_PAGER_IN_USE", "true", 1);
+ /* prime the term_columns() cache before it is too
+ * late and stdout is replaced */
+ (void) term_columns();
- width = retrieve_terminal_width();
- if (width)
- max_columns = width;
+ setenv("GIT_PAGER_IN_USE", "true", 1);
/* spawn the pager */
pager_argv[0] = pager;
@@ -132,17 +116,43 @@ int pager_in_use(void)
return env ? git_config_bool("GIT_PAGER_IN_USE", env) : 0;
}
+/*
+ * Return cached value (if set) or $COLUMNS (if set and positive) or
+ * ioctl(1, TIOCGWINSZ).ws_col (if positive) or 80.
+ *
+ * $COLUMNS even if set, is usually not exported, so
+ * the variable can be used to override autodection.
+ * This behaviour conforms to The Single UNIX Specification, Version 2
+ *
(http://pubs.opengroup.org/onlinepubs/7908799/xbd/envvar.html#tag_002_003).
+ */
int term_columns(void)
{
- char *col_string = getenv("COLUMNS");
+ static int term_columns_cache;
+
+ char *col_string;
int n_cols;
- if (col_string && (n_cols = atoi(col_string)) > 0)
- return n_cols;
+ if (term_columns_cache)
+ return term_columns_cache;
+
+ col_string = getenv("COLUMNS");
+ if (col_string && (n_cols = atoi(col_string)) > 0) {
+ term_columns_cache = n_cols;
+ return term_columns_cache;
+ }
- if (spawned_pager && max_columns)
- return max_columns;
+#ifdef TIOCGWINSZ
+ {
+ struct winsize ws;
+ if (!ioctl(1, TIOCGWINSZ, &ws)) {
+ if (ws.ws_col) {
+ term_columns_cache = ws.ws_col;
+ return term_columns_cache;
+ }
+ }
+ }
+#endif
- n_cols = retrieve_terminal_width();
- return n_cols ? n_cols : 80;
+ term_columns_cache = 80;
+ return term_columns_cache;
}
--
1.7.9.310.g883d84c.dirty
^ permalink raw reply related
* Re: Git SSH Authentication
From: Sitaram Chamarty @ 2012-02-11 10:23 UTC (permalink / raw)
To: Junio C Hamano; +Cc: isawk, git
In-Reply-To: <7vty2xn6tl.fsf@alter.siamese.dyndns.org>
On Sat, Feb 11, 2012 at 1:24 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Sitaram Chamarty <sitaramc@gmail.com> writes:
>
>> Common causes of pubkey access fail:
>>
>> - wrong pubkey being offered: if you are using ssh-agent, make sure
>> you have 'ssh-add'ed the key you want to offer. Confirm with 'ssh-add
>> -l'
>
> A failure related to this I saw is to have (too) many keys in ssh-agent,
> and running ssh without telling it which exact key to use. The client
> tries each key in turn and the server rejects the connection attempt after
> seeing too many keys tried. "ssh -v" is useful to diagnose this mode of
> failure, and an entry in ~/.ssh/config like:
>
> Host example.com
> User myusernameoverthere
> IdentityFile ~/.ssh/id_rsa-for-example.com
>
> would fix it.
Interesting... I didn't know this, so I checked man sshd_config
looking for exactly how many keys it would fail after. The setting
seems to be MaxAuthTries, and defaults to 6.
--
Sitaram
^ permalink raw reply
* Re: git svn problem
From: Serhat Sevki Dincer @ 2012-02-11 10:19 UTC (permalink / raw)
To: git
In-Reply-To: <CAPqC6xSJ7pfUQJz8FQ743mJMNmZyfJfWxutOnt+FVkP76eXOGw@mail.gmail.com>
Ok, I figured out the rest, in case someone needs it:
> I have the following at the moment:
>
> rm -rf plone.app.locales ; mkdir plone.app.locales ; cd plone.app.locales
> git svn init -T trunk http://svn.plone.org/svn/plone/plone.app.locales
> touch start ; git add start ; git commit -m start
> git svn fetch -r49624:HEAD
> git rebase --onto master --root trunk --preserve-merges
> git checkout -b plone
# edit .git/config: s#/plone/#/collective/# s#remotes/trunk#remotes/trunkcol#
git checkout master
git svn fetch -r248302:HEAD
git rebase --onto plone --root trunkcol --preserve-merges
git checkout -b collective
git branch -d master plone
^ permalink raw reply
* Re: [RFC/PATCH] tag: make list exclude !<pattern>
From: Jakub Narebski @ 2012-02-11 10:13 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Tom Grennan, pclouds, git, krh, jasampler
In-Reply-To: <7vy5s9n70x.fsf@alter.siamese.dyndns.org>
Junio C Hamano <gitster@pobox.com> writes:
> In a future versions, we may want to have "branch/tag --list" also ask for
> FNM_PATHNAME (this *is* a backward incompatible change, so it needs to be
> performed across major version boundary, with backward compatibility
> configurations, deprecation warnings and whole nine yards). Under the new
> match function, today's "branch --list 'maint*'" needs to be spelled as
> "branch --list 'maint*/*'" or something.
Or "branch --list 'maint**'
--
Jakub Narebski
^ permalink raw reply
* [PATCH] strbuf: move strbuf_readline_fd() from bundle.c to strbuf.{c,h}
From: 徐迪 @ 2012-02-11 9:50 UTC (permalink / raw)
To: Git 邮件列表
strbuf_readline_fd() existed in bundle.c since e9ee84cf, but this
function can be used elsewhere, and since it's relevant to strbuf, it
should be in strbuf.{c,h}.
Signed-off-by: Xu Di <xudifsd@gmail.com>
---
bundle.c | 18 +-----------------
strbuf.c | 16 ++++++++++++++++
strbuf.h | 1 +
3 files changed, 18 insertions(+), 17 deletions(-)
diff --git a/bundle.c b/bundle.c
index b8acf3c..9344a91 100644
--- a/bundle.c
+++ b/bundle.c
@@ -7,6 +7,7 @@
#include "list-objects.h"
#include "run-command.h"
#include "refs.h"
+#include "strbuf.h"
static const char bundle_signature[] = "# v2 git bundle\n";
@@ -23,23 +24,6 @@ static void add_to_ref_list(const unsigned char
*sha1, const char *name,
list->nr++;
}
-/* Eventually this should go to strbuf.[ch] */
-static int strbuf_readline_fd(struct strbuf *sb, int fd)
-{
- strbuf_reset(sb);
-
- while (1) {
- char ch;
- ssize_t len = xread(fd, &ch, 1);
- if (len <= 0)
- return len;
- strbuf_addch(sb, ch);
- if (ch == '\n')
- break;
- }
- return 0;
-}
-
static int parse_bundle_header(int fd, struct bundle_header *header,
const char *report_path)
{
diff --git a/strbuf.c b/strbuf.c
index ff0b96b..7532a13 100644
--- a/strbuf.c
+++ b/strbuf.c
@@ -282,6 +282,22 @@ void strbuf_addbuf_percentquote(struct strbuf
*dst, const struct strbuf *src)
}
}
+int strbuf_readline_fd(struct strbuf *sb, int fd)
+{
+ strbuf_reset(sb);
+
+ while (1) {
+ char ch;
+ ssize_t len = xread(fd, &ch, 1);
+ if (len <= 0)
+ return len;
+ strbuf_addch(sb, ch);
+ if (ch == '\n')
+ break;
+ }
+ return 0;
+}
+
size_t strbuf_fread(struct strbuf *sb, size_t size, FILE *f)
{
size_t res;
diff --git a/strbuf.h b/strbuf.h
index fbf059f..ecebd11 100644
--- a/strbuf.h
+++ b/strbuf.h
@@ -109,6 +109,7 @@ static inline void strbuf_complete_line(struct strbuf *sb)
}
extern size_t strbuf_fread(struct strbuf *, size_t, FILE *);
+extern int strbuf_readline_fd(struct strbuf *sb, int fd);
/* XXX: if read fails, any partial read is undone */
extern ssize_t strbuf_read(struct strbuf *, int fd, size_t hint);
extern int strbuf_read_file(struct strbuf *sb, const char *path, size_t hint);
--
1.7.8.1.749.gb6b3b
^ permalink raw reply related
* Re: [PATCH 1/2] pack-refs: remove all empty directories under $GIT_DIR/refs
From: Nguyen Thai Ngoc Duy @ 2012-02-11 8:55 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Jeff King
In-Reply-To: <7vhayxn5cg.fsf@alter.siamese.dyndns.org>
2012/2/11 Junio C Hamano <gitster@pobox.com>:
> Nguyễn Thái Ngọc Duy <pclouds@gmail.com> writes:
>
>> Deleting refs does not remove parent directories if they are empty.
>> Empty directories add extra overhead to startup time of most of git
>> commands because they have to traverse $GIT_DIR/refs.
>
> Perhaps drop the first line and replace with the description of what you
> do differently from the first round?
>
> "git pack-refs" tries to remove directory that becomes empty but it
> does not try to do so hard enough, leaving a parent directory full of
> empty children directories without removing.
Sure.
> While I agree with Peff that people would expect doing other things while
> pack-refs is running would be much "riskier" and doing this inside
> pack-refs is far more preferable than doing so during normal read-only
> operation, I wonder why we would want a completely separate pass that
> scans the entire hierarchy
Less complex code. Doing it in one pass, I think get_ref_dir() needs
to learn read-only vs read-write mode and I haven't figured out a
non-ugly way to do it.
> Would it make more sense to note the
> directory for which rmdir() fails in try_remove_empty_parents(), and
> revisit only these directories, at least?
That would leave empty directories not sharing the ref's path until
the failed rmdir() unexamined, I think.
> Wouldn't we want to rmdir() the corresponding logs/ hierarchy while at it
> to be consistent?
Good idea.
>> + subpath = xmalloc(pathlen + 257);
>
> What is this 257 about?
This function is a ripoff from get_ref_dir(). I think 257 is 255 below
plus '/' and NIL.
>> + if (namelen > 255)
>> + continue;
--
Duy
^ permalink raw reply
* Re: [PATCH 1/2] pack-refs: remove all empty directories under $GIT_DIR/refs
From: Junio C Hamano @ 2012-02-11 8:26 UTC (permalink / raw)
To: Nguyễn Thái Ngọc Duy; +Cc: git, Junio C Hamano, Jeff King
In-Reply-To: <1328946907-31650-1-git-send-email-pclouds@gmail.com>
Nguyễn Thái Ngọc Duy <pclouds@gmail.com> writes:
> Deleting refs does not remove parent directories if they are empty.
> Empty directories add extra overhead to startup time of most of git
> commands because they have to traverse $GIT_DIR/refs.
Perhaps drop the first line and replace with the description of what you
do differently from the first round?
"git pack-refs" tries to remove directory that becomes empty but it
does not try to do so hard enough, leaving a parent directory full of
empty children directories without removing.
or something?
> Some directories are kept by this patch even if they are empty (refs,
> refs/heads and refs/tags). The first one is one of git repository
> signature. The rest is created by init-db, one may expect them to always
> be there.
>
> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
> ---
> v2, no more refs code change.
>
> Part of the reason I do not want to update delete_ref() is because it
> won't remove empty directories in existing repositories.
While I agree with Peff that people would expect doing other things while
pack-refs is running would be much "riskier" and doing this inside
pack-refs is far more preferable than doing so during normal read-only
operation, I wonder why we would want a completely separate pass that
scans the entire hierarchy. Would it make more sense to note the
directory for which rmdir() fails in try_remove_empty_parents(), and
revisit only these directories, at least?
Wouldn't we want to rmdir() the corresponding logs/ hierarchy while at it
to be consistent?
>
> pack-refs.c | 53 +++++++++++++++++++++++++++++++++++++++++++++++++++++
> 1 files changed, 53 insertions(+), 0 deletions(-)
>
> diff --git a/pack-refs.c b/pack-refs.c
> index f09a054..bb3a9c4 100644
> --- a/pack-refs.c
> +++ b/pack-refs.c
> @@ -91,6 +91,58 @@ static void try_remove_empty_parents(char *name)
> }
> }
>
> +static int prune_empty_dirs(const char *path)
> +{
> + int nr_entries = 0, pathlen = strlen(path);
> + DIR *dir;
> + struct dirent *de;
> + char *subpath;
> +
> + dir = opendir(git_path("%s", path));
> +
> + if (!dir)
> + return 0;
> +
> + subpath = xmalloc(pathlen + 257);
What is this 257 about?
> + memcpy(subpath, path, pathlen);
> + if (pathlen && path[pathlen-1] != '/')
> + subpath[pathlen++] = '/';
> +
> + while ((de = readdir(dir)) != NULL) {
> + struct stat st;
> + int namelen;
> +
> + if (de->d_name[0] == '.') {
> + if (strcmp(de->d_name, "..") && strcmp(de->d_name, "."))
> + nr_entries++;
> + continue;
> + }
> + nr_entries++;
> + namelen = strlen(de->d_name);
> + if (namelen > 255)
> + continue;
> + if (has_extension(de->d_name, ".lock"))
> + continue;
This is a sign that somebody else might be actively accessing this
repository.
> + memcpy(subpath + pathlen, de->d_name, namelen+1);
> + if (stat(git_path("%s", subpath), &st) < 0)
> + continue;
> + if (S_ISDIR(st.st_mode)) {
> + int removed = prune_empty_dirs(subpath);
> + if (removed)
> + nr_entries--;
> + continue;
> + }
> + }
> + free(subpath);
> + closedir(dir);
> + if (nr_entries == 0 &&
> + strcmp(path, "refs") &&
> + strcmp(path, "refs/heads") &&
> + strcmp(path, "refs/tags"))
> + return rmdir(git_path("%s", path)) == 0;
> + return 0;
> +}
> +
> /* make sure nobody touched the ref, and unlink */
> static void prune_ref(struct ref_to_prune *r)
> {
> @@ -109,6 +161,7 @@ static void prune_refs(struct ref_to_prune *r)
> prune_ref(r);
> r = r->next;
> }
> + prune_empty_dirs("refs");
> }
>
> static struct lock_file packed;
^ permalink raw reply
* Re: [RFC/PATCH] tag: make list exclude !<pattern>
From: Junio C Hamano @ 2012-02-11 8:13 UTC (permalink / raw)
To: Michael Haggerty; +Cc: Tom Grennan, pclouds, git, krh, jasampler
In-Reply-To: <4F361DD4.9020108@alum.mit.edu>
Michael Haggerty <mhagger@alum.mit.edu> writes:
> The proposal, amended to use "^" instead of "!", is that
>
> git for-each-ref A B ^C
>
> should mean "the reference names A and B but not C". Therefore, the command
>
> git rev-list $(git for-each-ref A B ^C)
>
> , which consistency suggests should do the same thing as the first
> command,...
That is an utter rubbish that does not even deserve a response.
Your argument is like saying
git for-each-ref A
and
git for-each-ref $(git rev-parse A)
should somehow magically produce the same (or related) result. The
for-each-ref command operates on refname patterns, while rev-list and
rev-parse takes object names.
^ permalink raw reply
* Re: [PATCHv2 1/4] refs: add common refname_match_patterns()
From: Junio C Hamano @ 2012-02-11 8:06 UTC (permalink / raw)
To: Tom Grennan; +Cc: pclouds, git, krh, jasampler
In-Reply-To: <1328926618-17167-2-git-send-email-tmgrennan@gmail.com>
Tom Grennan <tmgrennan@gmail.com> writes:
> +int refname_match_patterns(const char **patterns, const char *refname)
> +{
> + int given_match_pattern = 0, had_match = 0;
> +
> + for (; *patterns; patterns++)
> + if (**patterns != '!') {
> + given_match_pattern = 1;
> + if (!fnmatch(*patterns, refname, 0))
> + had_match = 1;
> + } else if (!fnmatch(*patterns+1, refname, 0))
> + return 0;
> + return given_match_pattern ? had_match : 1;
> +}
This, while its semantics seem sane, is highly inefficient when you have
many patterns, and you will be calling this to filter dozens of refs. And
it can trivially improved by first pre-parsing the pattern[] array.
* If you know the patterns do not have any negative entry, you can return
true upon seeing the first match. Because you do not pre-parse the
pattern[] array, this loop does not know if there is any negative one,
and has to scan it always all the way.
* If you arrange the pattern[] array so that it has negative ones early,
again, you can return false upon seeing the first hit with a negative
one. If your input has negative ones at the end, the loop ends up
scanning all the way, noting the positive matches, only to discard upon
seeing the negative match at the end.
That is why I said Nguyen's idea of reusing pathspec matching logic
somewhat attractive, even though I think it has downsides (the exact
matching logic for pathspec is more similar to that of for-each-ref
and very different from branch/tag).
^ permalink raw reply
* Re: Git SSH Authentication
From: Junio C Hamano @ 2012-02-11 7:54 UTC (permalink / raw)
To: Sitaram Chamarty; +Cc: isawk, git
In-Reply-To: <CAMK1S_jmY5KvBH8z6YKszroMai4O5ULeCBYGAGFT4CgVUAfmwg@mail.gmail.com>
Sitaram Chamarty <sitaramc@gmail.com> writes:
> Common causes of pubkey access fail:
>
> - wrong pubkey being offered: if you are using ssh-agent, make sure
> you have 'ssh-add'ed the key you want to offer. Confirm with 'ssh-add
> -l'
A failure related to this I saw is to have (too) many keys in ssh-agent,
and running ssh without telling it which exact key to use. The client
tries each key in turn and the server rejects the connection attempt after
seeing too many keys tried. "ssh -v" is useful to diagnose this mode of
failure, and an entry in ~/.ssh/config like:
Host example.com
User myusernameoverthere
IdentityFile ~/.ssh/id_rsa-for-example.com
would fix it.
^ permalink raw reply
* Re: [RFC/PATCH] tag: make list exclude !<pattern>
From: Michael Haggerty @ 2012-02-11 7:50 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Tom Grennan, pclouds, git, krh, jasampler
In-Reply-To: <7vaa4qnk4u.fsf@alter.siamese.dyndns.org>
On 02/11/2012 04:06 AM, Junio C Hamano wrote:
> Tom Grennan <tmgrennan@gmail.com> writes:
>> The following series implements !<pattern> with: git-tag, git-branch, and
>> git-for-each-ref.
>>
>> This still requires Documentation and unit test updates but I think these are
>> close to functionally complete.
>
> Do we allow a refname whose pathname component begins with '!', by the
> way? If we do, how does a user look for a tag whose name is "!xyzzy"?
> "Naming your tag !xyzzy used to be allowed but it is now forbidden after
> this patch" is not an acceptable answer---it is called a regression. If
> the negation operator were "^" or something that we explicitly forbid from
> a refname, we wouldn't have such a problem.
According to git-check-ref-format(1), '!' are allowed in reference
names. (Whether that was a good idea is another question, but now we
have to support it.)
So using "^" would be an option. A problem is that the new meaning is
not consistent with the use of "^" in rev-list, and therefore would (1)
be confusing and (2) prevent the addition of a similar syntax in rev-list.
Currently,
git rev-list A B ^C
means "revisions reachable from A or B but not from C". (For simplicity
assume that A, B, and C are literal reference names without wildcards.)
The proposal, amended to use "^" instead of "!", is that
git for-each-ref A B ^C
should mean "the reference names A and B but not C". Therefore, the command
git rev-list $(git for-each-ref A B ^C)
, which consistency suggests should do the same thing as the first
command, would in fact be equivalent to
git rev-list A B
Moreover, it *would* be nice to have this kind of exclude-branch-name
syntax in rev-parse. Many times I have wanted to type the equivalent of
gitk --all --not-branch=remotes/korg/*
That is, "show the commits starting at *all branches except for the
specified branches*". This is different than "show *the commits
starting at all branches* except for *the commits reachable from the
specified branches*", because I want to see the *complete* history of
the non-excluded branches. (Is there an easy way to do this now?) The
proposed functionality is a step forward; I could type
gitk $(git for-each-ref !remotes/korg/*)
But it would be even nicer if this could be expressed directly in rev-parse.
In summary, I suggest we consider using a more verbose syntax for this
new functionality (probably via one or more new options, for example
--not-branch=PATTERN, --not-tag=PATTERN, and/or --not-ref=PATTERN) that
cannot be confused with the existing syntax for excluding commits.
Michael
--
Michael Haggerty
mhagger@alum.mit.edu
http://softwareswirl.blogspot.com/
^ permalink raw reply
* Re: [RFC/PATCH] tag: make list exclude !<pattern>
From: Junio C Hamano @ 2012-02-11 7:50 UTC (permalink / raw)
To: Tom Grennan; +Cc: pclouds, git, krh, jasampler
In-Reply-To: <7vaa4qnk4u.fsf@alter.siamese.dyndns.org>
Junio C Hamano <gitster@pobox.com> writes:
> ... Same
> for 1c, which I highly suspect will introduce regression without
> looking at the code (for-each-ref is prefix-match only), ...
This part needs correction. for-each-ref matches the command line
arguments differently from branch --list and tag --list in two important
ways.
(1) It allows (not "only" which was a mistake in my earlier message)
prefix matching, e.g. "for-each-ref refs/heads/", in addition to
fnmatch(); and
(2) The fnmatch() call is made with FNM_PATHMAME, which "branch --list"
and "tag --list" does not use.
Strictly speaking, therefore, if you make all three commands to use the
same matching logic, there is no way to avoid regression. If you choose
to use fnmatch() without FNM_PATHNAME, then for-each-ref suddenly starts
matching wildcards across name hierarchy boundary '/' for a pattern that
does not match today, e.g. "git for-each-ref 'refs/heads/*'" was a good
way to grab only the integration branches while excluding individual topic
branches such as refs/heads/tg/tag-points-at, but this technique can no
longer be used for such a purpose, which is an unpleasant regression.
I personally think that it was an annoying UI mistake that we let branch
and tag call fnmatch without FNM_PATHNAME, but we cannot fix it lightly,
either. People who use hierchical branch names (e.g. maint-1.0/$topic,
maint-2.0/$topic, and feature-2.0/$topic) may already be used to list all
the topics on the maintenance tracks with "branch --list 'maint*'", and we
need to keep "branch --list" and "tag --list" working as they expect.
One possible way forward (now I am talking about a longer term solution)
would be to introduce
refname_match_pattern(const char *refname,
const char **pattern,
unsigned flags);
where flags can tell the implementation if FNM_PATHNAME should be used,
and if prefix matching should be attempted, so that the three commands
share the single same matching function while still retaining their
current behaviour in the initial round. Inside the implementation, we
would use good old fnmatch(), with or without FNM_PATHNAME, depending on
the flags the caller passes.
In a future versions, we may want to have "branch/tag --list" also ask for
FNM_PATHNAME (this *is* a backward incompatible change, so it needs to be
performed across major version boundary, with backward compatibility
configurations, deprecation warnings and whole nine yards). Under the new
match function, today's "branch --list 'maint*'" needs to be spelled as
"branch --list 'maint*/*'" or something. The prefix matching is probably
safer to enable by default without causing big regression hassle if we
limit the prefix match to only patterns that end with an explicit slash,
as users already *know* today's "branch --list tg/" would not match
anything (because the pattern does not even match a brahch 'tg', so it is
unlikely they are using it and expecting only 'tg' to match), which means
that is an unlikely input we can safely give new meaning to match anything
under tg/ hierarchy.
^ permalink raw reply
* [PATCH 2/2] Revert be7c6d4 (pack-refs: remove newly empty directories)
From: Nguyễn Thái Ngọc Duy @ 2012-02-11 7:55 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Jeff King, Nguyễn Thái Ngọc Duy
In-Reply-To: <1328946907-31650-1-git-send-email-pclouds@gmail.com>
The functionality is taken over by prune_empty_dirs. Only code is
reverted. The added test remains to verify.
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
pack-refs.c | 32 --------------------------------
1 files changed, 0 insertions(+), 32 deletions(-)
diff --git a/pack-refs.c b/pack-refs.c
index bb3a9c4..746211e 100644
--- a/pack-refs.c
+++ b/pack-refs.c
@@ -60,37 +60,6 @@ static int handle_one_ref(const char *path, const unsigned char *sha1,
return 0;
}
-/*
- * Remove empty parents, but spare refs/ and immediate subdirs.
- * Note: munges *name.
- */
-static void try_remove_empty_parents(char *name)
-{
- char *p, *q;
- int i;
- p = name;
- for (i = 0; i < 2; i++) { /* refs/{heads,tags,...}/ */
- while (*p && *p != '/')
- p++;
- /* tolerate duplicate slashes; see check_refname_format() */
- while (*p == '/')
- p++;
- }
- for (q = p; *q; q++)
- ;
- while (1) {
- while (q > p && *q != '/')
- q--;
- while (q > p && *(q-1) == '/')
- q--;
- if (q == p)
- break;
- *q = '\0';
- if (rmdir(git_path("%s", name)))
- break;
- }
-}
-
static int prune_empty_dirs(const char *path)
{
int nr_entries = 0, pathlen = strlen(path);
@@ -151,7 +120,6 @@ static void prune_ref(struct ref_to_prune *r)
if (lock) {
unlink_or_warn(git_path("%s", r->name));
unlock_ref(lock);
- try_remove_empty_parents(r->name);
}
}
--
1.7.8.36.g69ee2
^ permalink raw reply related
* [PATCH 1/2] pack-refs: remove all empty directories under $GIT_DIR/refs
From: Nguyễn Thái Ngọc Duy @ 2012-02-11 7:55 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Jeff King, Nguyễn Thái Ngọc Duy
In-Reply-To: <1328891127-17150-1-git-send-email-pclouds@gmail.com>
Deleting refs does not remove parent directories if they are empty.
Empty directories add extra overhead to startup time of most of git
commands because they have to traverse $GIT_DIR/refs.
Some directories are kept by this patch even if they are empty (refs,
refs/heads and refs/tags). The first one is one of git repository
signature. The rest is created by init-db, one may expect them to always
be there.
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
v2, no more refs code change.
Part of the reason I do not want to update delete_ref() is because it
won't remove empty directories in existing repositories.
pack-refs.c | 53 +++++++++++++++++++++++++++++++++++++++++++++++++++++
1 files changed, 53 insertions(+), 0 deletions(-)
diff --git a/pack-refs.c b/pack-refs.c
index f09a054..bb3a9c4 100644
--- a/pack-refs.c
+++ b/pack-refs.c
@@ -91,6 +91,58 @@ static void try_remove_empty_parents(char *name)
}
}
+static int prune_empty_dirs(const char *path)
+{
+ int nr_entries = 0, pathlen = strlen(path);
+ DIR *dir;
+ struct dirent *de;
+ char *subpath;
+
+ dir = opendir(git_path("%s", path));
+
+ if (!dir)
+ return 0;
+
+ subpath = xmalloc(pathlen + 257);
+ memcpy(subpath, path, pathlen);
+ if (pathlen && path[pathlen-1] != '/')
+ subpath[pathlen++] = '/';
+
+ while ((de = readdir(dir)) != NULL) {
+ struct stat st;
+ int namelen;
+
+ if (de->d_name[0] == '.') {
+ if (strcmp(de->d_name, "..") && strcmp(de->d_name, "."))
+ nr_entries++;
+ continue;
+ }
+ nr_entries++;
+ namelen = strlen(de->d_name);
+ if (namelen > 255)
+ continue;
+ if (has_extension(de->d_name, ".lock"))
+ continue;
+ memcpy(subpath + pathlen, de->d_name, namelen+1);
+ if (stat(git_path("%s", subpath), &st) < 0)
+ continue;
+ if (S_ISDIR(st.st_mode)) {
+ int removed = prune_empty_dirs(subpath);
+ if (removed)
+ nr_entries--;
+ continue;
+ }
+ }
+ free(subpath);
+ closedir(dir);
+ if (nr_entries == 0 &&
+ strcmp(path, "refs") &&
+ strcmp(path, "refs/heads") &&
+ strcmp(path, "refs/tags"))
+ return rmdir(git_path("%s", path)) == 0;
+ return 0;
+}
+
/* make sure nobody touched the ref, and unlink */
static void prune_ref(struct ref_to_prune *r)
{
@@ -109,6 +161,7 @@ static void prune_refs(struct ref_to_prune *r)
prune_ref(r);
r = r->next;
}
+ prune_empty_dirs("refs");
}
static struct lock_file packed;
--
1.7.8.36.g69ee2
^ permalink raw reply related
* Re: [PATCHv2 1/4] refs: add common refname_match_patterns()
From: Michael Haggerty @ 2012-02-11 7:12 UTC (permalink / raw)
To: Tom Grennan; +Cc: pclouds, git, gitster, krh, jasampler
In-Reply-To: <1328926618-17167-2-git-send-email-tmgrennan@gmail.com>
On 02/11/2012 03:16 AM, Tom Grennan wrote:
> diff --git a/refs.h b/refs.h
> index 00ba1e2..13015ba 100644
> --- a/refs.h
> +++ b/refs.h
> @@ -152,4 +152,12 @@ int update_ref(const char *action, const char *refname,
> const unsigned char *sha1, const unsigned char *oldval,
> int flags, enum action_on_err onerr);
>
> +/**
> + * Returns:
> + * 1 with NULL patterns
> + * 0 if refname fnmatch()es any ! prefaced pattern
> + * 1 if refname fnmatch()es any pattern
> + */
> +extern int refname_match_patterns(const char **patterns, const char *refname);
> +
> #endif /* REFS_H */
This comment is unclear and incomplete.
1. What does "NULL patterns" mean? Your code fails if patterns==NULL,
so I guess you mean "1 if there are no patterns in the list".
2. Since the three conditions are not mutually exclusive, you should say
how they are connected. I believe that you want something like "A
otherwise B otherwise C".
3. You haven't specified what happens if refname matches neither a
!-prefixed pattern nor a non-!-prefixed pattern. Does this behavior
depend on which types of patterns were present in the list?
I see that you have described the behavior more completely in the commit
message for patch 2/4, but the commit message is not enough: this
behavior should be described precisely in both code comments (when the
function is defined) and in the user documentation (when the
functionality is added to a command).
Michael
--
Michael Haggerty
mhagger@alum.mit.edu
http://softwareswirl.blogspot.com/
^ permalink raw reply
* Re: [PATCH v2 28/51] refs.c: rename ref_array -> ref_dir
From: Michael Haggerty @ 2012-02-11 6:33 UTC (permalink / raw)
To: Junio C Hamano
Cc: Jeff King, git, Drew Northup, Jakub Narebski, Heiko Voigt,
Johan Herland, Julian Phillips
In-Reply-To: <7v62fepew8.fsf@alter.siamese.dyndns.org>
On 02/10/2012 10:17 PM, Junio C Hamano wrote:
> Jeff King <peff@peff.net> writes:
>
>>> If everything_local() is trying to check that the references are in the
>>> local repository plus alternates, then it is incorrect that
>>> everything_local() doesn't consider alternate references in its
>>> determination. My guess is that this is the case, and that something
>>> like the following might be the fix:
>>
>> Junio could answer more authoritatively than I, but I am pretty sure it
>> is the latter. The point is to skip the expensive find_common
>> negotiation if we know that there are no objects to fetch. Thus the
>> "local" here is "do we have them on this side of the git-protocol
>> connection", not "do we have them in our non-alternates repository".
>
> Correct. The function is about "do we need to get any object from the
> other side?" optimization.
Thanks for your feedback. I have just submitted a patch series [1] that
attempts to fix this problem.
Michael
[1] http://thread.gmane.org/gmane.comp.version-control.git/190488
--
Michael Haggerty
mhagger@alum.mit.edu
http://softwareswirl.blogspot.com/
^ permalink raw reply
* [PATCH 4/7] fetch-pack.c: inline insert_alternate_refs()
From: mhagger @ 2012-02-11 6:20 UTC (permalink / raw)
To: Junio C Hamano
Cc: git, Jeff King, Jakub Narebski, Heiko Voigt, Johan Herland,
Michael Haggerty
In-Reply-To: <1328941261-29746-1-git-send-email-mhagger@alum.mit.edu>
From: Michael Haggerty <mhagger@alum.mit.edu>
The logic of the (single) caller is clearer without encapsulating this
one line in a function.
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
builtin/fetch-pack.c | 7 +------
1 files changed, 1 insertions(+), 6 deletions(-)
diff --git a/builtin/fetch-pack.c b/builtin/fetch-pack.c
index 9bd2096..dbe9acb 100644
--- a/builtin/fetch-pack.c
+++ b/builtin/fetch-pack.c
@@ -256,11 +256,6 @@ static void insert_one_alternate_ref(const struct ref *ref, void *unused)
rev_list_insert_ref(NULL, ref->old_sha1, 0, NULL);
}
-static void insert_alternate_refs(void)
-{
- for_each_alternate_ref(insert_one_alternate_ref, NULL);
-}
-
#define INITIAL_FLUSH 16
#define PIPESAFE_FLUSH 32
#define LARGE_FLUSH 1024
@@ -295,7 +290,7 @@ static int find_common(int fd[2], unsigned char *result_sha1,
marked = 1;
for_each_ref(rev_list_insert_ref, NULL);
- insert_alternate_refs();
+ for_each_alternate_ref(insert_one_alternate_ref, NULL);
fetching = 0;
for ( ; refs ; refs = refs->next) {
--
1.7.9
^ permalink raw reply related
* [PATCH 7/7] refs: remove the extra_refs API
From: mhagger @ 2012-02-11 6:21 UTC (permalink / raw)
To: Junio C Hamano
Cc: git, Jeff King, Jakub Narebski, Heiko Voigt, Johan Herland,
Michael Haggerty
In-Reply-To: <1328941261-29746-1-git-send-email-mhagger@alum.mit.edu>
From: Michael Haggerty <mhagger@alum.mit.edu>
The extra_refs provided a kludgy way to create fake references at a
global level in the hope that they would only affect some particular
code path. The last user of this API been rewritten, so strip this
stuff out before somebody else gets the bad idea of using it.
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
refs.c | 23 +----------------------
refs.h | 8 --------
2 files changed, 1 insertions(+), 30 deletions(-)
diff --git a/refs.c b/refs.c
index b8843bb..c9f6835 100644
--- a/refs.c
+++ b/refs.c
@@ -183,12 +183,6 @@ static struct ref_cache {
static struct ref_entry *current_ref;
-/*
- * Never call sort_ref_array() on the extra_refs, because it is
- * allowed to contain entries with duplicate names.
- */
-static struct ref_array extra_refs;
-
static void clear_ref_array(struct ref_array *array)
{
int i;
@@ -289,16 +283,6 @@ static void read_packed_refs(FILE *f, struct ref_array *array)
}
}
-void add_extra_ref(const char *refname, const unsigned char *sha1, int flag)
-{
- add_ref(&extra_refs, create_ref_entry(refname, sha1, flag, 0));
-}
-
-void clear_extra_refs(void)
-{
- clear_ref_array(&extra_refs);
-}
-
static struct ref_array *get_packed_refs(struct ref_cache *refs)
{
if (!refs->did_packed) {
@@ -733,16 +717,11 @@ fallback:
static int do_for_each_ref(const char *submodule, const char *base, each_ref_fn fn,
int trim, int flags, void *cb_data)
{
- int retval = 0, i, p = 0, l = 0;
+ int retval = 0, p = 0, l = 0;
struct ref_cache *refs = get_ref_cache(submodule);
struct ref_array *packed = get_packed_refs(refs);
struct ref_array *loose = get_loose_refs(refs);
- struct ref_array *extra = &extra_refs;
-
- for (i = 0; i < extra->nr; i++)
- retval = do_one_ref(base, fn, trim, flags, cb_data, extra->refs[i]);
-
sort_ref_array(packed);
sort_ref_array(loose);
while (p < packed->nr && l < loose->nr) {
diff --git a/refs.h b/refs.h
index 00ba1e2..33202b0 100644
--- a/refs.h
+++ b/refs.h
@@ -56,14 +56,6 @@ extern void warn_dangling_symref(FILE *fp, const char *msg_fmt, const char *refn
*/
extern void add_packed_ref(const char *refname, const unsigned char *sha1);
-/*
- * Extra refs will be listed by for_each_ref() before any actual refs
- * for the duration of this process or until clear_extra_refs() is
- * called. Only extra refs added before for_each_ref() is called will
- * be listed on a given call of for_each_ref().
- */
-extern void add_extra_ref(const char *refname, const unsigned char *sha1, int flags);
-extern void clear_extra_refs(void);
extern int ref_exists(const char *);
extern int peel_ref(const char *refname, unsigned char *sha1);
--
1.7.9
^ permalink raw reply related
* [PATCH 5/7] everything_local(): mark alternate refs as complete
From: mhagger @ 2012-02-11 6:20 UTC (permalink / raw)
To: Junio C Hamano
Cc: git, Jeff King, Jakub Narebski, Heiko Voigt, Johan Herland,
Michael Haggerty
In-Reply-To: <1328941261-29746-1-git-send-email-mhagger@alum.mit.edu>
From: Michael Haggerty <mhagger@alum.mit.edu>
Objects in an alternate object database are already available to the
local repository and therefore don't need to be fetched. So mark them
as complete in everything_local().
This fixes a test in t5700.
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
builtin/fetch-pack.c | 6 ++++++
t/t5700-clone-reference.sh | 2 +-
2 files changed, 7 insertions(+), 1 deletions(-)
diff --git a/builtin/fetch-pack.c b/builtin/fetch-pack.c
index dbe9acb..0e8560f 100644
--- a/builtin/fetch-pack.c
+++ b/builtin/fetch-pack.c
@@ -581,6 +581,11 @@ static void filter_refs(struct ref **refs, int nr_match, char **match)
*refs = newlist;
}
+static void mark_alternate_complete(const struct ref *ref, void *unused)
+{
+ mark_complete(NULL, ref->old_sha1, 0, NULL);
+}
+
static int everything_local(struct ref **refs, int nr_match, char **match)
{
struct ref *ref;
@@ -609,6 +614,7 @@ static int everything_local(struct ref **refs, int nr_match, char **match)
if (!args.depth) {
for_each_ref(mark_complete, NULL);
+ for_each_alternate_ref(mark_alternate_complete, NULL);
if (cutoff)
mark_recent_complete_commits(cutoff);
}
diff --git a/t/t5700-clone-reference.sh b/t/t5700-clone-reference.sh
index 2dafee8..783f988 100755
--- a/t/t5700-clone-reference.sh
+++ b/t/t5700-clone-reference.sh
@@ -167,7 +167,7 @@ test_expect_success 'prepare branched repository' '
rm -f "$U.K"
-test_expect_failure 'fetch with incomplete alternates' '
+test_expect_success 'fetch with incomplete alternates' '
git init K &&
echo "$base_dir/A/.git/objects" >K/.git/objects/info/alternates &&
(
--
1.7.9
^ permalink raw reply related
* [PATCH 6/7] clone: do not add alternate references to extra_refs
From: mhagger @ 2012-02-11 6:21 UTC (permalink / raw)
To: Junio C Hamano
Cc: git, Jeff King, Jakub Narebski, Heiko Voigt, Johan Herland,
Michael Haggerty
In-Reply-To: <1328941261-29746-1-git-send-email-mhagger@alum.mit.edu>
From: Michael Haggerty <mhagger@alum.mit.edu>
Alternate references are directly (and now, correctly) handled by
fetch-pack, so there is no need to inform fetch-pack about them via
the extra_refs back channel.
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
builtin/clone.c | 12 ------------
1 files changed, 0 insertions(+), 12 deletions(-)
diff --git a/builtin/clone.c b/builtin/clone.c
index 279fdf0..b15fccb 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -232,9 +232,6 @@ static int add_one_reference(struct string_list_item *item, void *cb_data)
{
char *ref_git;
struct strbuf alternate = STRBUF_INIT;
- struct remote *remote;
- struct transport *transport;
- const struct ref *extra;
/* Beware: real_path() and mkpath() return static buffer */
ref_git = xstrdup(real_path(item->string));
@@ -249,14 +246,6 @@ static int add_one_reference(struct string_list_item *item, void *cb_data)
strbuf_addf(&alternate, "%s/objects", ref_git);
add_to_alternates_file(alternate.buf);
strbuf_release(&alternate);
-
- remote = remote_get(ref_git);
- transport = transport_get(remote, ref_git);
- for (extra = transport_get_remote_refs(transport); extra;
- extra = extra->next)
- add_extra_ref(extra->name, extra->old_sha1, 0);
-
- transport_disconnect(transport);
free(ref_git);
return 0;
}
@@ -500,7 +489,6 @@ static void update_remote_refs(const struct ref *refs,
const char *msg)
{
if (refs) {
- clear_extra_refs();
write_remote_refs(mapped_refs);
if (option_single_branch)
write_followtags(refs, msg);
--
1.7.9
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox