* Re: [git patches] libata updates, GPG signed (but see admin notes)
From: Linus Torvalds @ 2011-11-07 16:24 UTC (permalink / raw)
To: Valdis.Kletnieks
Cc: Ted Ts'o, Junio C Hamano, Shawn Pearce, git, James Bottomley,
Jeff Garzik, Andrew Morton, linux-ide, LKML
In-Reply-To: <22879.1320652337@turing-police.cc.vt.edu>
On Sun, Nov 6, 2011 at 11:52 PM, <Valdis.Kletnieks@vt.edu> wrote:
>
> OK.. I'll bite. How do you disambiguate a '42deadbeef' in the changelog part
> of a commit as being a commit ID, as opposed to being an address in a traceback
> or something similar? Yes, I know you only change the ones that actually map to
> a commit ID, but I'd not be surprised if by now we've got enough commits and
> stack tracebacks in the git history that we'll birthday-paradox ourselves into
> a false-positive in an automatic replacement.
I don't think we are quite there yet. And (sadly) most of the commit
ID's in the history are 7 hex characters, because that used to be the
default git abbreviation. So there is unlikely to be any real
conflicts.
If we do miss one or two, that will be sad and embarrassing, but is
not a real problem in practice.
We probably could add various heuristics (the SHA1 values are *often*
preceded by the string "commit"), and a really good import would also
have somebody at least visually inspecting ones that other heuristics
say might be debatable (for example - because they have 8 hex digits
and there are other numbers around them that were *not* converted),
but in the end perfection is the enemy of good. It's not really worth
the headache to worry about *all* the cases, if you can cheaply and
simply get 99+% right.
And I think the 99% is almost trivial. While the last 1% may or may
not be worth worrying about.
Linus
^ permalink raw reply
* Re: reset reports file as modified when it's in fact deleted
From: Jeff King @ 2011-11-07 16:26 UTC (permalink / raw)
To: Carlos Martín Nieto, git
In-Reply-To: <20111107094330.GB10936@beez.lab.cmartin.tk>
On Mon, Nov 07, 2011 at 10:43:30AM +0100, Carlos Martín Nieto wrote:
> When I delete a file (git rm) and then reset so it exists in the index
> again, the message tells me 'M file.txt' even though the file doesn't
> exist in the worktree anymore. Running git status afterwards does give
> the correct output. So, here's the minimal steps to reproduce:
>
> $ git init
> Initialized empty Git repository in /home/carlos/test/reset-err/.git/
> $ touch file.txt
> $ git add file.txt
> $ git ci file.txt -m initial
> [master (root-commit) a536393] initial
> 0 files changed, 0 insertions(+), 0 deletions(-)
> create mode 100644 file.txt
> $ git rm file.txt
> rm 'file.txt'
> $ git reset -- file.txt
> Unstaged changes after reset:
> M file.txt
> $ git status -b -s
> ## master
> D file.txt
You can simplify this even further by not touching the index at all:
git init -q &&
>file && git add file && git commit -q -m initial &&
rm file &&
git reset
produces:
Unstaged changes after reset:
M file
> I'd expect the output after "Unstaged changes after reset" to tell me
> file.txt has been deleted instead of modified. This happens with
> 1.7.8-rc0, 1.7.7 and 1.7.4.1 and I expect with many more that I don't
> have here.
>
> I thought the index diff code might have been checking the index at the
> wrong time, but I can run 'git reset HEAD -- file.txt' as many times
> as I want, and it will still say 'M', so now I'm not sure.
The index diff code isn't running at all. Those messages are produced by
refresh_index, which outputs only two flags: modified or unmerged. I
think the reason for this is somewhat historical. You would run
"update-index --refresh", and it would helpfully say "by the way, when
refreshing this entry, I noticed that it is in need of being updated in
the index". The text was "file.txt: needs update".
Later, many porcelain commands started to refresh the index themselves,
and the "needs update" message was very confusing. So it was switched to
the more familiar "M file.txt" (though you can still see the original
plumbing message if you run update-index yourself).
I think it is a little more friendly to distinguish deletion from just
modification. And there's shouldn't be any compatibility questions, as
these are explicitly porcelain output (plumbing will still say "needs
update").
There are a few other cases users might expect to see. We'll never show
copies or renames, of course, because we aren't actually doing a diff
with copy detection. We won't see an "A"dded file, because such a file
would be in the working tree but not the index, meaning it is not
tracked.
We could see a "T"ypechange, but the distinction between that and a
modified file is lost by the time we get to refresh_index. We could pass
it up, but I wonder if it's really worth it.
The patch to do "D"eleted is pretty simple:
diff --git a/read-cache.c b/read-cache.c
index dea7cd8..cc1ebdf 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -1103,9 +1103,11 @@ int refresh_index(struct index_state *istate, unsigned int flags, const char **p
int in_porcelain = (flags & REFRESH_IN_PORCELAIN);
unsigned int options = really ? CE_MATCH_IGNORE_VALID : 0;
const char *needs_update_fmt;
+ const char *needs_rm_fmt;
const char *needs_merge_fmt;
needs_update_fmt = (in_porcelain ? "M\t%s\n" : "%s: needs update\n");
+ needs_rm_fmt = (in_porcelain ? "D\t%s\n" : "%s: needs update\n");
needs_merge_fmt = (in_porcelain ? "U\t%s\n" : "%s: needs merge\n");
for (i = 0; i < istate->cache_nr; i++) {
struct cache_entry *ce, *new;
@@ -1145,7 +1147,10 @@ int refresh_index(struct index_state *istate, unsigned int flags, const char **p
}
if (quiet)
continue;
- show_file(needs_update_fmt, ce->name, in_porcelain, &first, header_msg);
+ if (cache_errno == ENOENT)
+ show_file(needs_rm_fmt, ce->name, in_porcelain, &first, header_msg);
+ else
+ show_file(needs_update_fmt, ce->name, in_porcelain, &first, header_msg);
has_errors = 1;
continue;
}
^ permalink raw reply related
* Re: [PATCH 3/3] grep: get rid of useless x < 0 comparison on an enum member
From: Jeff King @ 2011-11-07 16:38 UTC (permalink / raw)
To: Andreas Schwab
Cc: Ævar Arnfjörð Bjarmason, git, Junio C Hamano,
Jim Meyering, Fredrik Gustafsson
In-Reply-To: <m3pqh4krer.fsf@hase.home>
On Mon, Nov 07, 2011 at 02:12:28PM +0100, Andreas Schwab wrote:
> Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:
>
> > I.e. we'll always have GREP_HEADER_AUTHOR = 0 and
> > GREP_HEADER_COMMITTER = 1, we'll never have GREP_HEADER_AUTHOR = and
> > GREP_HEADER_COMMITTER = <some int>.
>
> That is irrelevant. You can always assign -1 to an object of enumerated
> type and the implicit conversion to the underlying type is fully
> defined, no matter what type the compiler choses.
Yes, but now you are getting into implementation-defined behavior, which
is also something to avoid. IOW, I don't think it's wrong for a static
analyzer to complain about:
if (enum_with_only_positive_values < 0)
just because you could say:
enum_with_only_positive_values = -1;
and it would work on _some_ platforms. That same static analyzer should
be complaining about the second line, which is where the real potential
bug is. If you want "-1" as an enumerated value, then add it to your
enum definition and the compiler will do the right thing.
It is perfectly fine to do:
enum foo { a = -1, b, c };
...
if (foo < 0)
...
and the static analyzer should not complain about the conditional there.
-Peff
^ permalink raw reply
* Re: BUG. Git config pager when --edit
From: Jeff King @ 2011-11-07 16:42 UTC (permalink / raw)
To: Frans Klaver; +Cc: Alexey Shumkin, git
In-Reply-To: <CAH6sp9Ox+6p4RkjCZ0j3tXG9F4u7SPuwbSrOWmLSXic9DxSKiQ@mail.gmail.com>
On Mon, Nov 07, 2011 at 02:43:41PM +0100, Frans Klaver wrote:
> > As far as my config is large enough to be paged I set pager.config=less
> > setting. But since that moment when I run
> > $ git config --edit
> > I get
> > Vim: Warning: Output is not to a terminal
> > And some messed config output
> >
> > The same happens if to run
> > $ vim .git/config | less
>
> So git is trying to tell vim to pipe its output to less. vim can't do
> that because it needs a terminal, as it's the only way vim is usable.
>
> Should pager.config then only be used with --list?
Yes, but it can't, because it is not the config command, but the git
wrapper that respects "pager.config". We have similar issues with
setting "pager.tag" (you want it for listing, but not for tag creation)
and others.
I should probably polish and submit the patch here:
http://thread.gmane.org/gmane.comp.version-control.git/182238/focus=182475
-Peff
^ permalink raw reply
* Re: [PATCH v2] pull: introduce a pull.rebase option to enable --rebase
From: Junio C Hamano @ 2011-11-07 16:44 UTC (permalink / raw)
To: Ævar Arnfjörð Bjarmason
Cc: Johannes Sixt, git, Eric Herman, Sverre Rabbelier,
Fernando Vezzosi, Johannes Schindelin
In-Reply-To: <CACBZZX5Lq7vunx-QnsrufQVWJ6xYPoMXnv+tMwhOC3XbrZO11A@mail.gmail.com>
Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:
> On Sun, Nov 6, 2011 at 20:53, Johannes Sixt <j6t@kdbg.org> wrote:
>> When you continue an indented item in a separate paragraph, then you
>> must not use an empty line, but have line with '+' and un-indented
>> continuation paragraphs. See examples in this file.
>
> Thanks for spotting that.
>
> Junio: Should I spam you with another patch or is something you'd
> prefer to just fix up?
It is about the same amount of work for me; I've just dedented the two
paragraphs that start with "*NOTE*:" and replaced the blank lines before
them with a single "+". Is that what you wanted to resend, or is there
anything else?
^ permalink raw reply
* Re: [PATCH] blame.c: Properly initialize strbuf after calling textconv_object(), again
From: Junio C Hamano @ 2011-11-07 16:47 UTC (permalink / raw)
To: Sebastian Schuberth; +Cc: Junio C Hamano, git
In-Reply-To: <4EB7FEE6.9000609@gmail.com>
Sebastian Schuberth <sschuberth@gmail.com> writes:
> 2564aa4 started to initialize buf.alloc, but that should actually be one
> more byte than the string length due to the trailing \0.
Even when the conversion result is a zero-length string?
> ---
Sign-off?
> builtin/blame.c | 2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/builtin/blame.c b/builtin/blame.c
> index 86c0537..45f0dcc 100644
> --- a/builtin/blame.c
> +++ b/builtin/blame.c
> @@ -2114,7 +2114,7 @@ static struct commit *fake_working_tree_commit(struct diff_options *opt,
> case S_IFREG:
> if (DIFF_OPT_TST(opt, ALLOW_TEXTCONV) &&
> textconv_object(read_from, mode, null_sha1, &buf.buf, &buf_len)) {
> - buf.alloc = buf_len;
> + buf.alloc = buf_len + 1;
> buf.len = buf_len;
> }
> else if (strbuf_read_file(&buf, read_from, st.st_size) != st.st_size)
^ permalink raw reply
* Re: [PATCH] blame.c: Properly initialize strbuf after calling textconv_object(), again
From: Jeff King @ 2011-11-07 16:49 UTC (permalink / raw)
To: Sebastian Schuberth; +Cc: git, Junio C Hamano
In-Reply-To: <4EB7FEE6.9000609@gmail.com>
On Mon, Nov 07, 2011 at 04:53:10PM +0100, Sebastian Schuberth wrote:
> --- a/builtin/blame.c
> +++ b/builtin/blame.c
> @@ -2114,7 +2114,7 @@ static struct commit *fake_working_tree_commit(struct diff_options *opt,
> case S_IFREG:
> if (DIFF_OPT_TST(opt, ALLOW_TEXTCONV) &&
> textconv_object(read_from, mode, null_sha1, &buf.buf, &buf_len)) {
> - buf.alloc = buf_len;
> + buf.alloc = buf_len + 1;
> buf.len = buf_len;
> }
> else if (strbuf_read_file(&buf, read_from, st.st_size) != st.st_size)
Your patch is correct in the sense that this fixes a bug, but it just
seems wrong to be touching the alloc parameter outside of the strbuf
code. It looks like the intent is to do the equivalent of:
strbuf_add(&buf, some_heap_buf, len);
free(some_heap_buf);
but avoid the extra allocation and copy. Should there be a
"strbuf_attach" function to encapsulate this pattern?
-Peff
^ permalink raw reply
* Re: [PATCH 1/4] Documentation/gitignore: "foo/" patterns match directories, not files under them
From: Junio C Hamano @ 2011-11-07 16:53 UTC (permalink / raw)
To: Nguyen Thai Ngoc Duy
Cc: Jonathan Nieder, git, Eric Blake, Johannes Sixt, Y.G.,
Eli Barzilay
In-Reply-To: <CACsJy8CZFihbS8MrG=0gWdRPu6F0BqG2FLp48KDxOXWc+4amuQ@mail.gmail.com>
Nguyen Thai Ngoc Duy <pclouds@gmail.com> writes:
> 2011/11/7 Jonathan Nieder <jrnieder@gmail.com>:
>> The gitignore(5) manpage says that "foo/" will match a directory foo
>> and paths underneath it.
>
> If git ignores a directory, then it essentially ignores all paths
> underneath it, doesn't it?
>
>> But that is completely false: as Johannes
>> Sixt likes to remind us, patterns with a trailing '/' match the named
>> directory, not files under that directory. For example, the following
>> .gitignore file
>>
>> /build/
>> !/build/tests/results
>>
>> does not un-ignore build/tests/results since it was never ignored in
>> the first place; and commands like "git status" will not notice
>> changes to build/tests/results because git doesn't enter the (ignored)
>> build/ directory.
>
> I haven't checked but I think it's because when a directory is
> ignored, git just stops checking further ignore rules. So "build" _is_
> ignored, too strongly that it does not care if some files may need to
> be un-ignored later on.
>
> I remember the argument was, because ignore rules are distributed
> across .gitignore files, we would need to go into ignored directories
> for collecting potential un-ignore rules (for example "!results" on
> build/tests/.gitignore) and that just does not make much sense because
> we always have to go into ignored directories.
>
> But in your example, where we know we have negated rules, we should
> follow the rules and ignore all but build/tests/results.
>
>> Correct the manual to just say that "foo/" matches the directory
>> "foo", and make the wording a little clearer in other ways while at
>> it.
>
> I haven't not read the next patches, maybe you have mentioned this
> already. We should make clear that git does not look for negated rules
> once a directory is ignored.
>
> Your example however demonstrates a bug that should be fixed in my
> opinion. So maybe one or two lines under BUGS section.
>
>> Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
>> ---
>> Documentation/gitignore.txt | 14 ++++++++------
>> 1 files changed, 8 insertions(+), 6 deletions(-)
>>
>> diff --git a/Documentation/gitignore.txt b/Documentation/gitignore.txt
>> index 2e7328b8..5b070bf0 100644
>> --- a/Documentation/gitignore.txt
>> +++ b/Documentation/gitignore.txt
>> @@ -72,12 +72,14 @@ PATTERN FORMAT
>> included again. If a negated pattern matches, this will
>> override lower precedence patterns sources.
>>
>> - - If the pattern ends with a slash, it is removed for the
>> - purpose of the following description, but it would only find
>> - a match with a directory. In other words, `foo/` will match a
>> - directory `foo` and paths underneath it, but will not match a
>> - regular file or a symbolic link `foo` (this is consistent
>> - with the way how pathspec works in general in git).
>> + - If the pattern ends with a slash, it will only match
>> + directories. In other words, `foo/` will match a
>> + directory `foo` but will not match a regular file or a
>> + symbolic link `foo` (this is consistent with the way
>> + pathspecs work in general in git).
>
> Looks good.
Or just remove "In other words, ..." that is bogus. Everything before that
is correct.
>> ++
>> +The trailing slash is removed before applying the remaining
>> +rules.
>
> Why does the trailing slash of a rule affect the remaining rules?
Later rule makes a path with and without a slash in it work differently.
A single token "foo/" acts as if there is no trailing slash to match
any directory in the hierarchy, e.g. it matches a directory "frotz/foo".
^ permalink raw reply
* Re: git-receive-pack missing credentials ?
From: Tay Ray Chuan @ 2011-11-07 17:02 UTC (permalink / raw)
To: François Dagorn; +Cc: git
In-Reply-To: <4EB7FA3A.8070908@univ-rennes1.fr>
On Mon, Nov 7, 2011 at 11:33 PM, François Dagorn
<Francois.Dagorn@univ-rennes1.fr> wrote:
> - on the client side
>
> cd (project-directory)
> $ git init
> $ (add some files)
> $ git add .
> $ git commit -m 'Initial commit'
> git push http://git.istic.smw.fr/test13 master
>
> Username:
> Password:
> error: Cannot access URL http://git.istic.smw.fr/test13/, return code 22
> fatal: git-http-push failed
>
> - apache acces_log
>
> 1) ip-address - metheuser [07/Nov/2011:16:17:31 +0100]
> "GET /test13/info/refs?service=git- receive-pack HTTP/1.1" 200 - "-" "git/1.7.3.4"
> 2) ip-address - metheuser [07/Nov/2011:16:17:32 +0100]
> "GET /test13/HEAD HTTP/1.1" 200 23 "-" "git/1.7.3.4"
> 3) ip-address - - [07/Nov/2011:16:17:32 +0100]
> "PROPFIND /test13/ HTTP/1.1" 401 492 "-" "git/1.7.3.4"
>
> - what sounds strange to me : the 2 firsts requests are generated by my client side
> (wireshark used as a clue) but the third comes from the server side and the users
> credentials are missed !
Can you update your git installation and try again? v1.7.3.4 sounds
pretty old (almost a year).
--
Cheers,
Ray Chuan
^ permalink raw reply
* Re: BUG. Git config pager when --edit
From: Junio C Hamano @ 2011-11-07 17:02 UTC (permalink / raw)
To: Jeff King; +Cc: Frans Klaver, Alexey Shumkin, git
In-Reply-To: <20111107164250.GC27055@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
> I should probably polish and submit the patch here:
>
> http://thread.gmane.org/gmane.comp.version-control.git/182238/focus=182475
I was actually hoping that you won't go that route, but the route to push
further to decide/spawn pager as late as possible. Clearly no sane person
would want to run --edit subcommand under pager and "pager.config = less"
^ permalink raw reply
* Re: BUG. Git config pager when --edit
From: Jeff King @ 2011-11-07 17:18 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Frans Klaver, Alexey Shumkin, git
In-Reply-To: <7vpqh327ds.fsf@alter.siamese.dyndns.org>
On Mon, Nov 07, 2011 at 09:02:23AM -0800, Junio C Hamano wrote:
> Jeff King <peff@peff.net> writes:
>
> > I should probably polish and submit the patch here:
> >
> > http://thread.gmane.org/gmane.comp.version-control.git/182238/focus=182475
>
> I was actually hoping that you won't go that route, but the route to push
> further to decide/spawn pager as late as possible. Clearly no sane person
> would want to run --edit subcommand under pager and "pager.config = less"
> should just be ignored in such a case.
The problem with that is that it dumps the responsibility for running
the pager to every subcommand. For builtins, we can have a flag that
says "respect the pager.log config" or "foo will handle this itself;
don't respect pager.tag".
But what about externals? If "pager.stash" does nothing in git.c, and
leaves it to "git-stash.sh" to start the pager if and when it's
appropriate, then what about my personal "git-foo" that I drop into my
PATH? Now I can't use "config.foo" without carrying code to do so in my
external command.
Maybe that's an OK tradeoff. But it's more of a pain for existing
scripts, and it's not backwards compatible. What do you think?
-Peff
^ permalink raw reply
* Re: [RFC/PATCH] remote: add new sync command
From: Jeff King @ 2011-11-07 17:22 UTC (permalink / raw)
To: Felipe Contreras; +Cc: git
In-Reply-To: <1320682032-12698-1-git-send-email-felipe.contreras@gmail.com>
On Mon, Nov 07, 2011 at 06:07:12PM +0200, Felipe Contreras wrote:
> This is useful to mirror all the branches in the current repo to
> another.
> [...]
> +'sync'::
> +
> +Synchronizes local branches with certain remote. This is useful to backup all
> +the branches in a local repository to a remote one, regardless of what upstream
> +is configured for each branch.
> ++
> +With `--prune`, remote branches will be deleted if they are not also locally.
> ++
> +With `--new`, local branches that are not yet in the remote will be pushed too.
> ++
> +With `--all`, basically both `--prune` and `--new` will be selected.
> ++
> +With `--force`, existing branches will be forced to update, like `git push
> +--force`.
> ++
> +With `--dry-run`, all the changes will be reported, but not really happen.
Why is this in "git remote", and not "git push"? The former is usually
about managing the configuration of remotes, not about actually doing
the ref transfer (the "-f" flag excepted, but that is clearly just
calling out to "fetch").
And how does this differ from "git push --mirror"? It looks like you
have more options for what pushing all versus pruning, but wouldn't it
be better for "git push" to grow those options?
-Peff
^ permalink raw reply
* Is there a pdf git manual?
From: Peng Yu @ 2011-11-07 17:24 UTC (permalink / raw)
To: git
Hi,
http://schacon.github.com/git/user-manual.html
The manual is in html. I'm not able to find a pdf version. Running
make in git/Documentation doesn't generate a pdf document
automatically. Could anybody generated the pdf document and post it to
the git project website? Thanks!
--
Regards,
Peng
^ permalink raw reply
* Re: [PATCH] blame.c: Properly initialize strbuf after calling textconv_object(), again
From: Sebastian Schuberth @ 2011-11-07 17:31 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7vy5vr282s.fsf@alter.siamese.dyndns.org>
On 07.11.2011 17:47, Junio C Hamano wrote:
>> 2564aa4 started to initialize buf.alloc, but that should actually be one
>> more byte than the string length due to the trailing \0.
>
> Even when the conversion result is a zero-length string?
In this case, yes. The string buffer is initialized (and detached) in
run_textconv, which calls strbuf_read, which grows the (yet empty)
string to 8192 bytes. So alloc is always > 0, and the detach will trim
alloc to len + 1.
However, Peff made another valid point and I'll send v2 soon.
--
Sebastian Schuberth
^ permalink raw reply
* [PATCHv2] blame.c: Properly initialize strbuf after calling textconv_object(), again
From: Sebastian Schuberth @ 2011-11-07 17:33 UTC (permalink / raw)
To: Jeff King; +Cc: git, Junio C Hamano
In-Reply-To: <20111107164946.GD27055@sigill.intra.peff.net>
2564aa4 started to initialize buf.alloc, but that should actually be one
more byte than the string length due to the trailing \0. Also, do not
modify buf.alloc out of the strbuf code. Use the existing strbuf_attach
instead.
Signed-off-by: Sebastian Schuberth <sschuberth@gmail.com>
---
builtin/blame.c | 7 +++----
1 files changed, 3 insertions(+), 4 deletions(-)
diff --git a/builtin/blame.c b/builtin/blame.c
index 86c0537..80febbe 100644
--- a/builtin/blame.c
+++ b/builtin/blame.c
@@ -2096,6 +2096,7 @@ static struct commit *fake_working_tree_commit(struct diff_options *opt,
if (!contents_from || strcmp("-", contents_from)) {
struct stat st;
const char *read_from;
+ char *buf_ptr;
unsigned long buf_len;
if (contents_from) {
@@ -2113,10 +2114,8 @@ static struct commit *fake_working_tree_commit(struct diff_options *opt,
switch (st.st_mode & S_IFMT) {
case S_IFREG:
if (DIFF_OPT_TST(opt, ALLOW_TEXTCONV) &&
- textconv_object(read_from, mode, null_sha1, &buf.buf, &buf_len)) {
- buf.alloc = buf_len;
- buf.len = buf_len;
- }
+ textconv_object(read_from, mode, null_sha1, &buf_ptr, &buf_len))
+ strbuf_attach(&buf, buf_ptr, buf_len, buf_len + 1);
else if (strbuf_read_file(&buf, read_from, st.st_size) != st.st_size)
die_errno("cannot open or read '%s'", read_from);
break;
--
1.7.8.rc0.47.gf2c75.dirty
^ permalink raw reply related
* Re: [PATCHv2] blame.c: Properly initialize strbuf after calling textconv_object(), again
From: Jeff King @ 2011-11-07 17:41 UTC (permalink / raw)
To: Sebastian Schuberth; +Cc: git, Junio C Hamano
In-Reply-To: <4EB8166E.9000703@gmail.com>
On Mon, Nov 07, 2011 at 06:33:34PM +0100, Sebastian Schuberth wrote:
> 2564aa4 started to initialize buf.alloc, but that should actually be one
> more byte than the string length due to the trailing \0. Also, do not
> modify buf.alloc out of the strbuf code. Use the existing strbuf_attach
> instead.
Heh. When I wrote the previous email I just made up the name
"strbuf_attach" as the opposite of "strbuf_detach". I didn't know it
actually already existed.
> - textconv_object(read_from, mode, null_sha1, &buf.buf, &buf_len)) {
> - buf.alloc = buf_len;
> - buf.len = buf_len;
> - }
> + textconv_object(read_from, mode, null_sha1, &buf_ptr, &buf_len))
> + strbuf_attach(&buf, buf_ptr, buf_len, buf_len + 1);
Looks OK to me.
-Peff
^ permalink raw reply
* Re: [PATCH 3/3] grep: get rid of useless x < 0 comparison on an enum member
From: Andreas Schwab @ 2011-11-07 18:24 UTC (permalink / raw)
To: Jeff King
Cc: Ævar Arnfjörð Bjarmason, git, Junio C Hamano,
Jim Meyering, Fredrik Gustafsson
In-Reply-To: <20111107163823.GB27055@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
> Yes, but now you are getting into implementation-defined behavior, which
> is also something to avoid.
The whole point of the statement is a sanity check to uncover bugs. If
you remove the first condition you completely ruin its point.
Andreas.
--
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5
"And now for something completely different."
^ permalink raw reply
* Re: Is there a pdf git manual?
From: Tony Godshall @ 2011-11-07 18:29 UTC (permalink / raw)
To: Peng Yu; +Cc: git
In-Reply-To: <CABrM6wkzV58WLnHkZ88y=MQVWjD8dwYMtG9HTto8t8QXBEW-hA@mail.gmail.com>
CUPS-PDF is a free program
that will generate PDFs from
any document- it's as easy as
printing.
On Mon, Nov 7, 2011 at 9:24 AM, Peng Yu <pengyu.ut@gmail.com> wrote:
> Hi,
>
> http://schacon.github.com/git/user-manual.html
>
> The manual is in html. I'm not able to find a pdf version. Running
> make in git/Documentation doesn't generate a pdf document
> automatically. Could anybody generated the pdf document and post it to
> the git project website? Thanks!
>
> --
> Regards,
> Peng
> --
> To unsubscribe from this list: send the line "unsubscribe git" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
--
Best Regards.
This is unedited.
^ permalink raw reply
* Re: [PATCH 3/3] grep: get rid of useless x < 0 comparison on an enum member
From: Jeff King @ 2011-11-07 18:34 UTC (permalink / raw)
To: Andreas Schwab
Cc: Ævar Arnfjörð Bjarmason, git, Junio C Hamano,
Jim Meyering, Fredrik Gustafsson
In-Reply-To: <m2k47b4wqi.fsf@igel.home>
On Mon, Nov 07, 2011 at 07:24:05PM +0100, Andreas Schwab wrote:
> Jeff King <peff@peff.net> writes:
>
> > Yes, but now you are getting into implementation-defined behavior, which
> > is also something to avoid.
>
> The whole point of the statement is a sanity check to uncover bugs. If
> you remove the first condition you completely ruin its point.
I'm somewhat dubious of the value of a bug-check that may or may not
actually kick in depending on your compiler's choice of enum
representation, and whose bugs are generally easier to check via static
analysis (i.e., making sure the enum value is one of the enumerated
values when it is initialized or assigned).
Yes, static analysis can miss some bugs (like passing the address of the
enum through a void pointer (e.g., when memset'ing a struct)). But
couldn't it just as easily be out of range in the other direction?
It seems like the bug trying to be caught is probably something like:
enum foo v = function_which_returns_value_or_negative_error();
if (v < 0)
die("BUG");
But in that case the bug is on the first line, and it is easily caught
by static analysis, no?
-Peff
^ permalink raw reply
* Re: [RFC/PATCH] remote: add new sync command
From: Felipe Contreras @ 2011-11-07 18:35 UTC (permalink / raw)
To: Jeff King; +Cc: git
In-Reply-To: <20111107172218.GB3621@sigill.intra.peff.net>
On Mon, Nov 7, 2011 at 7:22 PM, Jeff King <peff@peff.net> wrote:
> On Mon, Nov 07, 2011 at 06:07:12PM +0200, Felipe Contreras wrote:
>
>> This is useful to mirror all the branches in the current repo to
>> another.
>> [...]
>> +'sync'::
>> +
>> +Synchronizes local branches with certain remote. This is useful to backup all
>> +the branches in a local repository to a remote one, regardless of what upstream
>> +is configured for each branch.
>> ++
>> +With `--prune`, remote branches will be deleted if they are not also locally.
>> ++
>> +With `--new`, local branches that are not yet in the remote will be pushed too.
>> ++
>> +With `--all`, basically both `--prune` and `--new` will be selected.
>> ++
>> +With `--force`, existing branches will be forced to update, like `git push
>> +--force`.
>> ++
>> +With `--dry-run`, all the changes will be reported, but not really happen.
>
> Why is this in "git remote", and not "git push"? The former is usually
> about managing the configuration of remotes, not about actually doing
> the ref transfer (the "-f" flag excepted, but that is clearly just
> calling out to "fetch").
I don't know, seems logical to me what 'git remote sync' does, but
'git push sync'? That sounds weird, and there are no 'git push foo'
commands.
> And how does this differ from "git push --mirror"? It looks like you
> have more options for what pushing all versus pruning, but wouldn't it
> be better for "git push" to grow those options?
But how? --mirror is just an option, I want a separate command, with
it's own options.
Cheers.
--
Felipe Contreras
^ permalink raw reply
* Re: [RFC/PATCH] remote: add new sync command
From: Jeff King @ 2011-11-07 18:39 UTC (permalink / raw)
To: Felipe Contreras; +Cc: git
In-Reply-To: <CAMP44s358k4EsCg+K6MeLEU4eLbb4mWyX9AdAf4P9CHvf9Lrwg@mail.gmail.com>
On Mon, Nov 07, 2011 at 08:35:07PM +0200, Felipe Contreras wrote:
> I don't know, seems logical to me what 'git remote sync' does, but
> 'git push sync'? That sounds weird, and there are no 'git push foo'
> commands.
What I don't understand is why it is not:
git push --mirror <URL|remote>
> > And how does this differ from "git push --mirror"? It looks like you
> > have more options for what pushing all versus pruning, but wouldn't it
> > be better for "git push" to grow those options?
>
> But how? --mirror is just an option, I want a separate command, with
> it's own options.
That's what I don't understand from your proposal. Your command is just
pushing something to the remote, right? Why isn't "push" the command,
and your sync options become options to push?
Can you step back and describe the problem you're trying to solve? Maybe
we're not connecting there.
-Peff
^ permalink raw reply
* Re: [PATCH v2] pull: introduce a pull.rebase option to enable --rebase
From: Ævar Arnfjörð Bjarmason @ 2011-11-07 18:48 UTC (permalink / raw)
To: Junio C Hamano
Cc: Johannes Sixt, git, Eric Herman, Sverre Rabbelier,
Fernando Vezzosi, Johannes Schindelin
In-Reply-To: <7v39dz3ms7.fsf@alter.siamese.dyndns.org>
On Mon, Nov 7, 2011 at 17:44, Junio C Hamano <gitster@pobox.com> wrote:
> Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:
>
>> On Sun, Nov 6, 2011 at 20:53, Johannes Sixt <j6t@kdbg.org> wrote:
>>> When you continue an indented item in a separate paragraph, then you
>>> must not use an empty line, but have line with '+' and un-indented
>>> continuation paragraphs. See examples in this file.
>>
>> Thanks for spotting that.
>>
>> Junio: Should I spam you with another patch or is something you'd
>> prefer to just fix up?
>
> It is about the same amount of work for me; I've just dedented the two
> paragraphs that start with "*NOTE*:" and replaced the blank lines before
> them with a single "+". Is that what you wanted to resend, or is there
> anything else?
That should cover it, thanks!
^ permalink raw reply
* Re: Is there a pdf git manual?
From: Jakub Narebski @ 2011-11-07 18:53 UTC (permalink / raw)
To: Peng Yu; +Cc: git
In-Reply-To: <CABrM6wkzV58WLnHkZ88y=MQVWjD8dwYMtG9HTto8t8QXBEW-hA@mail.gmail.com>
Peng Yu <pengyu.ut@gmail.com> writes:
> Hi,
>
> http://schacon.github.com/git/user-manual.html
>
> The manual is in html. I'm not able to find a pdf version. Running
> make in git/Documentation doesn't generate a pdf document
> automatically. Could anybody generated the pdf document and post it to
> the git project website? Thanks!
"make pdf" would generate PDF version of (some of) documentation.
--
Jakub Narębski
^ permalink raw reply
* Re: [PATCH 3/3] grep: get rid of useless x < 0 comparison on an enum member
From: Jeff King @ 2011-11-07 18:55 UTC (permalink / raw)
To: Andreas Schwab
Cc: Ævar Arnfjörð Bjarmason, git, Junio C Hamano,
Jim Meyering, Fredrik Gustafsson
In-Reply-To: <20111107183402.GA5118@sigill.intra.peff.net>
On Mon, Nov 07, 2011 at 01:34:02PM -0500, Jeff King wrote:
> > The whole point of the statement is a sanity check to uncover bugs. If
> > you remove the first condition you completely ruin its point.
> [...]
> Yes, static analysis can miss some bugs (like passing the address of the
> enum through a void pointer (e.g., when memset'ing a struct)). But
> couldn't it just as easily be out of range in the other direction?
Oops, I just looked at the actual conditional, and it is indeed
range-checking the whole enum. So if you are going to do that at all,
then yes, checking both sides is sensible, and that is what the original
conditional does. Sorry for my confusion.
I do still question the value of the check at all, though, given that
static analysis can find those bugs.
-Peff
^ permalink raw reply
* Re: [PATCH 3/3] grep: get rid of useless x < 0 comparison on an enum member
From: Ævar Arnfjörð Bjarmason @ 2011-11-07 19:06 UTC (permalink / raw)
To: Jeff King
Cc: Andreas Schwab, git, Junio C Hamano, Jim Meyering,
Fredrik Gustafsson
In-Reply-To: <20111107185536.GA5450@sigill.intra.peff.net>
On Mon, Nov 7, 2011 at 19:55, Jeff King <peff@peff.net> wrote:
> I do still question the value of the check at all, though, given that
> static analysis can find those bugs.
Agreed, which is why I submitted this patch.
Also if this is the sort of thing we'd like to guard against we should
be discussing it more generally. We have a bunch of occurances where
we'd probably break down if someone manually assigned -1 to an enum
variable:
$ git grep -E -A1 'enum.*\{' | grep -B1 '=.*0' | grep enum
builtin/branch.c:enum color_branch {
builtin/branch.c:static enum merge_filter {
builtin/fetch-pack.c:enum ack_type {
builtin/fetch.c:enum {
builtin/grep.c: enum {
builtin/mv.c: enum update_mode { BOTH = 0, WORKING_DIRECTORY,
INDEX } *modes;
builtin/remote.c:enum {
builtin/remote.c: enum {
cache.h:enum rebase_setup_type {
cache.h:enum push_default_type {
cache.h:enum object_creation_mode {
cache.h:enum sharedrepo {
cache.h:enum date_mode {
cache.h: enum {
convert.h:enum safe_crlf {
convert.h:enum auto_crlf {
diff.h:enum diff_words_type {
diff.h:enum color_diff {
dir.c:enum exist_status {
dir.h: enum {
grep.h:enum grep_header_field {
http-push.c:enum dav_header_flag {
imap-send.c:enum CAPABILITY {
log-tree.c:enum decoration_type {
merge-recursive.c:enum rename_type {
merge-recursive.h: enum {
notes-merge.h: enum {
remote.h:enum match_refs_flags {
rerere.c: enum {
unpack-trees.h:enum unpack_trees_error_types {
wt-status.h:enum color_wt_status {
^ 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