* Re: Proposal: branch.<name>.remotepush
From: Jonathan Nieder @ 2013-02-07 23:30 UTC (permalink / raw)
To: Ramkumar Ramachandra; +Cc: Michael Schubert, Git List, Junio C Hamano
In-Reply-To: <CALkWK0=53riU3xKbKkyAVS8--9VoAU5P6h88MQ9-geW=H5+a-w@mail.gmail.com>
Hi Ram,
Ramkumar Ramachandra wrote:
> And yes, a regular `git push origin refs/for/master` is just retarded.
The usual incantation is "git push gerrit HEAD:refs/for/master". Is
the code review creation push that uses a different branchname from
the branch the integrator pulls what seems backward, or is it the need
to specify a refname at all on the command line?
I agree that a "[branch "master"] pushremote" configuration would be
handy. pushremote instead of remotepush to be less surprising to
people who have already seen pushurl.
Good luck,
Jonathan
^ permalink raw reply
* Re: [PATCHv2 4/5] Git.pm: add interface for git credential command
From: Junio C Hamano @ 2013-02-07 23:38 UTC (permalink / raw)
To: Matthieu Moy; +Cc: Michal Nazarewicz, Jeff King, git
In-Reply-To: <vpq38x8m06f.fsf@grenoble-inp.fr>
Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> writes:
> Michal Nazarewicz <mpn@google.com> writes:
>
>> From: Michal Nazarewicz <mina86@mina86.com>
>>
>> Add a credential() function which is an interface to the git
>> credential command. The code is heavily based on credential_*
>> functions in <contrib/mw-to-git/git-remote-mediawiki>.
>
> I'm no perl expert, so I cannot comment much on style (there are many
> small changes compared to the mediawiki code that look like improvement
> though), but:
>
> Reviewed-by: Matthieu Moy <Matthieu.Moy@imag.fr>
Thanks. I'd actually be more worried about the error checking issue
Peff raised during his review. I have a feeling that "when in doubt,
do not cause harm" is a more prudent way to go than "I do not know,
so I'll let anything pass".
^ permalink raw reply
* Re: Proposal: branch.<name>.remotepush
From: Junio C Hamano @ 2013-02-07 23:41 UTC (permalink / raw)
To: Jonathan Nieder; +Cc: Ramkumar Ramachandra, Michael Schubert, Git List
In-Reply-To: <20130207233017.GD19397@google.com>
Jonathan Nieder <jrnieder@gmail.com> writes:
> The usual incantation is "git push gerrit HEAD:refs/for/master". Is
> the code review creation push that uses a different branchname from
> the branch the integrator pulls what seems backward, or is it the need
> to specify a refname at all on the command line?
>
> I agree that a "[branch "master"] pushremote" configuration would be
> handy. pushremote instead of remotepush to be less surprising to
> people who have already seen pushurl.
I'd actually see this as Gerrit being weird.
If it wants to quarantine a commit destined to the "master" branch,
couldn't it just let people push to "master" and then internally
update "for/master" instead?
^ permalink raw reply
* Re: [PATCHv6] Add contrib/credentials/netrc with GPG support
From: Junio C Hamano @ 2013-02-07 23:52 UTC (permalink / raw)
To: Ted Zlatanov; +Cc: Jeff King, git
In-Reply-To: <876226p97h.fsf_-_@lifelogs.com>
Ted Zlatanov <tzz@lifelogs.com> writes:
> Add Git credential helper that can parse netrc/authinfo files.
I think this line is redundant; we already know it on the Subject: line.
> This credential helper supports multiple files, returning the first one
> that matches. It checks file permissions and owner. For *.gpg files,
> it will run GPG to decrypt the file.
>
> Signed-off-by: Ted Zlatanov <tzz@lifelogs.com>
> ---
> ...
> diff --git a/contrib/credential/netrc/Makefile b/contrib/credential/netrc/Makefile
> new file mode 100644
> index 0000000..18a924f
> --- /dev/null
> +++ b/contrib/credential/netrc/Makefile
> @@ -0,0 +1,12 @@
> +test_netrc:
> + @(echo "bad data" | ./git-credential-netrc -f A -d -v) || echo "Bad invocation test, ignoring failure"
> + @echo "=> Silent invocation... nothing should show up here with a missing file"
> + @echo "bad data" | ./git-credential-netrc -f A get
> + @echo "=> Back to noisy: -v and -d used below, missing file"
> + echo "bad data" | ./git-credential-netrc -f A -d -v get
> + @echo "=> Look for any entry in the default file set"
> + echo "" | ./git-credential-netrc -d -v get
> + @echo "=> Look for github.com in the default file set"
> + echo "host=google.com" | ./git-credential-netrc -d -v get
> + @echo "=> Look for a nonexistent machine in the default file set"
> + echo "host=korovamilkbar" | ./git-credential-netrc -d -v get
Whose netrc is this reading?
Don't we want all of them to have "-f A" and ship "A" (rename it to
something more reasonable), so that anybody can notice when he tries
to improve it and breaks it?
^ permalink raw reply
* Re: [PATCH 2/2] count-objects: report garbage files in .git/objects/pack directory too
From: Duy Nguyen @ 2013-02-07 23:58 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7vzjzg802d.fsf@alter.siamese.dyndns.org>
On Fri, Feb 8, 2013 at 1:12 AM, Junio C Hamano <gitster@pobox.com> wrote:
> Duy Nguyen <pclouds@gmail.com> writes:
>
>> I thought about that, but we may need to do extra stat() for loose
>> garbage as well. As it is now, garbage is complained loudly, which
>> gives me enough motivation to clean up, even without looking at how
>> much disk space it uses.
>
> I wouldn't call a single line "garbage: 4" exactly *loud*. I also
> think that this is not about *motivating* you, but about giving
> more information to the users to help them assess the health of
> their repository themselves.
That's not the only line it prints:
error: garbage found:
.git/objects/pack/pack-8074dfd2b01f494a30f02d0374baa57632d26fea.commits
error: garbage found:
.git/objects/pack/pack-834c9dccca7634c2b225db1b5050a980cb2c2de0.commits
error: garbage found: .git/objects/pack/tmp_pack_G235da
error: garbage found:
.git/objects/pack/pack-8bdf298e9252573289cd4f1e83e80c9f261882a2.commits
count: 604
size: 5576
in-pack: 172307
packs: 4
size-pack: 50421
prune-packable: 4
garbage: 4
> By the way, I wonder if we also want to notice .git/objects/garbage
> or .git/objects/ga/rbage if we are to do this?
I prefer not (for code simplicity) because we may need to catch
.git/objects/pack/info/garbage too if we do that.
>> + if (!has_extension(de->d_name, ".pack")) {
>> + report_pack_garbage(path, 0, NULL);
>> + continue;
>> + }
>
> Didn't I already say the logic should be inverted to whitelist the
> known ones? Saying "Anything that is not '.pack' is bad" here is a
> direct opposite, I think.
I must have missed it. Will do.
>
> Add "A '.keep' file is OK" to this codeflow and see how it goes.
OK
--
Duy
^ permalink raw reply
* [ANNOUNCE] Git v1.8.1.3
From: Junio C Hamano @ 2013-02-08 0:13 UTC (permalink / raw)
To: git; +Cc: Linux Kernel
The latest maintenance release Git v1.8.1.3 is now available at
the usual places.
The release tarballs are found at:
http://code.google.com/p/git-core/downloads/list
and their SHA-1 checksums are:
29ed9047263f9835726200226451339276641779 git-1.8.1.3.tar.gz
6b1e57bde2f2b0a86532390c15bfa7b181c50db2 git-htmldocs-1.8.1.3.tar.gz
12aaa8a0428e64d194665379ab0335d786728930 git-manpages-1.8.1.3.tar.gz
Also the following public repositories all have a copy of the v1.8.1.3
tag and the maint branch that the tag points at:
url = git://repo.or.cz/alt-git.git
url = https://code.google.com/p/git-core/
url = git://git.sourceforge.jp/gitroot/git-core/git.git
url = git://git-core.git.sourceforge.net/gitroot/git-core/git-core
url = https://github.com/gitster/git
Git 1.8.1.3 Release Notes
=========================
Fixes since v1.8.1.2
--------------------
* The attribute mechanism didn't allow limiting attributes to be
applied to only a single directory itself with "path/" like the
exclude mechanism does. The fix for this in 1.8.1.2 had
performance degradations.
* Command line completion code was inadvertently made incompatible with
older versions of bash by using a newer array notation.
* Scripts to test bash completion was inherently flaky as it was
affected by whatever random things the user may have on $PATH.
* A fix was added to the build procedure to work around buggy
versions of ccache broke the auto-generation of dependencies, which
unfortunately is still relevant because some people use ancient
distros.
* We used to stuff "user@" and then append what we read from
/etc/mailname to come up with a default e-mail ident, but a bug
lost the "user@" part.
* "git am" did not parse datestamp correctly from Hg generated patch,
when it is run in a locale outside C (or en).
* Attempt to "branch --edit-description" an existing branch, while
being on a detached HEAD, errored out.
* "git cherry-pick" did not replay a root commit to an unborn branch.
* We forgot to close the file descriptor reading from "gpg" output,
killing "git log --show-signature" on a long history.
* "git rebase --preserve-merges" lost empty merges in recent versions
of Git.
* Rebasing the history of superproject with change in the submodule
has been broken since v1.7.12.
* A failure to push due to non-ff while on an unborn branch
dereferenced a NULL pointer when showing an error message.
Also contains various documentation fixes.
----------------------------------------------------------------
Changes since v1.8.1.2 are as follows:
Brandon Casey (3):
git-completion.bash: replace zsh notation that breaks bash 3.X
git-p4.py: support Python 2.5
git-p4.py: support Python 2.4
Dmitry V. Levin (1):
am: invoke perl's strftime in C locale
Fraser Tweedale (1):
push: fix segfault when HEAD points nowhere
John Keeping (1):
git-cvsimport.txt: cvsps-2 is deprecated
Jonathan Nieder (2):
ident: do not drop username when reading from /etc/mailname
Makefile: explicitly set target name for autogenerated dependencies
Junio C Hamano (17):
Which merge_file() function do you mean?
merge-tree: lose unused "flags" from merge_list
merge-tree: lose unused "resolve_directories"
merge-tree: add comments to clarify what these functions are doing
merge-tree: fix d/f conflicts
Documentation: update "howto maintain git"
howto/maintain: mark titles for asciidoc
help: include <common-cmds.h> only in one file
t9902: protect test from stray build artifacts
howto/maintain: document "### match next" convention in jch/pu branch
README: update stale and/or incorrect information
INSTALL: git-p4 does not support Python 3
git-am: record full index line in the patch used while rebasing
apply: simplify build_fake_ancestor()
apply: diagnose incomplete submodule object name better
Start preparing for 1.8.1.3
Git 1.8.1.3
Martin von Zweigbergk (2):
tests: move test_cmp_rev to test-lib-functions
learn to pick/revert into unborn branch
Nguyễn Thái Ngọc Duy (4):
attr: fix off-by-one directory component length calculation
test-lib.sh: unfilter GIT_PERF_*
attr: avoid calling find_basename() twice per path
branch: no detached HEAD check when editing another branch's description
Phil Hord (1):
rebase --preserve-merges: keep all merge commits including empty ones
Ramsay Allan Jones (1):
Makefile: Replace merge-file.h with merge-blobs.h in LIB_H
Stephen Boyd (1):
gpg: close stderr once finished with it in verify_signed_buffer()
Torsten Bögershausen (3):
t0050: known breakage vanished in merge (case change)
t0050: honor CASE_INSENSITIVE_FS in add (with different case)
t0050: Use TAB for indentation
^ permalink raw reply
* [PATCH] git-mergetool: print filename when it contains %
From: Asheesh Laroia @ 2013-02-08 1:16 UTC (permalink / raw)
To: git; +Cc: Asheesh Laroia
Before this change, if git-mergetool was invoked with regard to
files with a percent sign (%) in their names, it would print an
error. For example, if you were calling mergetool on a file called
"%2F":
printf: %2F: invalid directive
This changes the behavior to pass "%s" to printf as its first argument
to avoid processing the filename as a format string.
Signed-off-by: Asheesh Laroia <asheesh@asheesh.org>
---
git-mergetool.sh | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/git-mergetool.sh b/git-mergetool.sh
index c50e18a..d2b9289 100755
--- a/git-mergetool.sh
+++ b/git-mergetool.sh
@@ -440,7 +440,7 @@ then
fi
printf "Merging:\n"
-printf "$files\n"
+printf "%s" "$files\n"
IFS='
'
--
1.7.10.4
^ permalink raw reply related
* Re: [PATCHv2 4/5] Git.pm: add interface for git credential command
From: Michal Nazarewicz @ 2013-02-08 1:37 UTC (permalink / raw)
To: Junio C Hamano, Matthieu Moy; +Cc: Jeff King, git
In-Reply-To: <7v7gmj66fq.fsf@alter.siamese.dyndns.org>
[-- Attachment #1: Type: text/plain, Size: 654 bytes --]
On Fri, Feb 08 2013, Junio C Hamano wrote:
> I'd actually be more worried about the error checking issue
> Peff raised during his review. I have a feeling that "when in doubt,
> do not cause harm" is a more prudent way to go than "I do not know,
> so I'll let anything pass".
I can implement whatever checking you wish, just tell me what to do in
corner cases I've listed. ;)
--
Best regards, _ _
.o. | Liege of Serenely Enlightened Majesty of o' \,=./ `o
..o | Computer Science, Michał “mina86” Nazarewicz (o o)
ooo +----<email/xmpp: mpn@google.com>--------------ooO--(_)--Ooo--
[-- Attachment #2.1: Type: text/plain, Size: 0 bytes --]
[-- Attachment #2.2: Type: application/pgp-signature, Size: 835 bytes --]
^ permalink raw reply
* Re: [PATCHv6] Add contrib/credentials/netrc with GPG support
From: Ted Zlatanov @ 2013-02-08 1:53 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Jeff King, git
In-Reply-To: <7vtxpn4r7a.fsf@alter.siamese.dyndns.org>
On Thu, 07 Feb 2013 15:52:41 -0800 Junio C Hamano <gitster@pobox.com> wrote:
>> + @echo "=> Look for any entry in the default file set"
>> + echo "" | ./git-credential-netrc -d -v get
>> + @echo "=> Look for github.com in the default file set"
>> + echo "host=google.com" | ./git-credential-netrc -d -v get
>> + @echo "=> Look for a nonexistent machine in the default file set"
>> + echo "host=korovamilkbar" | ./git-credential-netrc -d -v get
JCH> Whose netrc is this reading?
JCH> Don't we want all of them to have "-f A" and ship "A" (rename it to
JCH> something more reasonable), so that anybody can notice when he tries
JCH> to improve it and breaks it?
I agree this Makefile is not a good test to ship out. It was my quickie
test rig that I should have reworked before adding to the patch. Sorry.
I see contrib/subtree/t and contrib/mw-to-git/t that I could copy. The
test will have a few files to parse, and will be able to compare the
expected to the actual output. Does that sound like a good plan?
Ted
^ permalink raw reply
* [PATCH v4] Add utf8_fprintf helper which returns correct columns
From: Jiang Xin @ 2013-02-08 2:10 UTC (permalink / raw)
To: Junio C Hamano, Nguyễn Thái Ngọc Duy
Cc: Git List, Jiang Xin, Nguyễn Thái Ngọc Duy
In-Reply-To: <7va9rho350.fsf@alter.siamese.dyndns.org>
Since command usages can be translated, they may not align well especially
when they are translated to CJK. A wrapper utf8_fprintf can help to return
the correct columns required.
Signed-off-by: Jiang Xin <worldhello.net@gmail.com>
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
parse-options.c | 5 +++--
utf8.c | 22 ++++++++++++++++++++++
utf8.h | 1 +
3 files changed, 26 insertions(+), 2 deletions(-)
diff --git a/parse-options.c b/parse-options.c
index 67e98..a6ce9e 100644
--- a/parse-options.c
+++ b/parse-options.c
@@ -3,6 +3,7 @@
#include "cache.h"
#include "commit.h"
#include "color.h"
+#include "utf8.h"
static int parse_options_usage(struct parse_opt_ctx_t *ctx,
const char * const *usagestr,
@@ -482,7 +483,7 @@ static int usage_argh(const struct option *opts, FILE *outfile)
s = literal ? "[%s]" : "[<%s>]";
else
s = literal ? " %s" : " <%s>";
- return fprintf(outfile, s, opts->argh ? _(opts->argh) : _("..."));
+ return utf8_fprintf(outfile, s, opts->argh ? _(opts->argh) : _("..."));
}
#define USAGE_OPTS_WIDTH 24
@@ -541,7 +542,7 @@ static int usage_with_options_internal(struct parse_opt_ctx_t *ctx,
if (opts->long_name)
pos += fprintf(outfile, "--%s", opts->long_name);
if (opts->type == OPTION_NUMBER)
- pos += fprintf(outfile, "-NUM");
+ pos += utf8_fprintf(outfile, _("-NUM"));
if ((opts->flags & PARSE_OPT_LITERAL_ARGHELP) ||
!(opts->flags & PARSE_OPT_NOARG))
diff --git a/utf8.c b/utf8.c
index a4ee6..05925 100644
--- a/utf8.c
+++ b/utf8.c
@@ -430,6 +430,28 @@ int same_encoding(const char *src, const char *dst)
}
/*
+ * Wrapper for fprintf and returns the total number of columns required
+ * for the printed string, assuming that the string is utf8.
+ */
+int utf8_fprintf(FILE *stream, const char *format, ...)
+{
+ struct strbuf buf = STRBUF_INIT;
+ va_list arg;
+ int columns;
+
+ va_start (arg, format);
+ strbuf_vaddf(&buf, format, arg);
+ va_end (arg);
+
+ columns = fputs(buf.buf, stream);
+ /* If no error occurs, returns columns really required with utf8_strwidth. */
+ if (0 <= columns)
+ columns = utf8_strwidth(buf.buf);
+ strbuf_release(&buf);
+ return columns;
+}
+
+/*
* Given a buffer and its encoding, return it re-encoded
* with iconv. If the conversion fails, returns NULL.
*/
diff --git a/utf8.h b/utf8.h
index a2142..501b2 100644
--- a/utf8.h
+++ b/utf8.h
@@ -8,6 +8,7 @@ int utf8_strwidth(const char *string);
int is_utf8(const char *text);
int is_encoding_utf8(const char *name);
int same_encoding(const char *, const char *);
+int utf8_fprintf(FILE *, const char *, ...);
void strbuf_add_wrapped_text(struct strbuf *buf,
const char *text, int indent, int indent2, int width);
--
1.8.1.1.368.g917cd65
^ permalink raw reply
* [PATCH v2 0/3] count-objects improvements
From: Nguyễn Thái Ngọc Duy @ 2013-02-08 3:48 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Nguyễn Thái Ngọc Duy
In-Reply-To: <1359982145-10792-1-git-send-email-pclouds@gmail.com>
This series:
- updates count-objects -v documentation, describe each line in detail
- counts garbage files in pack directory in addition to loose odb
- shows how much disk space consumed by garbage files
Nguyễn Thái Ngọc Duy (3):
git-count-objects.txt: describe each line in -v output
count-objects: report garbage files in pack directory too
count-objects: report how much disk space taken by garbage files
Documentation/git-count-objects.txt | 22 +++++++---
builtin/count-objects.c | 41 ++++++++++++++-----
sha1_file.c | 81 +++++++++++++++++++++++++++++++++++--
3 files changed, 127 insertions(+), 17 deletions(-)
--
1.8.1.2.495.g3fdf5d5.dirty
^ permalink raw reply
* [PATCH v2 1/3] git-count-objects.txt: describe each line in -v output
From: Nguyễn Thái Ngọc Duy @ 2013-02-08 3:48 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Nguyễn Thái Ngọc Duy
In-Reply-To: <1360295307-5469-1-git-send-email-pclouds@gmail.com>
The current description requires a bit of guessing (what clause
corresponds to what printed line?) and lacks information, such as
the unit of size and size-pack.
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
Documentation/git-count-objects.txt | 20 +++++++++++++++-----
1 file changed, 15 insertions(+), 5 deletions(-)
diff --git a/Documentation/git-count-objects.txt b/Documentation/git-count-objects.txt
index 23c80ce..e816823 100644
--- a/Documentation/git-count-objects.txt
+++ b/Documentation/git-count-objects.txt
@@ -20,11 +20,21 @@ OPTIONS
-------
-v::
--verbose::
- In addition to the number of loose objects and disk
- space consumed, it reports the number of in-pack
- objects, number of packs, disk space consumed by those packs,
- and number of objects that can be removed by running
- `git prune-packed`.
+ Report in more detail:
++
+count: the number of loose objects
++
+size: disk space consumed by loose objects, in KiB
++
+in-pack: the number of in-pack objects
++
+size-pack: disk space consumed by the packs, in KiB
++
+prune-packable: the number of loose objects that are also present in
+the packs. These objects could be pruned using `git prune-packed`.
++
+garbage: the number of files in loose object database that are not
+valid loose objects
GIT
---
--
1.8.1.2.495.g3fdf5d5.dirty
^ permalink raw reply related
* [PATCH v2 2/3] count-objects: report garbage files in pack directory too
From: Nguyễn Thái Ngọc Duy @ 2013-02-08 3:48 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Nguyễn Thái Ngọc Duy
In-Reply-To: <1360295307-5469-1-git-send-email-pclouds@gmail.com>
prepare_packed_git_one() is modified to allow count-objects to hook a
report function to so we don't need to duplicate the pack searching
logic in count-objects.c. When report_pack_garbage is NULL, the
overhead is insignificant.
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
Documentation/git-count-objects.txt | 4 +-
builtin/count-objects.c | 18 ++++++++-
sha1_file.c | 81 +++++++++++++++++++++++++++++++++++--
3 files changed, 97 insertions(+), 6 deletions(-)
diff --git a/Documentation/git-count-objects.txt b/Documentation/git-count-objects.txt
index e816823..1611d7c 100644
--- a/Documentation/git-count-objects.txt
+++ b/Documentation/git-count-objects.txt
@@ -33,8 +33,8 @@ size-pack: disk space consumed by the packs, in KiB
prune-packable: the number of loose objects that are also present in
the packs. These objects could be pruned using `git prune-packed`.
+
-garbage: the number of files in loose object database that are not
-valid loose objects
+garbage: the number of files in object database that are not valid
+loose objects nor valid packs
GIT
---
diff --git a/builtin/count-objects.c b/builtin/count-objects.c
index 9afaa88..118b2ae 100644
--- a/builtin/count-objects.c
+++ b/builtin/count-objects.c
@@ -9,6 +9,20 @@
#include "builtin.h"
#include "parse-options.h"
+static unsigned long garbage;
+
+extern void (*report_pack_garbage)(const char *path, int len, const char *name);
+static void real_report_pack_garbage(const char *path, int len, const char *name)
+{
+ if (len && name)
+ error("garbage found: %.*s/%s", len, path, name);
+ else if (!len && name)
+ error("garbage found: %s%s", path, name);
+ else
+ error("garbage found: %s", path);
+ garbage++;
+}
+
static void count_objects(DIR *d, char *path, int len, int verbose,
unsigned long *loose,
off_t *loose_size,
@@ -76,7 +90,7 @@ int cmd_count_objects(int argc, const char **argv, const char *prefix)
const char *objdir = get_object_directory();
int len = strlen(objdir);
char *path = xmalloc(len + 50);
- unsigned long loose = 0, packed = 0, packed_loose = 0, garbage = 0;
+ unsigned long loose = 0, packed = 0, packed_loose = 0;
off_t loose_size = 0;
struct option opts[] = {
OPT__VERBOSE(&verbose, N_("be verbose")),
@@ -87,6 +101,8 @@ int cmd_count_objects(int argc, const char **argv, const char *prefix)
/* we do not take arguments other than flags for now */
if (argc)
usage_with_options(count_objects_usage, opts);
+ if (verbose)
+ report_pack_garbage = real_report_pack_garbage;
memcpy(path, objdir, len);
if (len && objdir[len-1] != '/')
path[len++] = '/';
diff --git a/sha1_file.c b/sha1_file.c
index 40b2329..cc6ef03 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -21,6 +21,7 @@
#include "sha1-lookup.h"
#include "bulk-checkin.h"
#include "streaming.h"
+#include "dir.h"
#ifndef O_NOATIME
#if defined(__linux__) && (defined(__i386__) || defined(__PPC__))
@@ -1000,6 +1001,54 @@ void install_packed_git(struct packed_git *pack)
packed_git = pack;
}
+/* A hook for count-objects to report invalid files in pack directory */
+void (*report_pack_garbage)(const char *path, int len, const char *name);
+
+static const char *known_pack_extensions[] = { ".pack", ".keep", NULL };
+
+static void report_garbage(struct string_list *list)
+{
+ struct strbuf sb = STRBUF_INIT;
+ struct packed_git *p;
+ int i;
+
+ if (!report_pack_garbage)
+ return;
+
+ sort_string_list(list);
+
+ for (p = packed_git; p; p = p->next) {
+ struct string_list_item *item;
+ if (!p->pack_local)
+ continue;
+ strbuf_reset(&sb);
+ strbuf_add(&sb, p->pack_name,
+ strlen(p->pack_name) - strlen(".pack"));
+ item = string_list_lookup(list, sb.buf);
+ if (!item)
+ continue;
+ /*
+ * string_list_lookup does not guarantee to return the
+ * first matched string if it's duplicated.
+ */
+ while (item - list->items &&
+ !strcmp(item[-1].string, item->string))
+ item--;
+ while (item - list->items < list->nr &&
+ !strcmp(item->string, sb.buf)) {
+ item->util = NULL; /* non-garbage mark */
+ item++;
+ }
+ }
+ for (i = 0; i < list->nr; i++) {
+ struct string_list_item *item = list->items + i;
+ if (!item->util)
+ continue;
+ report_pack_garbage(item->string, 0, item->util);
+ }
+ strbuf_release(&sb);
+}
+
static void prepare_packed_git_one(char *objdir, int local)
{
/* Ensure that this buffer is large enough so that we can
@@ -1009,6 +1058,7 @@ static void prepare_packed_git_one(char *objdir, int local)
int len;
DIR *dir;
struct dirent *de;
+ struct string_list garbage = STRING_LIST_INIT_DUP;
sprintf(path, "%s/pack", objdir);
len = strlen(path);
@@ -1024,14 +1074,37 @@ static void prepare_packed_git_one(char *objdir, int local)
int namelen = strlen(de->d_name);
struct packed_git *p;
- if (!has_extension(de->d_name, ".idx"))
+ if (len + namelen + 1 > sizeof(path)) {
+ if (report_pack_garbage)
+ report_pack_garbage(path, len - 1, de->d_name);
continue;
+ }
+
+ strcpy(path + len, de->d_name);
- if (len + namelen + 1 > sizeof(path))
+ if (!has_extension(de->d_name, ".idx")) {
+ struct string_list_item *item;
+ int i, n;
+ if (!report_pack_garbage)
+ continue;
+ if (is_dot_or_dotdot(de->d_name))
+ continue;
+ for (i = 0; known_pack_extensions[i]; i++)
+ if (has_extension(de->d_name,
+ known_pack_extensions[i]))
+ break;
+ if (!known_pack_extensions[i]) {
+ report_pack_garbage(path, 0, NULL);
+ continue;
+ }
+ n = strlen(path) - strlen(known_pack_extensions[i]);
+ item = string_list_append_nodup(&garbage,
+ xstrndup(path, n));
+ item->util = (void*)known_pack_extensions[i];
continue;
+ }
/* Don't reopen a pack we already have. */
- strcpy(path + len, de->d_name);
for (p = packed_git; p; p = p->next) {
if (!memcmp(path, p->pack_name, len + namelen - 4))
break;
@@ -1047,6 +1120,8 @@ static void prepare_packed_git_one(char *objdir, int local)
install_packed_git(p);
}
closedir(dir);
+ report_garbage(&garbage);
+ string_list_clear(&garbage, 0);
}
static int sort_pack(const void *a_, const void *b_)
--
1.8.1.2.495.g3fdf5d5.dirty
^ permalink raw reply related
* [PATCH v2 3/3] count-objects: report how much disk space taken by garbage files
From: Nguyễn Thái Ngọc Duy @ 2013-02-08 3:48 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Nguyễn Thái Ngọc Duy
In-Reply-To: <1360295307-5469-1-git-send-email-pclouds@gmail.com>
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
We may do some redundant stat() here, but I don't think it can slow
count-objects down much to worry about.
Documentation/git-count-objects.txt | 2 ++
builtin/count-objects.c | 29 ++++++++++++++++++-----------
2 files changed, 20 insertions(+), 11 deletions(-)
diff --git a/Documentation/git-count-objects.txt b/Documentation/git-count-objects.txt
index 1611d7c..da6e72e 100644
--- a/Documentation/git-count-objects.txt
+++ b/Documentation/git-count-objects.txt
@@ -35,6 +35,8 @@ the packs. These objects could be pruned using `git prune-packed`.
+
garbage: the number of files in object database that are not valid
loose objects nor valid packs
++
+size-garbage: disk space consumed by garbage files, in KiB
GIT
---
diff --git a/builtin/count-objects.c b/builtin/count-objects.c
index 118b2ae..90d476d 100644
--- a/builtin/count-objects.c
+++ b/builtin/count-objects.c
@@ -10,24 +10,33 @@
#include "parse-options.h"
static unsigned long garbage;
+static off_t size_garbage;
extern void (*report_pack_garbage)(const char *path, int len, const char *name);
static void real_report_pack_garbage(const char *path, int len, const char *name)
{
+ struct strbuf sb = STRBUF_INIT;
+ struct stat st;
+
if (len && name)
- error("garbage found: %.*s/%s", len, path, name);
+ strbuf_addf(&sb, "%.*s/%s", len, path, name);
else if (!len && name)
- error("garbage found: %s%s", path, name);
+ strbuf_addf(&sb, "%s%s", path, name);
else
- error("garbage found: %s", path);
+ strbuf_addf(&sb, "%s", path);
+ error(_("garbage found: %s"), sb.buf);
+
+ if (!stat(sb.buf, &st))
+ size_garbage += st.st_size;
+
garbage++;
+ strbuf_release(&sb);
}
static void count_objects(DIR *d, char *path, int len, int verbose,
unsigned long *loose,
off_t *loose_size,
- unsigned long *packed_loose,
- unsigned long *garbage)
+ unsigned long *packed_loose)
{
struct dirent *ent;
while ((ent = readdir(d)) != NULL) {
@@ -59,11 +68,8 @@ static void count_objects(DIR *d, char *path, int len, int verbose,
(*loose_size) += xsize_t(on_disk_bytes(st));
}
if (bad) {
- if (verbose) {
- error("garbage found: %.*s/%s",
- len + 2, path, ent->d_name);
- (*garbage)++;
- }
+ if (verbose)
+ report_pack_garbage(path, len + 2, ent->d_name);
continue;
}
(*loose)++;
@@ -113,7 +119,7 @@ int cmd_count_objects(int argc, const char **argv, const char *prefix)
if (!d)
continue;
count_objects(d, path, len, verbose,
- &loose, &loose_size, &packed_loose, &garbage);
+ &loose, &loose_size, &packed_loose);
closedir(d);
}
if (verbose) {
@@ -138,6 +144,7 @@ int cmd_count_objects(int argc, const char **argv, const char *prefix)
printf("size-pack: %lu\n", (unsigned long) (size_pack / 1024));
printf("prune-packable: %lu\n", packed_loose);
printf("garbage: %lu\n", garbage);
+ printf("size-garbage: %lu\n", (unsigned long) (size_garbage / 1024));
}
else
printf("%lu objects, %lu kilobytes\n",
--
1.8.1.2.495.g3fdf5d5.dirty
^ permalink raw reply related
* Re: [PATCH] Use __VA_ARGS__ for all of error's arguments
From: Jeff King @ 2013-02-08 4:24 UTC (permalink / raw)
To: Matt Kraai; +Cc: git, Max Horn, Junio C Hamano, Matt Kraai
In-Reply-To: <1360272632-22566-1-git-send-email-kraai@ftbfs.org>
On Thu, Feb 07, 2013 at 01:30:32PM -0800, Matt Kraai wrote:
> From: Matt Kraai <matt.kraai@amo.abbott.com>
>
> QNX 6.3.2 uses GCC 2.95.3 by default, and GCC 2.95.3 doesn't remove the
> comma if the error macro's variable argument is left out.
>
> Instead of testing for a sufficiently recent version of GCC, make
> __VA_ARGS__ match all of the arguments.
Thanks, this looks better than the original (we do not assume a C99
compiler, so just doing this unconditionally would probably break some
other older systems which do not use gcc).
> /*
> * Let callers be aware of the constant return value; this can help
> - * gcc with -Wuninitialized analysis. We have to restrict this trick to
> - * gcc, though, because of the variadic macro and the magic ## comma pasting
> - * behavior. But since we're only trying to help gcc, anyway, it's OK; other
> - * compilers will fall back to using the function as usual.
> + * gcc with -Wuninitialized analysis.
> */
> #if defined(__GNUC__) && ! defined(__clang__)
> -#define error(fmt, ...) (error((fmt), ##__VA_ARGS__), -1)
> +#define error(...) (error(__VA_ARGS__), -1)
Should you be dropping most of the comment like this? I would expect it
to be more like:
We have to restrict this trick to gcc, though, because we do not
assume all compilers support variadic macros. But since...
Other than that, I think it is OK. The compiler will still catch
"error()" with no arguments and generate the appropriate diagnostic (in
fact, it is better, because the error is now passing too few args to a
function, not to the macro).
-Peff
^ permalink raw reply
* Re: [PATCH 4/4] git-remote-mediawiki: use Git's Makefile to build the script
From: Jeff King @ 2013-02-08 4:28 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Matthieu Moy, git
In-Reply-To: <7vhaln7wkg.fsf@alter.siamese.dyndns.org>
On Thu, Feb 07, 2013 at 11:28:31AM -0800, Junio C Hamano wrote:
> Matthieu Moy <Matthieu.Moy@imag.fr> writes:
>
> > The configuration of the install directory is not reused from the
> > toplevel Makefile: we assume Git is already built, hence just call
> > "git --exec-path". This avoids too much surgery in the toplevel Makefile.
> >
> > git-remote-mediawiki.perl can now "use Git;".
> >
> > Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>
> > ---
>
> Continuing to the comment on 3/4, I wonder if it would be a lot
> simpler and more maintainable if you replaced 1/4 to 3/4 with a
> smaller patch to the top-level Makefile to teach it to munge
> arbitrary path/to/foo.perl to path/to/foo the same way as we do to
> other path/tool.perl that are known to the top-level Makefile
> (similarly, another target to install the resulting path/to/foo at
> an arbitrary place). Then do something like
>
> all::
> $(MAKE) -C ../.. \
> PERL_SCRIPT=contrib/mw-to-git/git-remote-mediawiki.perl \
> build-perl-script
> install::
> $(MAKE) -C ../.. \
> PERL_SCRIPT=contrib/mw-to-git/git-remote-mediawiki.perl \
> install-perl-script
>
> in this step.
That seems much cleaner to me. If done right, it could also let people
put:
CONTRIB_PERL += contrib/mw-to-git/git-remote-mediawiki
or similar into their config.mak, and just get specific contrib bits
built and installed along with the rest of git.
-Peff
^ permalink raw reply
* Re: [PATCH] Use __VA_ARGS__ for all of error's arguments
From: Matt Kraai @ 2013-02-08 4:39 UTC (permalink / raw)
To: Jeff King; +Cc: git, Max Horn, Junio C Hamano, Matt Kraai
In-Reply-To: <20130208042428.GA4157@sigill.intra.peff.net>
On Thu, Feb 07, 2013 at 11:24:28PM -0500, Jeff King wrote:
> Should you be dropping most of the comment like this? I would expect it
> to be more like:
>
> We have to restrict this trick to gcc, though, because we do not
> assume all compilers support variadic macros. But since...
I'll submit a new patch with this change tomorrow.
> Other than that, I think it is OK. The compiler will still catch
> "error()" with no arguments and generate the appropriate diagnostic (in
> fact, it is better, because the error is now passing too few args to a
> function, not to the macro).
Great, thanks for the review.
--
Matt
^ permalink raw reply
* Re: Proposal: branch.<name>.remotepush
From: Jeff King @ 2013-02-08 4:48 UTC (permalink / raw)
To: Ramkumar Ramachandra; +Cc: Git List
In-Reply-To: <CALkWK0nA4hQ0VWivk3AVVVq8Rbb-9CpQ9xFsSOsTQtvo4w08rw@mail.gmail.com>
On Thu, Feb 07, 2013 at 09:44:59PM +0530, Ramkumar Ramachandra wrote:
> This has been annoying me for a really long time, but I never really
> got around to scratching this particular itch. I have a very common
> scenario where I fork a project on GitHub. I have two configured
> remotes: origin which points to "git://upstream" and mine which points
> to "ssh://mine". By default, I always want to pull `master` from
> origin and push to mine.
Same here. Even without GitHub, working on git.git I treat Junio as my
"origin", but push to a publishing point.
> Unfortunately, there's only a branch.<name>.remote which specifies
> which remote to use for both pulling and pushing. There's also a
> remote.<name>.pushurl, but I get the feeling that this exists for an
> entirely different reason: when I have a server with a
> highly-available read-only mirror of the repository at
> git://anongit.*, and a less-available committer-only mirror at
> ssh://*.
Yeah, you don't want to use pushurl. It makes the assumption that you
are pushing to the same remote, so when you, e.g., push to the remote's
refs/heads/master, it will update refs/remotes/origin/master. But that's
not right; that ref should be tracking the true origin, not what you
pushed to.
> How about a branch.<name>.remotepush that specifies a special remote
> for pushing, falling back to branch.<name>.remote?
Sure, though I wonder if you really want a per-branch config, or if you
just want remote.pushDefault or similar, so that you do not have to
configure each branch independently as you create it. I'm imagining
lookup rules something like:
1. If we are on branch $b, check branch.$b.pushRemote.
2. If not set, check remote.pushDefault.
3. If not set, check branch.$b.remote.
4. If not set, check remote.default (there was a proposal for this a
few months ago, but it got stalled).
5. If not set, use "origin".
And then fetching could do the same, with s/push/fetch/. In both cases,
if you are not using the new variables, the behavior is the same as
the current behavior.
-Peff
^ permalink raw reply
* Re: [PATCHv2 4/5] Git.pm: add interface for git credential command
From: Jeff King @ 2013-02-08 5:11 UTC (permalink / raw)
To: Michal Nazarewicz; +Cc: Junio C Hamano, Matthieu Moy, git
In-Reply-To: <78516627e893e54d5aafe0694d1face9a37893de.1360242782.git.mina86@mina86.com>
On Thu, Feb 07, 2013 at 03:01:20PM +0100, Michal Nazarewicz wrote:
> > There are a few disallowed characters, like "\n" in key or value, and
> > "=" in a key. They should never happen unless the caller is buggy, but
> > should we check and catch them here?
>
> I left it as is for now since it's not entairly clear to me what to
> do in all cases. In particular:
>
> - when reading, what to do if the line is " foo = bar ",
According to the spec, whitespace (except for the final newline) is not
significant, and that parses key=" foo ", value=" bar ". The spec could
ignore whitespace on the key side, but I intentionally did not in an
attempt to keep the protocol simple. Your original implementation did
the right thing already.
> - when reading, what to do if the line is "foo=" (ie. empty value),
The empty string is a valid value.
> - when writing, what to do if value is a single space,
Then it's a single space. It's the caller's problem whether that is an
issue or not.
> - when writing, what to do if value ends with a new line,
That's bogus. We cannot represent that value. I'd suggest to simply die,
as it is a bug in the caller (we _could_ try to be nice and assume the
caller accidentally forgot to chomp, but I'd rather be careful than
nice).
> - when writing, what to do if value is empty (currently not printed at all),
I think you should still print it. It's unlikely to matter, but
technically a helper response may override keys (or set them to blank),
and the intermediate state gets sent on to the next helper, if there are
multiple.
> On Thu, Feb 07 2013, Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> wrote:
> > I think you should credit git-remote-mediawiki for the code in the
> > commit message. Perhaps have a first "copy/paste" commit, and then an
> > "adaptation" commit to add sort, ^ anchor in regexp, doc and your
> > callback mechanism, but I won't insist on that.
>
> Good point. Creating additional commit is a bit too much for my
> licking, but added note in commit message.
I think that's fine.
> +sub _credential_read {
> + my %credential;
> + my ($reader, $op) = (@_);
> + while (<$reader>) {
> + if (!/^([^=\s]+)=(.*?)\s*$/) {
> + throw Error::Simple("unable to parse git credential $op response:\n$_");
> + }
> + $credential{$1} = $2;
I think this is worse than your previous version. The spec really is as
simple as:
while (<$reader>) {
last if /^$/; # blank line is OK as end-of-credential
/^([^=]+)=(.*)/
or throw Error::Simple(...);
$credential{$1} = {$2};
}
(actually, the spec as written does not explicitly forbid an empty key,
but it is nonsensical, and it might be worth updating the docs).
> +sub _credential_write {
> + my ($credential, $writer) = @_;
> +
> + for my $key (sort {
> + # url overwrites other fields, so it must come first
> + return -1 if $a eq 'url';
> + return 1 if $b eq 'url';
> + return $a cmp $b;
> + } keys %$credential) {
> + if (defined $credential->{$key} && length $credential->{$key}) {
> + print $writer $key, '=', $credential->{$key}, "\n";
> + }
When I mentioned error-checking the format, I really just meant
something like:
$key =~ /[=\n\0]/
and die "BUG: credential key contains invalid characters: $key";
if (defined $credential->{$key}) {
$credential->{$key} =~ /[\n\0]/
and die "BUG: credential value contains invalid characters: $credential->{key}";
print $writer $key, '=', $credential->{$key}, "\n";
}
Those dies should never happen, and are indicative of a bug in the
caller. We can't even represent them in the protocol, so we might as
well alert the user and die rather than trying to guess what the caller
intended.
-Peff
^ permalink raw reply
* Re: [PATCH v4] Add utf8_fprintf helper which returns correct columns
From: Torsten Bögershausen @ 2013-02-08 6:03 UTC (permalink / raw)
To: Jiang Xin
Cc: Junio C Hamano, Nguyễn Thái Ngọc Duy, Git List,
Nguyễn Thái Ngọc Duy, Torsten Bögershausen
In-Reply-To: <4ea03e99bad13e2910b137fd3991951244fa23f1.1360289411.git.worldhello.net@gmail.com>
On 08.02.13 03:10, Jiang Xin wrote:
> Since command usages can be translated, they may not align well especially
> when they are translated to CJK. A wrapper utf8_fprintf can help to return
> the correct columns required.
>
> Signed-off-by: Jiang Xin <worldhello.net@gmail.com>
> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
> ---
> parse-options.c | 5 +++--
> utf8.c | 22 ++++++++++++++++++++++
> utf8.h | 1 +
> 3 files changed, 26 insertions(+), 2 deletions(-)
>
> diff --git a/parse-options.c b/parse-options.c
> index 67e98..a6ce9e 100644
> --- a/parse-options.c
> +++ b/parse-options.c
> @@ -3,6 +3,7 @@
> #include "cache.h"
> #include "commit.h"
> #include "color.h"
> +#include "utf8.h"
>
> static int parse_options_usage(struct parse_opt_ctx_t *ctx,
> const char * const *usagestr,
> @@ -482,7 +483,7 @@ static int usage_argh(const struct option *opts, FILE *outfile)
> s = literal ? "[%s]" : "[<%s>]";
> else
> s = literal ? " %s" : " <%s>";
> - return fprintf(outfile, s, opts->argh ? _(opts->argh) : _("..."));
> + return utf8_fprintf(outfile, s, opts->argh ? _(opts->argh) : _("..."));
> }
>
> #define USAGE_OPTS_WIDTH 24
> @@ -541,7 +542,7 @@ static int usage_with_options_internal(struct parse_opt_ctx_t *ctx,
> if (opts->long_name)
> pos += fprintf(outfile, "--%s", opts->long_name);
> if (opts->type == OPTION_NUMBER)
> - pos += fprintf(outfile, "-NUM");
> + pos += utf8_fprintf(outfile, _("-NUM"));
>
> if ((opts->flags & PARSE_OPT_LITERAL_ARGHELP) ||
> !(opts->flags & PARSE_OPT_NOARG))
> diff --git a/utf8.c b/utf8.c
> index a4ee6..05925 100644
> --- a/utf8.c
> +++ b/utf8.c
> @@ -430,6 +430,28 @@ int same_encoding(const char *src, const char *dst)
> }
>
> /*
> + * Wrapper for fprintf and returns the total number of columns required
> + * for the printed string, assuming that the string is utf8.
> + */
> +int utf8_fprintf(FILE *stream, const char *format, ...)
> +{
> + struct strbuf buf = STRBUF_INIT;
> + va_list arg;
> + int columns;
> +
> + va_start (arg, format);
> + strbuf_vaddf(&buf, format, arg);
> + va_end (arg);
> +
> + columns = fputs(buf.buf, stream);
> + /* If no error occurs, returns columns really required with utf8_strwidth. */
> + if (0 <= columns)
> + columns = utf8_strwidth(buf.buf);
> + strbuf_release(&buf);
> + return columns;
> +}
> +
I don't think we handle the return code from fputs() correctly.
Please dee below for specifications on fprintf(),
something like the following could do:
int utf8_fprintf(FILE *stream, const char *format, ...)
{
struct strbuf buf = STRBUF_INIT;
va_list arg;
int columns = 0;
va_start (arg, format);
strbuf_vaddf(&buf, format, arg);
va_end (arg);
if (EOF != fputs(buf.buf, stream))
columns = utf8_strwidth(buf.buf);
strbuf_release(&buf);
return columns;
}
And as a side note: would fprintf_strwidth() be a better name?
Linux:
RETURN VALUE
fputc(), putc() and putchar() return the character written as an
unsigned char cast to an int or EOF on error.
puts() and fputs() return a nonnegative number on success, or EOF on
error.
Mac OS:
COMPATIBILITY
fputs() now returns a non-negative number (as opposed to 0) on successful
completion. As a result, many tests (e.g., "fputs() == 0", "fputs() !=
0") do not give the desired result. Use "fputs() != EOF" or "fputs() ==
EOF" to determine success or failure.
Posix:
RETURN VALUE
Upon successful completion, fputs() shall return a non-negative number.
Otherwise, it shall return EOF, set an error indicator for the stream,
and set errno to indicate the error.
^ permalink raw reply
* Re: [RFC] test-lib.sh: No POSIXPERM for cygwin
From: Torsten Bögershausen @ 2013-02-08 6:08 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Ramsay Jones, Torsten Bögershausen, git, j6t
In-Reply-To: <7vd2wb7w96.fsf@alter.siamese.dyndns.org>
On 07.02.13 20:35, Junio C Hamano wrote:
> Ramsay Jones <ramsay@ramsay1.demon.co.uk> writes:
>
>> Torsten Bögershausen wrote:
>>> t0070 and t1301 fail when running the test suite under cygwin.
>>> Skip the failing tests by unsetting POSIXPERM.
>> t1301 does not fail for me. (WIN XP (SP3) on NTFS)
> Others run Cygwin with vfat or some other filesystem, and some of
> them do not cope will with POSIXPERM, perhaps?
>
> Not having POSIXPERM by default for Cygwin may be a saner default
> than having one, if we have to pick one.
>
> It may be debatable to have this default as platform attribute,
> though.
>
Yes, 1301 passes on cygwin 1.5, but not on 1.7.
And it breaks on VFAT, for all kind of OS.
Thanks for comments, I'll put more investigations on my todo stack.
/Torsten
^ permalink raw reply
* Re: Proposal: branch.<name>.remotepush
From: Junio C Hamano @ 2013-02-08 6:08 UTC (permalink / raw)
To: Jeff King; +Cc: Ramkumar Ramachandra, Git List
In-Reply-To: <20130208044836.GC4157@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
> On Thu, Feb 07, 2013 at 09:44:59PM +0530, Ramkumar Ramachandra wrote:
>
>> This has been annoying me for a really long time, but I never really
>> got around to scratching this particular itch. I have a very common
>> scenario where I fork a project on GitHub. I have two configured
>> remotes: origin which points to "git://upstream" and mine which points
>> to "ssh://mine". By default, I always want to pull `master` from
>> origin and push to mine.
>
> Same here. Even without GitHub, working on git.git I treat Junio as my
> "origin", but push to a publishing point.
The "you fetch from and push to the same place" semantics that
associates a branch to a single remote was primarily done for people
coming from CVS/SVN background [*1*]. I think the triangle
arrangement where you want to have "this is where I fetch from and
integrate with, and that is where I publish" is more common among
the Git users these days.
How best to express the triangle is somewhat tricky, but I think it
is sensible to say you have "origin" that points to your upstream
(i.e. me), and "peff" that points to your publishing point, in other
words, make it explicit that the user deals with two remotes. Then
have push.default name the remote "peff", so that "git push" goes to
that remote by default (and have "git fetch/pull" go to "origin).
You will have two sets of remote tracking branches (one from "origin"
that your push will never pretend to have fetched immediately after
finishing, the other from "peff" that keeps track of what you pushed
the last time).
Of course, some people may have "I use this and that branches to
interact with upstream X while I use these other branches to
interacct with upstream Y, and all of them push to different
places", and supporting that may need complex per branch "On this
branch fetch from and integrate with remote X, and push to remote Z"
settings, but as you said, "I fetch from and integrate with X, and
result is pushed out to Y" should be the most common, and it would
be desirable to have a simple way to express it with just a single
new configuration variable.
[Footnote]
*1* It also happens to work reasonably well for people like Linus
and I with the "I pull from random places, I locally integrate and I
publish the results" workflow, because we are trained to think that
it is not just being lazy but simply meaningless to say "git pull"
without saying "fetch and integrate _what_ and from _whom_", and
that is only because we do not have a fixed upstream. Linus and I
would practically never fetch from "origin", i.e. from ourselves.
^ permalink raw reply
* Re: [PATCH v4] Add utf8_fprintf helper which returns correct columns
From: Torsten Bögershausen @ 2013-02-08 6:13 UTC (permalink / raw)
To: Torsten Bögershausen
Cc: Jiang Xin, Junio C Hamano, Nguyễn Thái Ngọc Duy,
Git List, Nguyễn Thái Ngọc Duy
In-Reply-To: <51149542.8060307@web.de>
(Sorry for confusing: I should have written:)
Please see below for specifications on fputs()
Linux:
RETURN VALUE
fputc(), putc() and putchar() return the character written as an unsigned char cast to an int or EOF on error.
puts() and fputs() return a nonnegative number on success, or EOF on error.
Mac OS:
COMPATIBILITY
fputs() now returns a non-negative number (as opposed to 0) on successful completion. As a result, many tests (e.g., "fputs() == 0", "fputs() != 0") do not give the desired result.
Use "fputs() != EOF" or "fputs() == EOF" to determine success or failure.
Posix:
RETURN VALUE
Upon successful completion, fputs() shall return a non-negative number. Otherwise, it shall return EOF, set an error indicator for the stream, and set errno to indicate the error.
^ permalink raw reply
* Re: [PATCHv6] Add contrib/credentials/netrc with GPG support
From: Junio C Hamano @ 2013-02-08 6:15 UTC (permalink / raw)
To: Ted Zlatanov; +Cc: Jeff King, git
In-Reply-To: <87mwvfbmgi.fsf@lifelogs.com>
Ted Zlatanov <tzz@lifelogs.com> writes:
> I agree this Makefile is not a good test to ship out. It was my quickie
> test rig that I should have reworked before adding to the patch. Sorry.
Nothing to be sorry about. Starting with quick-and-dirty and
polishing for public consumption is what the review cycle is about,
and we are here to help that process.
> I see contrib/subtree/t and contrib/mw-to-git/t that I could copy. The
> test will have a few files to parse, and will be able to compare the
> expected to the actual output. Does that sound like a good plan?
Yup.
Thanks.
^ permalink raw reply
* Re: [PATCHv6] Add contrib/credentials/netrc with GPG support
From: Jeff King @ 2013-02-08 6:18 UTC (permalink / raw)
To: Ted Zlatanov; +Cc: Junio C Hamano, git
In-Reply-To: <876226p97h.fsf_-_@lifelogs.com>
On Tue, Feb 05, 2013 at 07:38:58PM -0500, Ted Zlatanov wrote:
> Add Git credential helper that can parse netrc/authinfo files.
>
> This credential helper supports multiple files, returning the first one
> that matches. It checks file permissions and owner. For *.gpg files,
> it will run GPG to decrypt the file.
>
> Signed-off-by: Ted Zlatanov <tzz@lifelogs.com>
> + # the following check is copied from Net::Netrc, for non-GPG files
> + # OS/2 and Win32 do not handle stat in a way compatable with this check :-(
s/compatable/compatible/
You mention os/2 and Win32 here, but the check has more:
> + unless ($gpgmode || $options{insecure} ||
> + $^O eq 'os2'
> + || $^O eq 'MSWin32'
> + || $^O eq 'MacOS'
> + || $^O =~ /^cygwin/) {
Does MacOS really not handle stat? Or is this old MacOS, not OS X?
> +sub load_netrc {
> [...]
> + foreach my $nentry (@netrc_entries) {
> + my %entry;
> + my $num_port;
> +
> + if (!defined $nentry->{machine}) {
> + next;
> + }
> + if (defined $nentry->{port} && $nentry->{port} =~ m/^\d+$/) {
> + $num_port = $nentry->{port};
> + delete $nentry->{port};
> + }
> +
> + # create the new entry for the credential helper protocol
> + $entry{$options{tmap}->{$_}} = $nentry->{$_} foreach keys %$nentry;
> +
> + # for "host X port Y" where Y is an integer (captured by
> + # $num_port above), set the host to "X:Y"
> + if (defined $entry{host} && defined $num_port) {
> + $entry{host} = join(':', $entry{host}, $num_port);
> + }
So this will convert:
machine foo port smtp
in the netrc into (protocol => "smtp", host => "foo"), but:
machine foo port 25
into (protocol => undef, host => "foo:25"), right? That makes sense to
me.
> +sub net_netrc_loader {
> [...]
I won't comment here, as I know very little about netrc (I always
thought it was line-oriented, too!) and Junio has covered it.
> +# takes the search tokens and then a list of entries
> +# each entry is a hash reference
> +sub find_netrc_entry {
> + my $query = shift @_;
> +
> + ENTRY:
> + foreach my $entry (@_)
> + {
> + my $entry_text = join ', ', map { "$_=$entry->{$_}" } keys %$entry;
> + foreach my $check (sort keys %$query) {
> + if (defined $query->{$check}) {
> + log_debug("compare %s [%s] to [%s] (entry: %s)",
> + $check,
> + $entry->{$check},
> + $query->{$check},
> + $entry_text);
> + unless ($query->{$check} eq $entry->{$check}) {
> + next ENTRY;
> + }
> + } else {
> + log_debug("OK: any value satisfies check $check");
> + }
This looks right to me.
> +sub print_credential_data {
I don't know if you want to take the hit of relying on Git.pm (it is
nice for the helper to be totally standalone and copy-able), but one
obvious possible refactor would be to use the credential read/write
functions recently added there. I'm OK with not doing that, though.
> + my $entry = shift @_;
> + my $query = shift @_;
> +
> + log_debug("entry has passed all the search checks");
> + TOKEN:
> + foreach my $git_token (sort keys %$entry) {
> + log_debug("looking for useful token $git_token");
> + # don't print unknown (to the credential helper protocol) tokens
> + next TOKEN unless exists $query->{$git_token};
> +
> + # don't print things asked in the query (the entry matches them)
> + next TOKEN if defined $query->{$git_token};
> +
> + log_debug("FOUND: $git_token=$entry->{$git_token}");
> + printf "%s=%s\n", $git_token, $entry->{$git_token};
> + }
Printf? Bleh, isn't this supposed to be perl? :P
I don't see anything wrong from the credential-handling side of things.
As I said, I didn't look closely at the netrc parsing bits. From my
reading of "perldoc macos", the answer to my question above is "yes,
stat doesn't work on MacOS Classic". So I think the script itself is
fine.
In your tests:
> +++ b/contrib/credential/netrc/Makefile
> @@ -0,0 +1,12 @@
> +test_netrc:
> + @(echo "bad data" | ./git-credential-netrc -f A -d -v) || echo "Bad invocation test, ignoring
> failure"
> + @echo "=> Silent invocation... nothing should show up here with a missing file"
> + @echo "bad data" | ./git-credential-netrc -f A get
> + @echo "=> Back to noisy: -v and -d used below, missing file"
> + echo "bad data" | ./git-credential-netrc -f A -d -v get
> + @echo "=> Look for any entry in the default file set"
> + echo "" | ./git-credential-netrc -d -v get
> + @echo "=> Look for github.com in the default file set"
> + echo "host=google.com" | ./git-credential-netrc -d -v get
> + @echo "=> Look for a nonexistent machine in the default file set"
> + echo "host=korovamilkbar" | ./git-credential-netrc -d -v get
You are depending on whatever the user has in their ~/.netrc, no?
Wouldn't it make more sense to ship a sample netrc and run all of the
tests with "-f netrc.example"?
It may also be worth building on top of the regular git test harness.
It's more work, but the resulting code (and the output) will be much
more readable.
-Peff
^ permalink raw reply
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