* Re: [EGIT PATCH 3/6] Add a method to get refs by object Id
From: Shawn O. Pearce @ 2008-10-06 22:43 UTC (permalink / raw)
To: Robin Rosenberg; +Cc: git
In-Reply-To: <200810070037.53841.robin.rosenberg@dewire.com>
Robin Rosenberg <robin.rosenberg@dewire.com> wrote:
> måndagen den 6 oktober 2008 10.15.54 skrev Shawn O. Pearce:
> > Robin Rosenberg <robin.rosenberg@dewire.com> wrote:
> > >
> > > /**
> > > + * @return a map with all objects referenced by a peeled ref.
> > > + */
> > > + public Map<AnyObjectId, List<Ref>> getAllRefsByPeeledObjectId() {
> >
> > Do we really want to promise List here? Can we make it just
> > Collection instead?
>
> Sure. Our promise is actually slightly better, it is Set, but Java doesn't have a suitable class for that.
java.util.Set? java.util.HashSet?
What am I missing?
> > > + for (Map.Entry<String,Ref> e : allRefs.entrySet()) {
> > > + Ref ref = e.getValue();
> >
> > I think this is cleaner:
> >
> > for (Ref ref : allRefs.values()) {
> >
> > as you never use the key.
>
> Yes. I was thinking it might be less efficient, but the JDK implementation looks quite well optimized in 1.6 at least
> so values() is slightly faster.
I wasn't concerned about performance, I was going for readability.
This loop is probably not performance critical as its done once
early when the PlotWalk starts. I just found that grabbing the
entrySet when all you care about is the values was odd.
> > List<Ref> nl = Collections.singletonList(ref);
> > List<Ref> ol = ret.put(target, nl);
> > if (ol != null) {
> > if (ol.size() == 1) {
> > nl = new ArrayList<Ref>(2);
> > nl.add(ol.get(0));
> > nl.add(ref);
> > ret.put(target, nl);
> > } else {
> > ol.add(ref)
> > ret.put(target, ol);
> > }
> > }
>
> ok, I guess one just has has to include the comment on why for the casual reader.
I'm probably too used to this pattern of "put, then test" because
I use it a lot when the odds of put returning null are very high.
Its an odd idiom. I think most developers wouldn't write it.
So I guess a comment of some sort is probably a good idea if you
chose to use this mess. ;-)
--
Shawn.
^ permalink raw reply
* [PATCH] Replace xmalloc/memset(0) pairs with xcalloc
From: Brandon Casey @ 2008-10-06 23:39 UTC (permalink / raw)
To: Git Mailing List
Many call sites immediately initialize allocated memory with zero after
calling xmalloc. A single call to xcalloc can replace this two-call
sequence.
Signed-off-by: Brandon Casey <casey@nrlssc.navy.mil>
---
builtin-merge.c | 3 +--
builtin-pack-objects.c | 4 +---
builtin-unpack-objects.c | 3 +--
merge-tree.c | 3 +--
remote.c | 3 +--
5 files changed, 5 insertions(+), 11 deletions(-)
diff --git a/builtin-merge.c b/builtin-merge.c
index 5c65a58..dcf8987 100644
--- a/builtin-merge.c
+++ b/builtin-merge.c
@@ -123,8 +123,7 @@ static struct strategy *get_strategy(const char *name)
exit(1);
}
- ret = xmalloc(sizeof(struct strategy));
- memset(ret, 0, sizeof(struct strategy));
+ ret = xcalloc(1, sizeof(struct strategy));
ret->name = xstrdup(name);
return ret;
}
diff --git a/builtin-pack-objects.c b/builtin-pack-objects.c
index 1158e42..59c30d1 100644
--- a/builtin-pack-objects.c
+++ b/builtin-pack-objects.c
@@ -1369,12 +1369,10 @@ static void find_deltas(struct object_entry **list, unsigned *list_size,
int window, int depth, unsigned *processed)
{
uint32_t i, idx = 0, count = 0;
- unsigned int array_size = window * sizeof(struct unpacked);
struct unpacked *array;
unsigned long mem_usage = 0;
- array = xmalloc(array_size);
- memset(array, 0, array_size);
+ array = xcalloc(window, sizeof(struct unpacked));
for (;;) {
struct object_entry *entry = *list++;
diff --git a/builtin-unpack-objects.c b/builtin-unpack-objects.c
index d2796b6..9f4bdd3 100644
--- a/builtin-unpack-objects.c
+++ b/builtin-unpack-objects.c
@@ -477,8 +477,7 @@ static void unpack_all(void)
if (!quiet)
progress = start_progress("Unpacking objects", nr_objects);
- obj_list = xmalloc(nr_objects * sizeof(*obj_list));
- memset(obj_list, 0, nr_objects * sizeof(*obj_list));
+ obj_list = xcalloc(nr_objects, sizeof(*obj_list));
for (i = 0; i < nr_objects; i++) {
unpack_one(i);
display_progress(progress, i + 1);
diff --git a/merge-tree.c b/merge-tree.c
index 02fc10f..2d1413e 100644
--- a/merge-tree.c
+++ b/merge-tree.c
@@ -158,9 +158,8 @@ static int same_entry(struct name_entry *a, struct name_entry *b)
static struct merge_list *create_entry(unsigned stage, unsigned mode, const unsigned char *sha1, const char *path)
{
- struct merge_list *res = xmalloc(sizeof(*res));
+ struct merge_list *res = xcalloc(1, sizeof(*res));
- memset(res, 0, sizeof(*res));
res->stage = stage;
res->path = path;
res->mode = mode;
diff --git a/remote.c b/remote.c
index c45d96e..a2d7ab1 100644
--- a/remote.c
+++ b/remote.c
@@ -751,8 +751,7 @@ int remote_find_tracking(struct remote *remote, struct refspec *refspec)
struct ref *alloc_ref(unsigned namelen)
{
- struct ref *ret = xmalloc(sizeof(struct ref) + namelen);
- memset(ret, 0, sizeof(struct ref) + namelen);
+ struct ref *ret = xcalloc(1, sizeof(struct ref) + namelen);
return ret;
}
--
1.6.0.2.445.g1e1e0
^ permalink raw reply related
* merge -s ffonly
From: Shawn O. Pearce @ 2008-10-06 23:56 UTC (permalink / raw)
To: git; +Cc: Randal L. Schwartz
I really don't care about this feature. But Randal's whining on
#git made me stop what I was doing and write something that might
turn into it.
Totally untested code. It might reformat your C:\ drive and install
Windows ME. Install as $(git --exec-path)/git-merge-ffonly and
call as `git merge -s ffonly`.
If you care about this sort of feature, test it, write tests for it,
make a formal patch, and send it for review. No, I will not do this
for you. As I said, I don't care about this as a feature.
--8<--
diff --git a/git-merge-ffonly.sh b/git-merge-ffonly.sh
new file mode 100644
index 0000000..24363b5
--- /dev/null
+++ b/git-merge-ffonly.sh
@@ -0,0 +1,20 @@
+#!/bin/sh
+while test $# -gt 0
+do
+ if test "z$1" = z--
+ then
+ shift
+ break
+ else
+ shift
+ fi
+done
+
+while test $# -gt 0
+do
+ if test -n "$(git rev-list $1..HEAD)"
+ then
+ exit 2
+ fi
+ shift
+done
--
Shawn.
^ permalink raw reply related
* [PATCH 1/2 v2] check-attr: add an internal check_attr() function
From: Dmitry Potapov @ 2008-10-07 0:14 UTC (permalink / raw)
To: Johannes Sixt; +Cc: Alexander Gavrilov, git, Shawn O. Pearce, Paul Mackerras
In-Reply-To: <48E9B997.1010006@viscovery.net>
This step is preparation to introducing --stdin-paths option.
I have also added maybe_flush_or_die() at the end of main() to ensure that
we exit with the zero code only when we flushed the output successfully.
Signed-off-by: Dmitry Potapov <dpotapov@gmail.com>
---
maybe_flush_or_die() is added in this version of patch.
builtin-check-attr.c | 42 ++++++++++++++++++++++++------------------
1 files changed, 24 insertions(+), 18 deletions(-)
diff --git a/builtin-check-attr.c b/builtin-check-attr.c
index cb783fc..786256e 100644
--- a/builtin-check-attr.c
+++ b/builtin-check-attr.c
@@ -6,6 +6,27 @@
static const char check_attr_usage[] =
"git check-attr attr... [--] pathname...";
+static void check_attr(int cnt, struct git_attr_check *check,
+ const char** name, const char *file)
+{
+ int j;
+ if (git_checkattr(file, cnt, check))
+ die("git_checkattr died");
+ for (j = 0; j < cnt; j++) {
+ const char *value = check[j].value;
+
+ if (ATTR_TRUE(value))
+ value = "set";
+ else if (ATTR_FALSE(value))
+ value = "unset";
+ else if (ATTR_UNSET(value))
+ value = "unspecified";
+
+ quote_c_style(file, NULL, stdout, 0);
+ printf(": %s: %s\n", name[j], value);
+ }
+}
+
int cmd_check_attr(int argc, const char **argv, const char *prefix)
{
struct git_attr_check *check;
@@ -42,23 +63,8 @@ int cmd_check_attr(int argc, const char **argv, const char *prefix)
check[i].attr = a;
}
- for (i = doubledash; i < argc; i++) {
- int j;
- if (git_checkattr(argv[i], cnt, check))
- die("git_checkattr died");
- for (j = 0; j < cnt; j++) {
- const char *value = check[j].value;
-
- if (ATTR_TRUE(value))
- value = "set";
- else if (ATTR_FALSE(value))
- value = "unset";
- else if (ATTR_UNSET(value))
- value = "unspecified";
-
- quote_c_style(argv[i], NULL, stdout, 0);
- printf(": %s: %s\n", argv[j+1], value);
- }
- }
+ for (i = doubledash; i < argc; i++)
+ check_attr(cnt, check, argv+1, argv[i]);
+ maybe_flush_or_die(stdout, "attribute to stdout");
return 0;
}
--
1.6.0.2.447.g3befd
^ permalink raw reply related
* [PATCH 2/2 v2] check-attr: Add --stdin-paths option
From: Dmitry Potapov @ 2008-10-07 0:16 UTC (permalink / raw)
To: Johannes Sixt; +Cc: Alexander Gavrilov, git, Shawn O. Pearce, Paul Mackerras
In-Reply-To: <48E9B997.1010006@viscovery.net>
This allows multiple paths to be specified on stdin.
Signed-off-by: Dmitry Potapov <dpotapov@gmail.com>
---
On Mon, Oct 06, 2008 at 09:09:11AM +0200, Johannes Sixt wrote:
>
> The least that is needed is fflush(stdout) in this loop (after each
> iteration!)
Thanks. Somehow, I forgot about it though it is quite obvious.
I have added maybe_flush_or_die().
Documentation/git-check-attr.txt | 4 ++
builtin-check-attr.c | 70 ++++++++++++++++++++++++++++++++------
t/t0003-attributes.sh | 17 +++++++++
3 files changed, 80 insertions(+), 11 deletions(-)
diff --git a/Documentation/git-check-attr.txt b/Documentation/git-check-attr.txt
index 2b821f2..0839a57 100644
--- a/Documentation/git-check-attr.txt
+++ b/Documentation/git-check-attr.txt
@@ -9,6 +9,7 @@ git-check-attr - Display gitattributes information.
SYNOPSIS
--------
'git check-attr' attr... [--] pathname...
+'git check-attr' --stdin-paths attr... < <list-of-paths
DESCRIPTION
-----------
@@ -17,6 +18,9 @@ For every pathname, this command will list if each attr is 'unspecified',
OPTIONS
-------
+--stdin-paths::
+ Read file names from stdin instead of from the command-line.
+
\--::
Interpret all preceding arguments as attributes, and all following
arguments as path names. If not supplied, only the first argument will
diff --git a/builtin-check-attr.c b/builtin-check-attr.c
index 786256e..fa1e4d5 100644
--- a/builtin-check-attr.c
+++ b/builtin-check-attr.c
@@ -2,9 +2,19 @@
#include "cache.h"
#include "attr.h"
#include "quote.h"
+#include "parse-options.h"
-static const char check_attr_usage[] =
-"git check-attr attr... [--] pathname...";
+static int stdin_paths;
+static const char * const check_attr_usage[] = {
+"git check-attr attr... [--] pathname...",
+"git check-attr --stdin-paths attr... < <list-of-paths>",
+NULL
+};
+
+static const struct option check_attr_options[] = {
+ OPT_BOOLEAN(0 , "stdin-paths", &stdin_paths, "read file names from stdin"),
+ OPT_END()
+};
static void check_attr(int cnt, struct git_attr_check *check,
const char** name, const char *file)
@@ -27,17 +37,44 @@ static void check_attr(int cnt, struct git_attr_check *check,
}
}
+static void check_attr_stdin_paths(int cnt, struct git_attr_check *check,
+ const char** name)
+{
+ struct strbuf buf, nbuf;
+
+ strbuf_init(&buf, 0);
+ strbuf_init(&nbuf, 0);
+ while (strbuf_getline(&buf, stdin, '\n') != EOF) {
+ if (buf.buf[0] == '"') {
+ strbuf_reset(&nbuf);
+ if (unquote_c_style(&nbuf, buf.buf, NULL))
+ die("line is badly quoted");
+ strbuf_swap(&buf, &nbuf);
+ }
+ check_attr(cnt, check, name, buf.buf);
+ maybe_flush_or_die(stdout, "attribute to stdout");
+ }
+ strbuf_release(&buf);
+ strbuf_release(&nbuf);
+}
+
int cmd_check_attr(int argc, const char **argv, const char *prefix)
{
struct git_attr_check *check;
int cnt, i, doubledash;
+ const char *errstr = NULL;
+
+ argc = parse_options(argc, argv, check_attr_options, check_attr_usage,
+ PARSE_OPT_KEEP_DASHDASH);
+ if (!argc)
+ usage_with_options(check_attr_usage, check_attr_options);
if (read_cache() < 0) {
die("invalid cache");
}
doubledash = -1;
- for (i = 1; doubledash < 0 && i < argc; i++) {
+ for (i = 0; doubledash < 0 && i < argc; i++) {
if (!strcmp(argv[i], "--"))
doubledash = i;
}
@@ -45,26 +82,37 @@ int cmd_check_attr(int argc, const char **argv, const char *prefix)
/* If there is no double dash, we handle only one attribute */
if (doubledash < 0) {
cnt = 1;
- doubledash = 1;
+ doubledash = 0;
} else
- cnt = doubledash - 1;
+ cnt = doubledash;
doubledash++;
- if (cnt <= 0 || argc < doubledash)
- usage(check_attr_usage);
+ if (cnt <= 0)
+ errstr = "No attribute specified";
+ else if (stdin_paths && doubledash < argc)
+ errstr = "Can't specify files with --stdin-paths";
+ if (errstr) {
+ error (errstr);
+ usage_with_options(check_attr_usage, check_attr_options);
+ }
+
check = xcalloc(cnt, sizeof(*check));
for (i = 0; i < cnt; i++) {
const char *name;
struct git_attr *a;
- name = argv[i + 1];
+ name = argv[i];
a = git_attr(name, strlen(name));
if (!a)
return error("%s: not a valid attribute name", name);
check[i].attr = a;
}
- for (i = doubledash; i < argc; i++)
- check_attr(cnt, check, argv+1, argv[i]);
- maybe_flush_or_die(stdout, "attribute to stdout");
+ if (stdin_paths)
+ check_attr_stdin_paths(cnt, check, argv);
+ else {
+ for (i = doubledash; i < argc; i++)
+ check_attr(cnt, check, argv, argv[i]);
+ maybe_flush_or_die(stdout, "attribute to stdout");
+ }
return 0;
}
diff --git a/t/t0003-attributes.sh b/t/t0003-attributes.sh
index 3d8e06a..f6901b4 100755
--- a/t/t0003-attributes.sh
+++ b/t/t0003-attributes.sh
@@ -47,6 +47,23 @@ test_expect_success 'attribute test' '
'
+test_expect_success 'attribute test: read paths from stdin' '
+
+ cat <<EOF > expect
+f: test: f
+a/f: test: f
+a/c/f: test: f
+a/g: test: a/g
+a/b/g: test: a/b/g
+b/g: test: unspecified
+a/b/h: test: a/b/h
+a/b/d/g: test: a/b/d/*
+EOF
+
+ sed -e "s/:.*//" < expect | git check-attr --stdin-paths test > actual &&
+ test_cmp expect actual
+'
+
test_expect_success 'root subdir attribute test' '
attr_check a/i a/i &&
--
1.6.0.2.447.g3befd
^ permalink raw reply related
* Re: [PATCH] error out if path is invalid
From: Dmitry Potapov @ 2008-10-07 0:22 UTC (permalink / raw)
To: Johannes Sixt; +Cc: Shawn O. Pearce, git
In-Reply-To: <48E9B7FE.2000503@viscovery.net>
On Mon, Oct 06, 2008 at 09:02:22AM +0200, Johannes Sixt wrote:
> Dmitry Potapov schrieb:
> > if (!verify_path(path))
> > - return -1;
> > + return error("Invalid path '%s'", path);
>
> Look at this change. Didn't the code error out before, too?
It is certainly did not here. As to its caller, it depends. In fact,
there are two chunks like that in my patch, so I am not sure to which
one you refer here. If we speak about add_cacheinfo() then though the
function did not error out, its caller died with one of the following
messages:
git update-index: unable to update some-file-name
or
git update-index: --cacheinfo cannot add some-file-name
However, if we speak about add_index_entry_with_check then the caller
will not produce any error. The git would exit successfully (it still
does) and there was no error message as if everything was fine.
Perhaps, the exit code should be corrected too, but if the git just dies
when add_index_entry() fails it may cause that having one invalid path
will prevent to check out other files, which does not seem to be the
right thing to do.
As to correction to correction to make_cache_entry then after my
previous patch, it started to error out:
make_cache_entry failed for path 'some-file-name'
before that it silently segfaulted.
> Same in the
> other cases. Hence, your patch subject does not describe the patch.
Should I include the above explanation in the commit message or do you
have any objection to having the above error message in cases where the
caller already produce some message when it dies?
Dmitry
^ permalink raw reply
* error pushing stash ?
From: David Bryson @ 2008-10-07 0:34 UTC (permalink / raw)
To: git
[-- Attachment #1: Type: text/plain, Size: 1051 bytes --]
I have a git mirror remote setup on a few of my repositories:
[remote "backup"]
url = /users/dbryson/backup/janus.git/
fetch = +refs/heads/*:refs/remotes/origin/*
receivepack = sudo -u dbryson git-receive-pack
mirror = 1
I send my refs to the backup with:
$ git push backup
Only to find some odd error messages:
Counting objects: 133, done.
Compressing objects: 100% (109/109), done.
Writing objects: 100% (109/109), 31.25 KiB, done.
Total 109 (delta 82), reused 0 (delta 0)
error: refusing to create funny ref 'refs/stash' remotely
To /users/dbryson/backup/janus.git/
549f8a4..8e93d51 8654 -> 8654
ef6195b..549f8a4 origin/8654 -> origin/8654
+ 623e7cb...63d7262 origin/master -> origin/master (forced update)
! [remote rejected] refs/stash -> refs/stash (funny refname)
error: failed to push some refs to '/users/dbryson/backup/janus.git/'
Should I be concnerned about this or is it normal ? To be honest the
fact that the stash isn't pushing doesn't bother me. But maybe it is a
symptom of a larger problem ?
Dave
[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]
^ permalink raw reply
* Re: error pushing stash ?
From: Shawn O. Pearce @ 2008-10-07 0:40 UTC (permalink / raw)
To: David Bryson; +Cc: git
In-Reply-To: <20081007003435.GZ5774@eratosthenes.cryptobackpack.org>
David Bryson <david@statichacks.org> wrote:
> [remote "backup"]
...
> mirror = 1
>
> Only to find some odd error messages:
>
> $ git push backup
> Counting objects: 133, done.
> Compressing objects: 100% (109/109), done.
> Writing objects: 100% (109/109), 31.25 KiB, done.
> Total 109 (delta 82), reused 0 (delta 0)
> error: refusing to create funny ref 'refs/stash' remotely
> To /users/dbryson/backup/janus.git/
> 549f8a4..8e93d51 8654 -> 8654
> ef6195b..549f8a4 origin/8654 -> origin/8654
> + 623e7cb...63d7262 origin/master -> origin/master (forced update)
> ! [remote rejected] refs/stash -> refs/stash (funny refname)
> error: failed to push some refs to '/users/dbryson/backup/janus.git/'
refs/stash is a funny refname because it contains only 1 '/'.
Normally a valid ref has at least 2 '/', e.g. refs/heads/8654 or
refs/tags/v1.0.
Naming the stash refs/stash was perhaps funny in the first place
since it cannot be moved about on the transport protocol, but then
again the bulk of the stash data is actually in the reflog for the
stash (and not the stash ref itself) so there is basically no point
in pushing or fetching a stash directly.
--
Shawn.
^ permalink raw reply
* Re: Files with colons under Cygwin
From: Dmitry Potapov @ 2008-10-07 0:53 UTC (permalink / raw)
To: Johannes Sixt; +Cc: Giovanni Funchal, git, Shawn O. Pearce
In-Reply-To: <48E9B634.6040909@viscovery.net>
On Mon, Oct 06, 2008 at 08:54:44AM +0200, Johannes Sixt wrote:
>
> IIUC, verify_path() checks paths that were found in the database or the
> index.
It also checks paths that was given to git add and other commands to
prevent an invalid path to enter to the index. If I am not mistaken,
if invalid path has entered in the index then it will be committed to
the database without any further checks.
> As such, it checks for the integrity of the database. And paths
> with backslashes or colons certainly do not violate the database integrity.
No, it has nothing to do with the database. You can run git fsck --full
on a repository that contains '..' or '.' or '.git', and there will be
no error. Having those names does not violate the database integrity,
as the database is concerned all names are just bytes separated by '/',
so having name '.' is not a problem for it. However, names '.' and '..'
have special meaning for the filesystem, and paths starting with .git/
have special meaning for Git repository. If you work in a bare repo
then those names are not a problem, but once you have a working tree,
you want make sure that there is nothing wrong with it.
>
> More precisely, the exchange of path names between the index and tree
> objects (both directions) should not do this new check, nor if a path is
> added to the index. The check is only meaningful[*] when a path is read
> from the index or a tree object and "applied" to the working directory.
> Unfortunately, I think there are lots of places where this happens.
>
> [*] I say "meaningful" and not "necessary" because the situation is just
> like when you grab some random SoftwarePackage.tar.gz, and run ./configure
> without looking first what it is going to do.
When I grab any tar, I can look at its context without myself of any
risk that some files can be overwritten on my file system. And when
I want to look at some remote git repository, I usually do:
git clone URL
If it can overwrite some files behind my back, it is security a hole.
BTW, it was the reason why the idea of allowing .gitconfig to be stored
in git repository (similar to .gitignore) was stroke down about a year ago
though it could help some clueless users...
On Linux (or other sane file systems), we have all required checks to
prevent that from happening, and they are places in verify_path, which
prevents malicious names entering into the index and thus to the file
system too. So, we should do all required checks on Windows too.
Now, I've realized that my checks were not sufficient (due to Windows
being case-insensitive), so I will add more checks and resend this
patch later.
Dmitry
^ permalink raw reply
* Re: [PATCH 0/4] diff text conversion filter
From: Jeff King @ 2008-10-07 1:20 UTC (permalink / raw)
To: Matthieu Moy; +Cc: Johannes Sixt, git
In-Reply-To: <vpqtzbpwy9h.fsf@bauges.imag.fr>
On Mon, Oct 06, 2008 at 05:15:22PM +0200, Matthieu Moy wrote:
> Actually, I understand you don't want git gui and gitk to load MS-Word
> anytime you click something, but I'd love to see the textconv+diff in
> gitk.
>
> (yeah, that's pretty hard to specify right, the ideal requirement
> seems to be "in a gui, use the good part of the diff driver, but not
> the other" :-\).
I think it is even more complex than that. Sometimes when doing "git
show" I want to see the textconv'd version, and sometimes I don't. So I
really want a command-line flag or environment variable that I can use
to control it (with a sane default).
So probably the way forward is to stop relying on "oh, we just didn't
parse some of the config, so it won't get used" to just parsing the
config all the time and having a diffopt for "do textconv, do external
diff, etc". And then plumbing can make sure that those flags never get
set, and porcelain can tie them to command-line options.
-Peff
^ permalink raw reply
* Re: [msysGit] [FYI][PATCH] Customizing the WinGit installer
From: Dmitry Potapov @ 2008-10-07 1:53 UTC (permalink / raw)
To: Linus Torvalds
Cc: Johannes Schindelin, Jakub Narebski, Petr Baudis, msysgit, git
In-Reply-To: <alpine.LFD.2.00.0810061246460.3208@nehalem.linux-foundation.org>
On Mon, Oct 06, 2008 at 12:47:16PM -0700, Linus Torvalds wrote:
>
> On Mon, 6 Oct 2008, Johannes Schindelin wrote:
> >
> > It is not a bug.
>
> If it's unwanted functionality, that's a bug. That's kind of the whole
> (and only) difference between "bug" and "feature". Do people want it?
It looks like you have changed your opinion since what you wrote half
a year ago.
Back then you said:
> Yeah. Why not just rather than the whole ok/cancel discussion, go with a
> single button saying "good for me!" and be done with it.
>
> IOW, the license thing should be considered *informational* rather than a
> choice. Because to a user, that's exactly what the GPL is.
http://article.gmane.org/gmane.comp.version-control.msysgit/1677
Also, please, re-read what Junio said in this thread:
"I think removing the license dialog is a bad idea. You need to
tell the end-user about his rights, and one of the things is
that he can get source to git under the terms of GPLv2. The bug
is not about showing the license, but is about refusing to
install unless the end user agrees with it."
I think such a step as removing the license should be approved by
Junio and not be taken behind his back as it's happening now.
Dmitry
^ permalink raw reply
* Re: [msysGit] [FYI][PATCH] Customizing the WinGit installer
From: Dmitry Potapov @ 2008-10-07 1:59 UTC (permalink / raw)
To: Heikki Orsila
Cc: Johannes Schindelin, Linus Torvalds, Jakub Narebski, Petr Baudis,
msysgit, git
In-Reply-To: <20081006201125.GI16289@zakalwe.fi>
On Mon, Oct 06, 2008 at 11:11:25PM +0300, Heikki Orsila wrote:
>
> I disagree VIOLENTLY with you. I've been utterly struck with this
> Windows crap here.. I spent a day installing stupid trivial software
> and answering pointless EULAs. I REALLY REALLY hate extra questions..
Git installer should NOT ask you whether you agree with the license,
but it merely shows you the license. (At least, it is what was decided
half a year ago).
> This is my 20th reboot..
I am pretty sure that 20th reboot has nothing to do with how many times
the license has been shown...
Dmitry
^ permalink raw reply
* RFH: gitk equivalent of "git log -p --full-diff file.cpp"
From: Eric Raible @ 2008-10-07 2:04 UTC (permalink / raw)
To: Git Mailing List; +Cc: paulus
Hello All -
Hopefully I'm just being dense, but there's a useful tortoisesvn feature that
(so far) I haven't found in gitk. Blasphemy! Sniveling windoze luser, etc^2.
Anyway what I'm looking for is the gitk equivalent of:
git log -p --full-diff file.cpp
That is to say: I want to see all commits which touch a given
file along with all of the other files changed by that commit.
gitk -p --full-diff EntryPoints.cpp
doesn't quite do it - it treats the "extra" diffs as part of the commit msg.
Anyone got more if a clue than me?
Thanks - Eric
PS - Aside from aliases, am I missing a simpler spelling of the above
git log command?
^ permalink raw reply
* Re: Files with colons under Cygwin
From: Joshua Juran @ 2008-10-07 2:05 UTC (permalink / raw)
To: Dmitry Potapov; +Cc: Giovanni Funchal, git, Shawn O. Pearce
In-Reply-To: <20081004233945.GM21650@dpotapov.dyndns.org>
On Oct 4, 2008, at 4:39 PM, Dmitry Potapov wrote:
> On Thu, Oct 02, 2008 at 04:02:23PM +0200, Giovanni Funchal wrote:
>>
>> Cygwin does not allow files with colons, I think this is Windows
>> stuff
>> one just can't avoid.
>
> I wonder if the problem exists on Mac OS X too. From what I heard, it
> does not treat ':' as a normal symbol.
The short answer is that Mac OS X's POSIX implementation works as
expected (internally replacing ':' with '/') without issue.
Furthermore, my POSIX-like environment Lamp (Lamp ain't Mac POSIX)
that runs on classic Mac OS (much in the manner of Cygwin) provides
the same Unix filing behavior, so Mac filing syntax isn't an issue in
running git on Mac OS 9. (I'm sure everybody's breathing a HUGE sigh
of relief at this news...)
Josh
^ permalink raw reply
* Re: [msysGit] [FYI][PATCH] Customizing the WinGit installer
From: Linus Torvalds @ 2008-10-07 2:10 UTC (permalink / raw)
To: Dmitry Potapov
Cc: Johannes Schindelin, Jakub Narebski, Petr Baudis, msysgit, git
In-Reply-To: <20081007015336.GU21650@dpotapov.dyndns.org>
On Tue, 7 Oct 2008, Dmitry Potapov wrote:
>
> It looks like you have changed your opinion since what you wrote half
> a year ago.
Back then, nobody had really complained and sent in a patch to make it
optional.
That changes things. Once some user actually complains, and sends in a
fix to make the whole dialog optional, I don't see why anybody would ever
argue against such a patch being accepted.
Linus
^ permalink raw reply
* Re: [PATCH RFC] rebase--interactive: if preserving merges, use first-parent to limit what is shown.
From: Stephen Haberman @ 2008-10-07 2:20 UTC (permalink / raw)
To: Avi Kivity; +Cc: git
In-Reply-To: <20081006102118.3e817a0f.stephen@exigencecorp.com>
> I agree with Avi on what the rebase -i -p behavior should be for his
> scenario. This patch makes it so. However, the bane of my existence,
> t3404 is failing ~12 tests in, which is a real PITA to debug, so
> please let me know if this is a worthwhile tangent to continue on.
Ah, good old t3404--it caught me on an aspect I had considered but
wanted to avoid--Avi's (and my) preferred "--first-parent" way of
listing merges works great if the right hand side of the merge commits
are outside of the branch getting rebased.
E.g. my use case is when I merge in a stable release with ~100 commits
or so and could potentially want to move it around, perhaps squash
around it as Avi pointed it, I don't want all 100 commits that are in
the stable branch to be listed in my todo file.
However, t3404 makes a good point that if the right hand of the merge
has parents that are going to get rebased, the right hand side does
need to be included/shown/rewritten.
I also went and looked at the git-sequencer rewrite of rebase-i and it
looks slick. I don't fully understand it yet, but I'm much more
inclined now to just let Stephan (& sponors/list) ably handle the
problem. Especially since its moving to builtin, it moves the required
technical ability to contribute above my current skillset--perhaps that
is the intent. :-)
So, unless I think of something else, I'm done hacking on this and
am withdrawing the patch.
Though I am curious--with the sequencer, is the Avi/my request of not
listing out-of-band commits in the todo file going to be handled?
Some sort of "--first-parent-unless-included-in-rebase" flag.
Thanks,
Stephen
^ permalink raw reply
* Re: RFH: gitk equivalent of "git log -p --full-diff file.cpp"
From: Jeff King @ 2008-10-07 2:27 UTC (permalink / raw)
To: Eric Raible; +Cc: Git Mailing List, paulus
In-Reply-To: <279b37b20810061904n4f50f650l81590cb6fd239282@mail.gmail.com>
On Mon, Oct 06, 2008 at 07:04:14PM -0700, Eric Raible wrote:
> Anyway what I'm looking for is the gitk equivalent of:
>
> git log -p --full-diff file.cpp
Try turning off the option "Limit diffs to listed paths" in the
preferences menu (or adding "set limitdiffs 0" to your ~/.gitk).
> That is to say: I want to see all commits which touch a given
> file along with all of the other files changed by that commit.
>
> gitk -p --full-diff EntryPoints.cpp
You definitely don't want the "-p" here, which is what is causing the
extra diffs as part of the commit message. What happens is that gitk
actually gets the diff separately from the history (since it gets each
diff on the fly as you highlight it). The command line arguments go
towards getting the history; so gitk isn't expecting parse any diff (and
you will make gitk startup a _lot_ slower as it waits for git to
generate the diffs for every revision).
The "--full-diff" is simply ignored, since the actual diff happens
during a later command. In theory, gitk could remember your --full-diff
and use it later. But I think for your use, the preference setting
is probably good enough.
> PS - Aside from aliases, am I missing a simpler spelling of the above
> git log command?
Nope, it looks right to me. At one point, we discussed a "full-diff"
config option (and I think I maybe even posted a patch), but nobody
really cared.
-Peff
^ permalink raw reply
* Re: [msysGit] [FYI][PATCH] Customizing the WinGit installer
From: Avery Pennarun @ 2008-10-07 2:40 UTC (permalink / raw)
To: Dmitry Potapov
Cc: Heikki Orsila, Johannes Schindelin, Linus Torvalds,
Jakub Narebski, Petr Baudis, msysgit, git
In-Reply-To: <20081007015942.GV21650@dpotapov.dyndns.org>
On Mon, Oct 6, 2008 at 9:59 PM, Dmitry Potapov <dpotapov@gmail.com> wrote:
> On Mon, Oct 06, 2008 at 11:11:25PM +0300, Heikki Orsila wrote:
>>
>> I disagree VIOLENTLY with you. I've been utterly struck with this
>> Windows crap here.. I spent a day installing stupid trivial software
>> and answering pointless EULAs. I REALLY REALLY hate extra questions..
>
> Git installer should NOT ask you whether you agree with the license,
> but it merely shows you the license. (At least, it is what was decided
> half a year ago).
FWIW, the only reason *installers* ever show licenses is because they
want to give you *additional* restrictions on top of normal copyright,
and making you "click through" the license before using the
application might (or might not) make those restrictions more legally
enforceable than simply including that license along with the program.
Installers don't show you the license to increase visibility; they do
it for legal reasons.
The most visible place to put a license is in either a splash screen
or a Help|About dialog. If I'm using a program and I want to know the
license, that's where I look.
This whole discussion is silly because end users, who are the ones we
claim to be worried about, will not be the ones running the installer
anyway at a large company. It'll be the IT department guy, who
already knows the license and will have to run the installer 500
times. And if it's not a large company, individuals will see the
license when they go to download the software.
Have fun,
Avery
^ permalink raw reply
* Re: [msysGit] [FYI][PATCH] Customizing the WinGit installer
From: Dmitry Potapov @ 2008-10-07 3:08 UTC (permalink / raw)
To: Linus Torvalds
Cc: Johannes Schindelin, Jakub Narebski, Petr Baudis, msysgit, git
In-Reply-To: <alpine.LFD.2.00.0810061909260.3208@nehalem.linux-foundation.org>
On Mon, Oct 06, 2008 at 07:10:37PM -0700, Linus Torvalds wrote:
>
> That changes things. Once some user actually complains, and sends in a
> fix to make the whole dialog optional, I don't see why anybody would ever
> argue against such a patch being accepted.
First, he did not complain. He did not even mention that in the commit
message. He mentioning some other things like removing release notes,
but not the license. Second, I would expect that any change that goes
against the previous achieved agreement may deserve some discussion,
and not blindly accepted just because the user sent a patch. Okay, I
don't really care whether the installer shows the license or not...
Perhaps, something like Help|License would be a better place for it.
Dmitry
^ permalink raw reply
* Re: GitTogether topics status (4th of October)
From: Jeff King @ 2008-10-07 3:15 UTC (permalink / raw)
To: Christian Couder
Cc: git, Shawn O. Pearce, Junio C Hamano, Scott Chacon, Sam Vilain,
Petr Baudis
In-Reply-To: <200810041816.41026.chriscool@tuxfamily.org>
On Sat, Oct 04, 2008 at 06:16:40PM +0200, Christian Couder wrote:
> As can be seen on the GitTogether page on the wiki:
>
> http://git.or.cz/gitwiki/GitTogether
>
> the planned speakers/topics changed a lot during the last weeks and are now:
I just added two proposed half-hour meetings, both of which I intend to
be a few minutes of me talking followed by group discussion. The topics
are:
1. Helping new developers join the git community
This title is a little bit sneaky. I want to talk about not just how
we can get new developers to help improve git, but also how we can
convince them to adopt workflows that make less work for reviewers
and maintainers. It seems like there are some things that we tell
new people over and over about formatting code, formatting patches,
sending patches, etc. Probably the end goal will be improvements to
SubmittingPatches, but maybe putting similar content somewhere more
visible, or maybe even adjusting our workflows a bit.
I hope to make it useful for veterans (who may _constructively_
complain about the habits of new developers), and new developers,
who may learn something about contributing.
This might overlap a bit with Dscho's "contributing with git" talk
(which I am not sure is a talk about using git to contribute in
general, or using git to contribute to git), but I think the
discussion-like forum will make it different enough to be valuable.
2. What needs refactoring?
I occasionally run up against parts of the code that just make my
eyes bleed everytime I touch them. I think we've made significant
progress in maintanability and bug-avoidance with things like the
strbuf library, refactoring of remote and transport handling, etc.
What areas might still benefit from such refactoring?
-Peff
^ permalink raw reply
* [PATCH v2] correct verify_path for Windows
From: Dmitry Potapov @ 2008-10-07 3:26 UTC (permalink / raw)
To: Joshua Juran; +Cc: Giovanni Funchal, git, Shawn O. Pearce, Johannes Sixt
In-Reply-To: <B985AE98-F6E2-4C23-8D34-5A22A9F89FA7@gmail.com>
Colon and backslash in names may be used on Windows to overwrite files
outside of the working directory. Due to the file-system being case-
insensitive, .git can be written as any combination of upper and lower
characters, so we should check that too.
Signed-off-by: Dmitry Potapov <dpotapov@gmail.com>
---
In this version, I have added the check that files in .git/ will not
be overwritten by checkout. Overwriting such files as .git/config is
potentially exploitable.
Josh,
Does OS X need the same check below? I believe it has case-insensitive
filesystem, so it needs that too, but I am not sure what is the right
define should be used.
Thanks,
Dmitry
read-cache.c | 19 +++++++++++++++++++
1 files changed, 19 insertions(+), 0 deletions(-)
diff --git a/read-cache.c b/read-cache.c
index aff6390..7f855ee 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -668,10 +668,19 @@ static int verify_dotfile(const char *rest)
* shares the path end test with the ".." case.
*/
case 'g':
+#if defined(_WIN32) || defined(__CYGWIN__)
+ /* On Windows, file names are case-insensitive */
+ case 'G':
+ if ((rest[1]|0x20) != 'i')
+ break;
+ if ((rest[2]|0x20) != 't')
+ break;
+#else
if (rest[1] != 'i')
break;
if (rest[2] != 't')
break;
+#endif
rest += 2;
/* fallthrough */
case '.':
@@ -703,6 +712,16 @@ inside:
}
return 0;
}
+#if defined(_WIN32) || defined(__CYGWIN__)
+ /*
+ * There is a bunch of other characters that are not allowed
+ * in Win32 API, but the following two create a security hole
+ * by allowing to overwrite files outside of the working tree,
+ * therefore they are explicitly prohibited.
+ */
+ else if (c == ':' || c == '\\')
+ return 0;
+#endif
c = *path++;
}
}
--
1.6.0
^ permalink raw reply related
* Re: RFH: gitk equivalent of "git log -p --full-diff file.cpp"
From: Eric Raible @ 2008-10-07 3:45 UTC (permalink / raw)
To: Jeff King; +Cc: Git Mailing List, paulus
In-Reply-To: <20081007022705.GA4496@coredump.intra.peff.net>
On Mon, Oct 6, 2008 at 7:27 PM, Jeff King <peff@peff.net> wrote:
> On Mon, Oct 06, 2008 at 07:04:14PM -0700, Eric Raible wrote:
>
>> Anyway what I'm looking for is the gitk equivalent of:
>>
>> git log -p --full-diff file.cpp
>
> Try turning off the option "Limit diffs to listed paths" in the
> preferences menu (or adding "set limitdiffs 0" to your ~/.gitk).
Yes, that's exactly what I was looking for.
Thanks - Eric
^ permalink raw reply
* Re: GitTogether topics status (4th of October)
From: Nicolas Pitre @ 2008-10-07 3:46 UTC (permalink / raw)
To: Jeff King
Cc: Christian Couder, git, Shawn O. Pearce, Junio C Hamano,
Scott Chacon, Sam Vilain, Petr Baudis
In-Reply-To: <20081007031509.GA6031@coredump.intra.peff.net>
On Mon, 6 Oct 2008, Jeff King wrote:
> 2. What needs refactoring?
>
> I occasionally run up against parts of the code that just make my
> eyes bleed everytime I touch them. I think we've made significant
> progress in maintanability and bug-avoidance with things like the
> strbuf library, refactoring of remote and transport handling, etc.
> What areas might still benefit from such refactoring?
Tree object walking and commit object parsing. This is what's holding
pack v4 performances back. But I think that Shawn will cover this
already (I won't be there but I sent my thoughts about this to him).
Nicolas
^ permalink raw reply
* Re: [PATCH] Replace xmalloc/memset(0) pairs with xcalloc
From: Andreas Ericsson @ 2008-10-07 5:12 UTC (permalink / raw)
To: Brandon Casey; +Cc: Git Mailing List
In-Reply-To: <48R7WYLSZ5lElWvJbfurso2ZPBQbzSCmtOgIsqTWyaA8yfW6Ndq1aA@cipher.nrlssc.navy.mil>
Brandon Casey wrote:
> Many call sites immediately initialize allocated memory with zero after
> calling xmalloc. A single call to xcalloc can replace this two-call
> sequence.
>
Nicely done. Apart from reducing LoC count, this is also a slight
optimization as we'll no longer bzero() memory handed to us directly
by the kernel (which is already nul'ed out).
--
Andreas Ericsson andreas.ericsson@op5.se
OP5 AB www.op5.se
Tel: +46 8-230225 Fax: +46 8-230231
^ permalink raw reply
* Re: [PATCH v2] git init: --bare/--shared overrides system/global config
From: Deskin Miller @ 2008-10-07 5:37 UTC (permalink / raw)
To: Shawn O. Pearce; +Cc: git
In-Reply-To: <20081006141452.GA7684@spearce.org>
If core.bare or core.sharedRepository are set in /etc/gitconfig or
~/.gitconfig, then 'git init' will read the values when constructing a
new config file; reading them, however, will override the values
specified on the command line. In the case of --bare, this ends up
causing a segfault, without the repository being properly initialised;
in the case of --shared, the permissions are set according to the
existing config settings, not what was specified on the command line.
This fix saves any specified values for --bare and --shared prior to
reading existing config settings, and restores them after reading but
before writing the new config file. core.bare is ignored in all
situations, while core.sharedRepository will only be used if --shared
is not specified to git init.
Also includes testcases which use a specified global config file
override, demonstrating the former failure scenario.
Signed-off-by: Deskin Miller <deskinm@umich.edu>
---
On Mon, Oct 06, 2008 at 07:14:52AM -0700, Shawn O. Pearce wrote:
> Deskin Miller <deskinm@umich.edu> wrote:
> > From b6144562983703079a1eba8cdac3506c18d751a3 Mon Sep 17 00:00:00 2001
> > From: Deskin Miller <deskinm@umich.edu>
> > Date: Sat, 4 Oct 2008 20:07:44 -0400
>
> FWIW please don't include these lines in the commit message part
> of the patch email. [...]
Alright, sorry for the messiness; I guess I thought preserving the original
commit date was important? Won't happen again.
> > diff --git a/builtin-init-db.c b/builtin-init-db.c
> > index 8140c12..38e282c 100644
> > --- a/builtin-init-db.c
> > +++ b/builtin-init-db.c
> > @@ -191,6 +194,8 @@ static int create_default_files(const char *template_path)
> > copy_templates(template_path);
> >
> > git_config(git_default_config, NULL);
> > + is_bare_repository_cfg = init_is_bare_repository;
> > + shared_repository = init_shared_repository;
>
> Is this really the right thing to do? It seems like it would prevent
> a user from setting core.sharedRepository = group in their template
> and thus always have a shared repository on their system.
You're right. Fixed in this version: core.bare ignores any config, while
--shared will override config settings, but init will use the config settings
if --shared is not specified.
> A second related test would be a ~/.gitconfig which sets
> core.sharedRepository = 0666 and then does "git init". I think
> the right outcome is a repository which has that set.
Good suggestion, I added such a case in this version. My first version fails
this new testcase, while maint fails the original testcase I wrote.
builtin-init-db.c | 12 ++++++++++--
t/t0001-init.sh | 32 ++++++++++++++++++++++++++++++++
2 files changed, 42 insertions(+), 2 deletions(-)
diff --git a/builtin-init-db.c b/builtin-init-db.c
index 8140c12..d30c3fe 100644
--- a/builtin-init-db.c
+++ b/builtin-init-db.c
@@ -17,6 +17,9 @@
#define TEST_FILEMODE 1
#endif
+static int init_is_bare_repository = 0;
+static int init_shared_repository = -1;
+
static void safe_create_dir(const char *dir, int share)
{
if (mkdir(dir, 0777) < 0) {
@@ -191,6 +194,9 @@ static int create_default_files(const char *template_path)
copy_templates(template_path);
git_config(git_default_config, NULL);
+ is_bare_repository_cfg = init_is_bare_repository;
+ if (init_shared_repository != -1)
+ shared_repository = init_shared_repository;
/*
* We would have created the above under user's umask -- under
@@ -277,6 +283,8 @@ int init_db(const char *template_dir, unsigned int flags)
safe_create_dir(get_git_dir(), 0);
+ init_is_bare_repository = is_bare_repository();
+
/* Check to see if the repository version is right.
* Note that a newly created repository does not have
* config file, so this will not fail. What we are catching
@@ -381,9 +389,9 @@ int cmd_init_db(int argc, const char **argv, const char *prefix)
setenv(GIT_DIR_ENVIRONMENT, getcwd(git_dir,
sizeof(git_dir)), 0);
} else if (!strcmp(arg, "--shared"))
- shared_repository = PERM_GROUP;
+ init_shared_repository = PERM_GROUP;
else if (!prefixcmp(arg, "--shared="))
- shared_repository = git_config_perm("arg", arg+9);
+ init_shared_repository = git_config_perm("arg", arg+9);
else if (!strcmp(arg, "-q") || !strcmp(arg, "--quiet"))
flags |= INIT_DB_QUIET;
else
diff --git a/t/t0001-init.sh b/t/t0001-init.sh
index 620da5b..5ac0a27 100755
--- a/t/t0001-init.sh
+++ b/t/t0001-init.sh
@@ -167,4 +167,36 @@ test_expect_success 'init with --template (blank)' '
! test -f template-blank/.git/info/exclude
'
+test_expect_success 'init --bare/--shared overrides system/global config' '
+ (
+ HOME="`pwd`" &&
+ export HOME &&
+ test_config="$HOME"/.gitconfig &&
+ unset GIT_CONFIG_NOGLOBAL &&
+ git config -f "$test_config" core.bare false &&
+ git config -f "$test_config" core.sharedRepository 0640 &&
+ mkdir init-bare-shared-override &&
+ cd init-bare-shared-override &&
+ git init --bare --shared=0666
+ ) &&
+ check_config init-bare-shared-override true unset &&
+ test x0666 = \
+ x`git config -f init-bare-shared-override/config core.sharedRepository`
+'
+
+test_expect_success 'init honors global core.sharedRepository' '
+ (
+ HOME="`pwd`" &&
+ export HOME &&
+ test_config="$HOME"/.gitconfig &&
+ unset GIT_CONFIG_NOGLOBAL &&
+ git config -f "$test_config" core.sharedRepository 0666 &&
+ mkdir shared-honor-global &&
+ cd shared-honor-global &&
+ git init
+ ) &&
+ test x0666 = \
+ x`git config -f shared-honor-global/.git/config core.sharedRepository`
+'
+
test_done
--
1.6.0.2.307.gc427
^ 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