* Re: [PATCH] clone: forbid --bare --separate-git-dir <dir>
From: Junio C Hamano @ 2013-01-06 23:13 UTC (permalink / raw)
To: Jonathan Nieder
Cc: Nguyễn Thái Ngọc Duy, git, Jens Lehmann,
Heiko Voigt, Manlio Perillo, W. Trevor King
In-Reply-To: <20130106101948.GD10956@elie.Belkin>
Jonathan Nieder <jrnieder@gmail.com> writes:
> Nguyễn Thái Ngọc Duy wrote:
>
>> --separate-git-dir was added to clone with the repository away from
>> standard position <worktree>/.git. It does not make sense to use it
>> without creating working directory.
>>
>> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
>
> The patch correctly implements the above. The description leaves out
> detail. I'd say something like
>
> The --separate-git-dir option was introduced to make it simple
> to put the git directory somewhere outside the worktree, for
> example when cloning a repository for use as a submodule.
>
> It was not intended for use when creating a bare repository.
> In that case there is no worktree and it is more natural to
> directly clone the repository and create a .git file as
> separate steps:
>
> git clone --bare /path/to/repo.git bar.git
> printf 'gitdir: bar.git\n' >foo.git
>
> Unfortunately we forgot to forbid the --bare
> --separate-git-dir combination. In practice, we know no one
> could be using --bare with --separate-git-dir because it is
> broken in the following way: <explanation here>. So it is
> safe to make good on our mistake and forbid the combination,
> making the command easier to explain.
>
> I don't know what would go in the <explanation here> blank above,
> though. Is it possible that some people are relying on this option
> combination?
I do not necessarily think we must say "it happens not to work
already for such and such reasons, lucky us!", but it is indeed a
good idea to think things through, justifying why this cannot be a
regression, and record the fact that we did that thinking, in the
log message.
Thanks.
^ permalink raw reply
* Re: [PATCH] git-fast-import(1): reorganise options
From: Junio C Hamano @ 2013-01-06 23:10 UTC (permalink / raw)
To: John Keeping
Cc: Jonathan Nieder, Eric S. Raymond, git, David Michael Barr,
Pete Wyckoff, Thomas Rast
In-Reply-To: <20130106142825.GH6440@serenity.lan>
John Keeping <john@keeping.me.uk> writes:
> On Sun, Jan 06, 2013 at 05:51:09AM -0800, Jonathan Nieder wrote:
> ...
>> Nice description.
>>
>> > While doing this, fix the duplicate '--done' documentation by taking the
>> > best bits of each. Also combine the descriptions of '--relative-marks'
>> > and '--no-relative-marks' since they make more sense together.
>>
>> I'd prefer to keep those as separate patches, if that's manageable.
>
> I'll send a series of three patches if the discussion below seems
> reasonable:
>
> [1/3] remove duplicate '--done'
> [2/3] combine --[no-]relative-marks
> [3/3] reorganize options
Sounds sensible and I like the direction in which this discussion is
progressing.
>> I'd worry that the catch-all toplevel category would grow larger
>> and larger with time, since it's the obvious place to put any new
>> option.
>
> I agree that that's a concern, perhaps '--cat-blob-fd' should be
> combined with '--date-format' and '--done' into a section called
> "Options for frontends" or similar?
>
> And maybe '--export-pack-edges' can move to the performance/compression
> tuning section? I expect the interested audience would be the same.
>
> That only leaves three options in that section, which seems more
> reasonable.
I'll leave it to others to decide which individual options would
fall into that catch-all category, but the idea you outlined above
sounds sensible overall.
> I realise it's personal taste, but I like the subheadings of the form
> "Options (for|related to) ...", so maybe:
>
> Options for input stream features
> Options related to marks files
> Options for performance and compression tuning
Again, sounds sensible.
>> I like how you put important options like --force on top. Perhaps
>> the less important --quiet and --stats could be split off from that
>> into a subsection like "Verbosity" to make them stand out even more.
>
> I quite like having the verbosity options near the top since those are
> the ones that are most likely to be of interest to a user, whereas the
> rest are likely to be prescribed by the frontend (or only really useful
> to frontend authors).
I tend to agree with Jonathan that verbosity options are less
important ones than the ones that affect how things work.
Thanks.
^ permalink raw reply
* Re: [PATCH 04/10] mailmap: simplify map_user() interface
From: Junio C Hamano @ 2013-01-06 22:56 UTC (permalink / raw)
To: Antoine Pelisse; +Cc: git
In-Reply-To: <1357421206-5014-5-git-send-email-apelisse@gmail.com>
Antoine Pelisse <apelisse@gmail.com> writes:
> diff --git a/builtin/blame.c b/builtin/blame.c
> index dd4aff9..86450e3 100644
> --- a/builtin/blame.c
> +++ b/builtin/blame.c
> ...
> @@ -1356,51 +1356,61 @@ static void get_ac_line(const char *inbuf, const char *what,
> len = strlen(tmp);
> else
> len = endp - tmp;
>
> if (split_ident_line(&ident, tmp, len)) {
> error_out:
> /* Ugh */
> + tmp = "(unknown)";
> + strbuf_addstr(name, tmp);
> + strbuf_addstr(mail, tmp);
> + strbuf_addstr(tz, tmp);
> *time = 0;
> return;
> }
>
> namelen = ident.name_end - ident.name_begin;
> + tmpname = ident.name_begin;
>
> + maillen = ident.mail_end - ident.mail_begin;
> + tmpmail = ident.mail_begin;
>
> *time = strtoul(ident.date_begin, NULL, 10);
>
> + len = ident.tz_end - ident.tz_begin;
> + strbuf_add(tz, ident.tz_begin, len);
>
> /*
> * Now, convert both name and e-mail using mailmap
> */
> + map_user(&mailmap, &tmpmail, &maillen,
> + &tmpname, &namelen);
I like the general simplification this change gives us, but do we
still want to name these variables "tmp"-something? At least to me,
it makes it look like the variable holds a pointer to a piece of
memory that was temporarily allocated. Calling it "mail_begin" or
something might be less confusing.
The changes to pretty.c (pp_user_info) and shortlog.c
(insert_one_record) calls these variables mailbuf and namebuf,
so perhaps these are better names?
^ permalink raw reply
* Re: [PATCH v3 11/19] dir.c: use a single struct exclude_list per source of excludes
From: Adam Spiers @ 2013-01-06 22:53 UTC (permalink / raw)
To: git list
In-Reply-To: <7v7gnqnjn7.fsf@alter.siamese.dyndns.org>
On Sun, Jan 06, 2013 at 12:25:48PM -0800, Junio C Hamano wrote:
> Adam Spiers <git@adamspiers.org> writes:
>
> > On Fri, Jan 04, 2013 at 01:03:59PM -0800, Junio C Hamano wrote:
> >> Adam Spiers <git@adamspiers.org> writes:
> >>
> >> > diff --git a/builtin/clean.c b/builtin/clean.c
> >> > index 0c7b3d0..bd18b88 100644
> >> > --- a/builtin/clean.c
> >> > +++ b/builtin/clean.c
> >> > @@ -97,9 +97,10 @@ int cmd_clean(int argc, const char **argv, const char *prefix)
> >> > if (!ignored)
> >> > setup_standard_excludes(&dir);
> >> >
> >> > + add_exclude_list(&dir, EXC_CMDL);
> >> > for (i = 0; i < exclude_list.nr; i++)
> >> > add_exclude(exclude_list.items[i].string, "", 0,
> >> > - &dir.exclude_list[EXC_CMDL]);
> >> > + &dir.exclude_list_groups[EXC_CMDL].ary[0]);
> >>
> >> This looks somewhat ugly for two reasons.
> >>
> >> * The abstraction add_exclude() offers to its callers is just to
> >> let them add one pattern to the list of patterns for the kind
> >> (here, EXC_CMDL); why should they care about .ary[0] part?
> >
> > Because the caller has to decide which exclude list the new exclude
> > should be added to; that is how it has been for a long time, and I am
> > not proposing we change that.
>
> Unless I was mistaken, I never objected to the EXC_CMDL, etc
> appearing in the text of the calling site of add_exclude().
>
> The objection was about the .ary[0] bit. From the point of view of
> a caller of the API, it:
>
> - calls add_exclude_list() to declare "I now start adding new
> patterns that come from a new source of patterns"; then
>
> - calls add_exclude() repeatedly to add the patterns that come
> from that source.
>
> no?
Correct.
> Why does the latter has to keep repeating "Here is the new
> pattern for the EXC_CMDL group; it comes from the latest source I
> earlier declared, by the way", instead of just "Here is the new
> pattern for the EXC_CMDL group"?
Mainly because there is no guarantee that such a group exists.
unpack_trees() has:
if (add_excludes_from_file_to_list(git_path("info/sparse-checkout"), "", 0, &el, 0) < 0)
so if you change the signature of add_exclude() to require an
exclude_list_group, then there is no way to implement
add_excludes_from_file_to_list().
Even if you could, you still haven't reduced the number of parameters
add_exclude() requires, so I'm dubious of the benefits of this
"simplification".
> >> Are
> >> there cases any sane caller (not the implementation of the
> >> exclude_list_group machinery e.g. add_excludes_from_... function)
> >> may want to call it with .ary[1]?
> >
> > Effectively yes, although it is not written like ".ary[1]". For
> > example prep_exclude() calls add_excludes_from_file_to_list() for each
> > new .gitignore file
>
> That is part of the "implementation of the machinery". If the API
> for the outside callers are to call add_exclude_list() to declare
> that patterns added by subsequent calls to add_exclude() are from
> one new source of the patterns (e.g. .gitignore file in a new
> directory level), and then call add_exclude() to add each pattern,
> then the callers to add_exclude() shouldn't have to care about the
> implementation detail that individual sources in exclude_list_group
> is implemented as an array in that sructure, and the latest ones
> should go to its ->array[0].
That's a valid point. However, the ary[0] part which assumes external
knowledge of the internal implementation can trivially be avoided by
squashing this patch onto the commit we are discussing:
diff --git a/builtin/clean.c b/builtin/clean.c
index dd89737..6e21ca6 100644
--- a/builtin/clean.c
+++ b/builtin/clean.c
@@ -45,6 +45,7 @@ int cmd_clean(int argc, const char **argv, const char *prefix)
static const char **pathspec;
struct strbuf buf = STRBUF_INIT;
struct string_list exclude_list = STRING_LIST_INIT_NODUP;
+ struct exclude_list *el;
const char *qname;
char *seen = NULL;
struct option options[] = {
@@ -97,10 +98,9 @@ int cmd_clean(int argc, const char **argv, const char *prefix)
if (!ignored)
setup_standard_excludes(&dir);
- add_exclude_list(&dir, EXC_CMDL);
+ el = add_exclude_list(&dir, EXC_CMDL);
for (i = 0; i < exclude_list.nr; i++)
- add_exclude(exclude_list.items[i].string, "", 0,
- &dir.exclude_list_group[EXC_CMDL].el[0]);
+ add_exclude(exclude_list.items[i].string, "", 0, el);
pathspec = get_pathspec(prefix, argv);
and by adopting the same approach for ls-files.c:
diff --git a/builtin/ls-files.c b/builtin/ls-files.c
index 0ca9d8e..0406adc 100644
--- a/builtin/ls-files.c
+++ b/builtin/ls-files.c
@@ -420,10 +420,11 @@ static int option_parse_z(const struct option *opt,
static int option_parse_exclude(const struct option *opt,
const char *arg, int unset)
{
- struct exclude_list_group *group = opt->value;
+ struct string_list *exclude_list = opt->value;
exc_given = 1;
- add_exclude(arg, "", 0, &group->el[0]);
+ string_list_append(exclude_list, arg);
+ fprintf(stderr, "append %s\n", arg);
return 0;
}
@@ -452,9 +453,11 @@ static int option_parse_exclude_standard(const struct option *opt,
int cmd_ls_files(int argc, const char **argv, const char *cmd_prefix)
{
- int require_work_tree = 0, show_tag = 0;
+ int require_work_tree = 0, show_tag = 0, i;
const char *max_prefix;
struct dir_struct dir;
+ struct exclude_list *el;
+ struct string_list exclude_list = STRING_LIST_INIT_NODUP;
struct option builtin_ls_files_options[] = {
{ OPTION_CALLBACK, 'z', NULL, NULL, NULL,
"paths are separated with NUL character",
@@ -489,7 +492,7 @@ int cmd_ls_files(int argc, const char **argv, const char *cmd_prefix)
OPT_BOOLEAN(0, "resolve-undo", &show_resolve_undo,
"show resolve-undo information"),
{ OPTION_CALLBACK, 'x', "exclude",
- &dir.exclude_list_group[EXC_CMDL], "pattern",
+ &exclude_list, "pattern",
"skip files matching pattern",
0, option_parse_exclude },
{ OPTION_CALLBACK, 'X', "exclude-from", &dir, "file",
@@ -524,9 +527,13 @@ int cmd_ls_files(int argc, const char **argv, const char *cmd_prefix)
if (read_cache() < 0)
die("index file corrupt");
- add_exclude_list(&dir, EXC_CMDL);
argc = parse_options(argc, argv, prefix, builtin_ls_files_options,
ls_files_usage, 0);
+ el = add_exclude_list(&dir, EXC_CMDL);
+ for (i = 0; i < exclude_list.nr; i++) {
+ fprintf(stderr, "adding exclude: %s\n", exclude_list.items[i].string);
+ add_exclude(exclude_list.items[i].string, "", 0, el);
+ }
if (show_tag || show_valid_bit) {
tag_cached = "H ";
tag_unmerged = "M ";
> The implementation of the machinery may find it more convenient if
> they can add one or more "sources" to an exclude_list_group before
> starting to add patterns to ->array[0] or ->array[1] or ->array[2],
> and a finer grained internal API that lets the caller pass an
> instance of "struct exclude_list" regardless of where in an
> exclude_list_group's ary[] that instance sits may be necessary to do
> so.
>
> But that does not mean other existing callers has to be aware of
> such inner detail. If the implementation of the machinery needs a
> helper function that adds an element to any struct exclude_list, not
> necessarily the one at the tip of an exclude_list_group, we can
> still do that by having the bulk of the logic in the internal, finer
> grained helper, say, add_pattern_to_exclude_list(), and keep the
> external API simpler by making it a thin wrapper around it, perhaps
> like:
>
> static void add_pattern_to_exclude_list(const char *pattern,
> const char *base, int baselen,
> struct exclude_list *el);
>
> void add_exclude(const char *pattern,
> const char *base, int baselen,
> struct exclude_list_group *group) {
> add_pattern_to_exclude_list(pattern, base, baselen, &group->ary[0]);
Presumably you mean
add_pattern_to_exclude_list(pattern, base, baselen,
&group->ary[group->nr - 1]);
(although at your request, I already renamed 'ary' to 'el').
I have made a genuine attempt to implement your suggestion, but due to
the unpack_trees() case stated above, I don't see how it can be done.
^ permalink raw reply related
* Re: Version 1.8.1 does not compile on Cygwin 1.7.14
From: Mark Levedahl @ 2013-01-06 22:16 UTC (permalink / raw)
To: Junio C Hamano
Cc: Jonathan Nieder, Torsten Bögershausen,
Stephen & Linda Smith, Jason Pyeron, git, Eric Blake
In-Reply-To: <7vfw2enl2l.fsf@alter.siamese.dyndns.org>
On 01/06/2013 02:54 PM, Junio C Hamano wrote:
> Jonathan Nieder <jrnieder@gmail.com> writes:
>
>> Mark Levedahl wrote:
>>
>>> However, the newer
>>> win32api is provided only for the current cygwin release series, which can
>>> be reliably identified by having dll version 1.7.x, while the older frozen
>>> releases (dll versions 1.6.x from redhat, 1.5.x open source) still have the
>>> older api as no updates are being made for the legacy version(s).
>> Ah. That makes sense, thanks.
>>
>> (For the future, if we wanted to diagnose an out-of-date win32api and
>> print a helpful message, I guess cygcheck would be the command to use.)
> Hmph, so we might see somebody who cares about Cygwin to come up
> with a solution based on cygcheck (not on uname) to update this
> part, perhaps on top of Peff's "split default settings based on
> uname into separate file" patch?
>
> If I understood what Mark and Torsten wrote correctly, you will have
> the new win32api if you install 1.7.17 (or newer) from scratch, but
> if you are on older 1.7.x then you can update the win32api part as a
> package update (as opposed to the whole-system upgrade). A test
> based on "uname -r" cannot notice that an older 1.7.x (say 1.7.14)
> installation has a newer win32api because the user updated it from
> the package (hence the user should not define CYGWIN_V15_WIN32API).
>
> Am I on the same page as you guys, or am I still behind?
>
> In the meantime, perhaps we would need something like this?
It's perhaps worth noting how we got into this mess. The problems have
their root in
adbc0b6b6e57c11ca49779d01f549260a920a97d
Cygwin's entire goal is a completely POSIX compliant environment running
under Windows. The above commit circumvents some of Cygwin's API
regarding stat/fstat to make things perhaps a bit faster, and definitely
not POSIX compliant (The commit message is wrong, the commit definitely
breaks POSIX compliance). That code is also what will not compile on
different w32api versions. It is curious: the Cygwin mailing list has
been absolutely silent since the w32api change was introduced last
summer, this is the only piece of code I am aware of that was broken by
the new headers, and of course the purpose of this code is to circumvent
the Cygwin API (and by extension, Cygwin project goals).
So, perhaps a better path forward is to disable / remove the above code
by default. (Those wanting a native Win32 git should just use the native
Win32 git).
Mark
^ permalink raw reply
* [PATCH v2] status: always report ignored tracked directories
From: Antoine Pelisse @ 2013-01-06 22:09 UTC (permalink / raw)
To: Jeff King; +Cc: Antoine Pelisse, tboegi, git
In-Reply-To: <20130105230303.GA5195@sigill.intra.peff.net>
Tracked directories (i.e. directories containing tracked files) that
are ignored must be reported as ignored if they contain untracked files.
Currently, files in the index can't be reported as ignored and are
automatically dropped from the list:
- When core.ignorecase is false, directories (which are not directly
tracked) are not listed as part of the index, and the directory can be
shown as ignored.
- When core.ignorecase is true on the other hand, directories are
reported as part of the index, and the directory is dropped, thus not
displayed as ignored.
Fix that behavior by allowing indexed file to be added when looking for
ignored files.
- Ignored tracked and untracked directories are treated the same way
when looking for ignored files, so we should not care about their index
status.
- Files are dismissed by treat_file() if they belong to the
index, so that step would have been a no-op anyway.
Signed-off-by: Antoine Pelisse <apelisse@gmail.com>
---
dir.c | 9 +++------
1 file changed, 3 insertions(+), 6 deletions(-)
diff --git a/dir.c b/dir.c
index 9b80348..f836590 100644
--- a/dir.c
+++ b/dir.c
@@ -672,7 +672,8 @@ static struct dir_entry *dir_entry_new(const char *pathname, int len)
static struct dir_entry *dir_add_name(struct dir_struct *dir, const char *pathname, int len)
{
- if (cache_name_exists(pathname, len, ignore_case))
+ if (!(dir->flags & DIR_SHOW_IGNORED) &&
+ cache_name_exists(pathname, len, ignore_case))
return NULL;
ALLOC_GROW(dir->entries, dir->nr+1, dir->alloc);
@@ -877,11 +878,7 @@ static int treat_file(struct dir_struct *dir, struct strbuf *path, int exclude,
if (exclude)
exclude_file = !(dir->flags & DIR_SHOW_IGNORED);
else if (dir->flags & DIR_SHOW_IGNORED) {
- /*
- * Optimization:
- * Don't spend time on indexed files, they won't be
- * added to the list anyway
- */
+ /* Always exclude indexed files */
struct cache_entry *ce = index_name_exists(&the_index,
path->buf, path->len, ignore_case);
--
1.7.12.4.3.g90f5e2d
^ permalink raw reply related
* Re: Version 1.8.1 does not compile on Cygwin 1.7.14
From: Mark Levedahl @ 2013-01-06 22:00 UTC (permalink / raw)
To: Junio C Hamano
Cc: Jonathan Nieder, Torsten Bögershausen,
Stephen & Linda Smith, Jason Pyeron, git, Eric Blake
In-Reply-To: <7vtxqum1u9.fsf@alter.siamese.dyndns.org>
On 01/06/2013 04:35 PM, Junio C Hamano wrote:
> Thanks; so perhaps you can give me an OK to forge your S-o-b to the
> following?
>
> -- >8 --
> From: Mark Levedahl <mlevedahl@gmail.com>
> Date: Sun, 6 Jan 2013 11:56:33 -0800
> Subject: [PATCH] Makefile: add comment on CYGWIN_V15_WIN32API
>
> There is no documented, reliable, and future-proof method to
> determine the installed w32api version on Cygwin. There are many
> things that can be done that will work frequently, except when they
> won't.
>
> The only sane thing is to follow the guidance of the Cygwin
> developers: the only supported configuration is that which the
> current setup.exe produces, and in the case of problems, if the
> installation is not up to date then updating is the first required
> action.
>
> Signed-off-by: Mark Levedahl <mlevedahl@gmail.com>
> ---
> Makefile | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/Makefile b/Makefile
> index 4d47af5..52e298a 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -273,6 +273,10 @@ all::
> #
> # Define NO_REGEX if you have no or inferior regex support in your C library.
> #
> +# Define CYGWIN_V15_WIN32API if you are using Cygwin v1.7.x but are not
> +# using the current w32api packages. The recommended approach, however,
> +# is to update your installation if compilation errors occur.
> +#
> # Define HAVE_DEV_TTY if your system can open /dev/tty to interact with the
> # user.
> #
Absolutely, that is ok by me.
Mark
^ permalink raw reply
* Re: [PATCH 2/4] t0024, t5000: use test_lazy_prereq for UNZIP
From: René Scharfe @ 2013-01-06 21:59 UTC (permalink / raw)
To: git discussion list, Junio C Hamano
In-Reply-To: <20130106180621.GA16494@ftbfs.org>
Am 06.01.2013 19:06, schrieb Matt Kraai:
> On Sun, Jan 06, 2013 at 06:49:00PM +0100, René Scharfe wrote:
>> This change makes the code smaller and we can put it at the top of
>> the script, its rightful place as setup code.
>
> Would it be better to add the setting of GIT_UNZIP and
> test_lazy_prereq to test-lib.sh so they aren't duplicated in both
> t0024-crlf-archive.sh and t5000-tar-tree.sh, something like the
> following (modulo UNZIP/GIT_UNZIP)?
We could do that in a follow-up patch, but I'm not sure it's worth it
for the two use cases.
René
^ permalink raw reply
* RE: Version 1.8.1 does not compile on Cygwin 1.7.14
From: Jason Pyeron @ 2013-01-06 21:46 UTC (permalink / raw)
To: git
In-Reply-To: <7vtxqum1u9.fsf@alter.siamese.dyndns.org>
> -----Original Message-----
> From: Junio C Hamano
> Sent: Sunday, January 06, 2013 16:36
>
> Thanks; so perhaps you can give me an OK to forge your S-o-b
> to the following?
I am personally fine with it, because cygwin is used by developers not
production systems and I expect my developers to upgrade their environment for
security fixes, etc.
If I ever had a situation where I am using git, in production, on cygwin, where
I could not upgrade I would effort to make a compile test based patch to the
make file to accommodate the issue.
>
> -- >8 --
> From: Mark Levedahl <mlevedahl@gmail.com>
> Date: Sun, 6 Jan 2013 11:56:33 -0800
> Subject: [PATCH] Makefile: add comment on CYGWIN_V15_WIN32API
>
> There is no documented, reliable, and future-proof method to
> determine the installed w32api version on Cygwin. There are
> many things that can be done that will work frequently,
> except when they won't.
>
> The only sane thing is to follow the guidance of the Cygwin
> developers: the only supported configuration is that which
> the current setup.exe produces, and in the case of problems,
> if the installation is not up to date then updating is the
> first required action.
>
> Signed-off-by: Mark Levedahl <mlevedahl@gmail.com>
> ---
> Makefile | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/Makefile b/Makefile
> index 4d47af5..52e298a 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -273,6 +273,10 @@ all::
> #
> # Define NO_REGEX if you have no or inferior regex support
> in your C library.
> #
> +# Define CYGWIN_V15_WIN32API if you are using Cygwin v1.7.x
> but are not
> +# using the current w32api packages. The recommended
> approach, however,
> +# is to update your installation if compilation errors occur.
> +#
> # Define HAVE_DEV_TTY if your system can open /dev/tty to
> interact with the # user.
> #
> --
> 1.8.1.302.g0f4eaa7
--
-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-
- -
- Jason Pyeron PD Inc. http://www.pdinc.us -
- Principal Consultant 10 West 24th Street #100 -
- +1 (443) 269-1555 x333 Baltimore, Maryland 21218 -
- -
-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-
This message is copyright PD Inc, subject to license 20080407P00.
^ permalink raw reply
* Re: [PATCH 00/21] "struct pathspec" conversion
From: Junio C Hamano @ 2013-01-06 21:45 UTC (permalink / raw)
To: Nguyễn Thái Ngọc Duy; +Cc: git
In-Reply-To: <1357453268-12543-1-git-send-email-pclouds@gmail.com>
Nguyễn Thái Ngọc Duy <pclouds@gmail.com> writes:
> This is another step towards the pathspec unification. This series
> introduces a get_pathspec() alternative: parse_pathspec(). The new
> function intializes struct pathspec directly. Many builtin commands
> (except mv) are converted to use this function. As a result, struct
> pathspec is used from the start for many commands.
>
> The next step would be dealing with pathspec manipulation code blocks
> that use "raw" field, init_pathspec or get_pathspec(). add.c, dir.c,
> rm.c and mv.c are hot places. And perhaps move pathspec code from
> dir.c and setup.c to pathspec.c after as/check-ignore enters "master".
>
> This series shares a patch (the first one) with nd/pathspec-wildcard. I
> put the patch in the series to avoid dependency.
>
> This series also disables wildcards in the prefix part, but it's only
> effective in combination with nd/pathspec-wildcard. And of course it's
> not fully effective until all "raw" use is eliminated.
Yay!
Thanks, looking forward to reading it through.
^ permalink raw reply
* Re: Version 1.8.1 does not compile on Cygwin 1.7.14
From: Junio C Hamano @ 2013-01-06 21:35 UTC (permalink / raw)
To: Mark Levedahl
Cc: Jonathan Nieder, Torsten Bögershausen,
Stephen & Linda Smith, Jason Pyeron, git, Eric Blake
In-Reply-To: <50E9E822.4020709@gmail.com>
Thanks; so perhaps you can give me an OK to forge your S-o-b to the
following?
-- >8 --
From: Mark Levedahl <mlevedahl@gmail.com>
Date: Sun, 6 Jan 2013 11:56:33 -0800
Subject: [PATCH] Makefile: add comment on CYGWIN_V15_WIN32API
There is no documented, reliable, and future-proof method to
determine the installed w32api version on Cygwin. There are many
things that can be done that will work frequently, except when they
won't.
The only sane thing is to follow the guidance of the Cygwin
developers: the only supported configuration is that which the
current setup.exe produces, and in the case of problems, if the
installation is not up to date then updating is the first required
action.
Signed-off-by: Mark Levedahl <mlevedahl@gmail.com>
---
Makefile | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/Makefile b/Makefile
index 4d47af5..52e298a 100644
--- a/Makefile
+++ b/Makefile
@@ -273,6 +273,10 @@ all::
#
# Define NO_REGEX if you have no or inferior regex support in your C library.
#
+# Define CYGWIN_V15_WIN32API if you are using Cygwin v1.7.x but are not
+# using the current w32api packages. The recommended approach, however,
+# is to update your installation if compilation errors occur.
+#
# Define HAVE_DEV_TTY if your system can open /dev/tty to interact with the
# user.
#
--
1.8.1.302.g0f4eaa7
^ permalink raw reply related
* Re: Version 1.8.1 does not compile on Cygwin 1.7.14
From: Mark Levedahl @ 2013-01-06 21:34 UTC (permalink / raw)
To: Torsten Bögershausen
Cc: Junio C Hamano, Jonathan Nieder, Stephen & Linda Smith,
Jason Pyeron, git, Eric Blake
In-Reply-To: <50E9E3C5.4070104@web.de>
On 01/06/2013 03:51 PM, Torsten Bögershausen wrote:
> Hm, I haven't understood the connection between the dll (cygwin1.dll
> ?) which is used in runtime, and the header files which are used when
> compiling. Are they updated at the same time when updating from 1.7.16
> to 1.7.17 ? Until I updated my cygwin 1.7 (following Marks
> recommendation) this did the trick for me: +ifeq ($(shell grep mingw
> /usr/include/w32api/winsock2.h />/dev/null 2>/dev/null && echo y),y) +
> CYGWIN_V15_WIN32API=YesPlease +endif As an alternative, would this be
> easier to read?
>> +# Define CYGWIN_V15_WIN32API for Cygwin versions up to 1.7.16
>
>
The cygwin distribution has a very large number of packages, each with
its own unique version number and update rhythm, just as in any linux
distro. There is no "cygwin version", just a version for each individual
package. So, "Cygwin version 1.7.16" is really nonsensical: there is
only cygwin.dll version 1.7.16. What folks are noticing is a
coincidence in the time when the cygwin dll package updated and when the
old w32api was obsoleted. uname -r reports the cygwin dll version, not
the version of any other package. Note that the cygwin api is "stable",
meaning a package compiled against the 1.7.1 dll will still run against
the current one: updating the cygwin dll does not require other packages
to update.
The only hard linkage here is that the Cygwin developers are maintaining
a legacy cygwin version (v1.5.x) as the newer dll series (v.1.7.x)
dropped support for all Windows versions predating (I think) WinXP. So,
someone on an old Windows version must use the legacy cygwin version
which has not been updated since the first v1.7 dll was released, nor
are there any plans by the developers to ever update the v1.5 packages.
Cygwin 1.5 lives in a separate distribution repository, with packages
frozen in time as of the last updates prior to going to v1.7 (packages
compiled against v1.7 will not run on v.1.5).
So, encountering a v1.5.x dll is a guarantee of using the older w32api
shared with the mingw project, rather than the current one now
maintained by the mingw64 project. However, a cygwin with any v1.7.x dll
could in theory have either w32api installed, or in theory yet another
newer one we don't know about yet. Unless and until the w32api
establishes a version number (independent of the Windows API version),
we have nothing reliable to use.
Therefore, if using the v1.7 series, *update*
Mark
^ permalink raw reply
* RE: Version 1.8.1 does not compile on Cygwin 1.7.14
From: Jason Pyeron @ 2013-01-06 21:33 UTC (permalink / raw)
To: git
In-Reply-To: <50E9E822.4020709@gmail.com>
> -----Original Message-----
> From: git-owner@vger.kernel.org
> [mailto:git-owner@vger.kernel.org] On Behalf Of Mark Levedahl
> Sent: Sunday, January 06, 2013 16:10
> To: Junio C Hamano
> Cc: Jonathan Nieder; Torsten Bögershausen; Stephen & Linda
> Smith; Jason Pyeron; git@vger.kernel.org; Eric Blake
> Subject: Re: Version 1.8.1 does not compile on Cygwin 1.7.14
>
> On 01/06/2013 02:54 PM, Junio C Hamano wrote:
> > Jonathan Nieder <jrnieder@gmail.com> writes:
> >
> >> Mark Levedahl wrote:
> >>
> >>>
> However,
> >>> the newer win32api is provided only for the current
> cygwin release
> >>> series, which can be reliably identified by having dll version
> >>> 1.7.x, while the older frozen releases (dll versions 1.6.x from
> >>> redhat, 1.5.x open source) still have the older api as no
> updates are being made for the legacy version(s).
> >> Ah. That makes sense, thanks.
> >>
> >> (For the future, if we wanted to diagnose an out-of-date
> win32api and
> >> print a helpful message, I guess cygcheck would be the command to
> >> use.)
> > Hmph, so we might see somebody who cares about Cygwin to
> come up with
> > a solution based on cygcheck (not on uname) to update this part,
> > perhaps on top of Peff's "split default settings based on
> uname into
> > separate file" patch?
> >
> > If I understood what Mark and Torsten wrote correctly, you
> will have
> > the new win32api if you install 1.7.17 (or newer) from
> scratch, but if
> > you are on older 1.7.x then you can update the win32api part as a
> > package update (as opposed to the whole-system upgrade). A
> test based
> > on "uname -r" cannot notice that an older 1.7.x (say 1.7.14)
> > installation has a newer win32api because the user updated
> it from the
> > package (hence the user should not define CYGWIN_V15_WIN32API).
> >
> > Am I on the same page as you guys, or am I still behind?
> >
> > In the meantime, perhaps we would need something like this?
> >
> >
> > diff --git a/Makefile b/Makefile
> > index 8e225ca..b45b06d 100644
> > --- a/Makefile
> > +++ b/Makefile
> > @@ -281,6 +281,9 @@ all::
> > #
> > # Define NO_REGEX if you have no or inferior regex
> support in your C library.
> > #
> > +# Define CYGWIN_V15_WIN32API if your Cygwin uses win32api
> dll older
> > +than # 1.7.x (this typically is true on Cygwin older than 1.7.17) #
> > # Define HAVE_DEV_TTY if your system can open /dev/tty to
> interact with the
> > # user.
> > #
> >
> Looking at a current setup.ini, the obsolete win32 api is a
> single package "w32api" with last version 3.17-2, and is now
> replaced by the new win32 api is in two packages,
> "w32api-headers" + "w32api-runtime", both at version
> 3.0b_svn5496-1. If setup.exe updated an older installation of
> w32api, the old package is not deleted, but replaced by a
> special "empty" package with (as of today) version 9999-1.
> Note that all of this could change at any time. Also, note
> that the new w32api packages have version numbers that are
> lower than the older obsoleted version.
I would not rely on that information as it is not designed to convey the
information the git build needs.
>
> Running "cygcheck -c w32api w32api-headers w32api-runtime" on
> one machine gives
>
> Cygwin Package Information
> Package Version Status
> w32api 9999-1 OK
> w32api-headers 3.0b_svn5496-1 OK
> w32api-runtime 3.0b_svn5496-1 OK
>
> So now, what do folks propose checking for?
> a) w32api is installed? Nope - the package is not "removed",
> it was updated to a special empty version to delete its
> former contents, but a new fresh installation won't have this.
> b) w32api-headers is installed? Nope - what happens on the
> next repackaging?
> c) w32api version is 9999-1? Maybe, but that number could change.
> etc.
This is what is typically done in a configure script by test compiling.
>
> There is no documented, reliable, future-proof, method of
> determining the installed w32api version on Cygwin. There are
> many things that can be done that will work frequently,
> except when they won't. I really think the only sane thing is
> to follow the guidance of the Cygwin
> developers: the only supported configuration is that which
> the current setup.exe produces, and in the case of problems,
> if the installation is not up to date then updating is the
> first required action.
>
> So, in the makefile, you might add:
>
> +# Define CYGWIN_V15_WIN32API if you are using Cygwin v1.7.x
> but are not
> +# using the current w32api packages. But, the recommended
> approach is
> +to # update your installation if compilation errors occur.
> +#
--
-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-
- -
- Jason Pyeron PD Inc. http://www.pdinc.us -
- Principal Consultant 10 West 24th Street #100 -
- +1 (443) 269-1555 x333 Baltimore, Maryland 21218 -
- -
-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-
This message is copyright PD Inc, subject to license 20080407P00.
^ permalink raw reply
* Re: Version 1.8.1 does not compile on Cygwin 1.7.14
From: Mark Levedahl @ 2013-01-06 21:09 UTC (permalink / raw)
To: Junio C Hamano
Cc: Jonathan Nieder, Torsten Bögershausen,
Stephen & Linda Smith, Jason Pyeron, git, Eric Blake
In-Reply-To: <7vfw2enl2l.fsf@alter.siamese.dyndns.org>
On 01/06/2013 02:54 PM, Junio C Hamano wrote:
> Jonathan Nieder <jrnieder@gmail.com> writes:
>
>> Mark Levedahl wrote:
>>
>>> However, the newer
>>> win32api is provided only for the current cygwin release series, which can
>>> be reliably identified by having dll version 1.7.x, while the older frozen
>>> releases (dll versions 1.6.x from redhat, 1.5.x open source) still have the
>>> older api as no updates are being made for the legacy version(s).
>> Ah. That makes sense, thanks.
>>
>> (For the future, if we wanted to diagnose an out-of-date win32api and
>> print a helpful message, I guess cygcheck would be the command to use.)
> Hmph, so we might see somebody who cares about Cygwin to come up
> with a solution based on cygcheck (not on uname) to update this
> part, perhaps on top of Peff's "split default settings based on
> uname into separate file" patch?
>
> If I understood what Mark and Torsten wrote correctly, you will have
> the new win32api if you install 1.7.17 (or newer) from scratch, but
> if you are on older 1.7.x then you can update the win32api part as a
> package update (as opposed to the whole-system upgrade). A test
> based on "uname -r" cannot notice that an older 1.7.x (say 1.7.14)
> installation has a newer win32api because the user updated it from
> the package (hence the user should not define CYGWIN_V15_WIN32API).
>
> Am I on the same page as you guys, or am I still behind?
>
> In the meantime, perhaps we would need something like this?
>
>
> diff --git a/Makefile b/Makefile
> index 8e225ca..b45b06d 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -281,6 +281,9 @@ all::
> #
> # Define NO_REGEX if you have no or inferior regex support in your C library.
> #
> +# Define CYGWIN_V15_WIN32API if your Cygwin uses win32api dll older than
> +# 1.7.x (this typically is true on Cygwin older than 1.7.17)
> +#
> # Define HAVE_DEV_TTY if your system can open /dev/tty to interact with the
> # user.
> #
>
Looking at a current setup.ini, the obsolete win32 api is a single
package "w32api" with last version 3.17-2, and is now replaced by the
new win32 api is in two packages, "w32api-headers" + "w32api-runtime",
both at version 3.0b_svn5496-1. If setup.exe updated an older
installation of w32api, the old package is not deleted, but replaced by
a special "empty" package with (as of today) version 9999-1. Note that
all of this could change at any time. Also, note that the new w32api
packages have version numbers that are lower than the older obsoleted
version.
Running "cygcheck -c w32api w32api-headers w32api-runtime" on one
machine gives
Cygwin Package Information
Package Version Status
w32api 9999-1 OK
w32api-headers 3.0b_svn5496-1 OK
w32api-runtime 3.0b_svn5496-1 OK
So now, what do folks propose checking for?
a) w32api is installed? Nope - the package is not "removed", it was
updated to a special empty version to delete its former contents, but a
new fresh installation won't have this.
b) w32api-headers is installed? Nope - what happens on the next repackaging?
c) w32api version is 9999-1? Maybe, but that number could change.
etc.
There is no documented, reliable, future-proof, method of determining
the installed w32api version on Cygwin. There are many things that can
be done that will work frequently, except when they won't. I really
think the only sane thing is to follow the guidance of the Cygwin
developers: the only supported configuration is that which the current
setup.exe produces, and in the case of problems, if the installation is
not up to date then updating is the first required action.
So, in the makefile, you might add:
+# Define CYGWIN_V15_WIN32API if you are using Cygwin v1.7.x but are not
+# using the current w32api packages. But, the recommended approach is to
+# update your installation if compilation errors occur.
+#
Mark
^ permalink raw reply
* Re: [PATCH] api-allocation-growing.txt: encourage better variable naming
From: Junio C Hamano @ 2013-01-06 20:58 UTC (permalink / raw)
To: Adam Spiers; +Cc: git list
In-Reply-To: <20130106205207.GA6552@pacific.linksys.moosehall>
Adam Spiers <git@adamspiers.org> writes:
>> Sounds good. To follow "not type but contents", a further rewrite
>> with s/array/item/ is even better, no?
>
> I agree.
Thanks for a quick response; let's do this then.
-- >8 --
From: Adam Spiers <git@adamspiers.org>
The documentation for the ALLOC_GROW API implicitly encouraged
developers to use "ary" as the variable name for the array which is
dynamically grown. However "ary" is an unusual abbreviation hardly
used anywhere else in the source tree, and it is also better to name
variables based on their contents not on their type.
Signed-off-by: Adam Spiers <git@adamspiers.org>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
Documentation/technical/api-allocation-growing.txt | 14 ++++++++------
1 file changed, 8 insertions(+), 6 deletions(-)
diff --git a/Documentation/technical/api-allocation-growing.txt b/Documentation/technical/api-allocation-growing.txt
index 43dbe09..542946b 100644
--- a/Documentation/technical/api-allocation-growing.txt
+++ b/Documentation/technical/api-allocation-growing.txt
@@ -5,7 +5,9 @@ Dynamically growing an array using realloc() is error prone and boring.
Define your array with:
-* a pointer (`ary`) that points at the array, initialized to `NULL`;
+* a pointer (`item`) that points at the array, initialized to `NULL`
+ (although please name the variable based on its contents, not on its
+ type);
* an integer variable (`alloc`) that keeps track of how big the current
allocation is, initialized to `0`;
@@ -13,22 +15,22 @@ Define your array with:
* another integer variable (`nr`) to keep track of how many elements the
array currently has, initialized to `0`.
-Then before adding `n`th element to the array, call `ALLOC_GROW(ary, n,
+Then before adding `n`th element to the item, call `ALLOC_GROW(item, n,
alloc)`. This ensures that the array can hold at least `n` elements by
calling `realloc(3)` and adjusting `alloc` variable.
------------
-sometype *ary;
+sometype *item;
size_t nr;
size_t alloc
for (i = 0; i < nr; i++)
- if (we like ary[i] already)
+ if (we like item[i] already)
return;
/* we did not like any existing one, so add one */
-ALLOC_GROW(ary, nr + 1, alloc);
-ary[nr++] = value you like;
+ALLOC_GROW(item, nr + 1, alloc);
+item[nr++] = value you like;
------------
You are responsible for updating the `nr` variable.
--
1.8.1.302.g0f4eaa7
^ permalink raw reply related
* Re: [PATCH] api-allocation-growing.txt: encourage better variable naming
From: Adam Spiers @ 2013-01-06 20:52 UTC (permalink / raw)
To: git list
In-Reply-To: <7v38yenjgy.fsf@alter.siamese.dyndns.org>
On Sun, Jan 06, 2013 at 12:29:33PM -0800, Junio C Hamano wrote:
> Adam Spiers <git@adamspiers.org> writes:
>
> > The documentation for the ALLOC_GROW API implicitly encouraged
> > developers to use "ary" as the variable name for the array which is
> > dynamically grown. However "ary" is an unusual abbreviation hardly
> > used anywhere else in the source tree, and it is also better to name
> > variables based on their contents not on their type.
>
> Sounds good. To follow "not type but contents", a further rewrite
> with s/array/item/ is even better, no?
>
> I can obviously squash it in without resending, if you agree, or you
> can point out why item[] is not a good idea and array[] is better.
I agree.
^ permalink raw reply
* Re: Version 1.8.1 does not compile on Cygwin 1.7.14
From: Torsten Bögershausen @ 2013-01-06 20:51 UTC (permalink / raw)
To: Junio C Hamano
Cc: Mark Levedahl, Jonathan Nieder, Torsten Bögershausen,
Stephen & Linda Smith, Jason Pyeron, git, Eric Blake
In-Reply-To: <7vfw2enl2l.fsf@alter.siamese.dyndns.org>
On 06.01.13 20:54, Junio C Hamano wrote:
> Jonathan Nieder <jrnieder@gmail.com> writes:
>
>> Mark Levedahl wrote:
>>
>>> However, the newer
>>> win32api is provided only for the current cygwin release series, which can
>>> be reliably identified by having dll version 1.7.x, while the older frozen
>>> releases (dll versions 1.6.x from redhat, 1.5.x open source) still have the
>>> older api as no updates are being made for the legacy version(s).
>>
>> Ah. That makes sense, thanks.
>>
>> (For the future, if we wanted to diagnose an out-of-date win32api and
>> print a helpful message, I guess cygcheck would be the command to use.)
>
> Hmph, so we might see somebody who cares about Cygwin to come up
> with a solution based on cygcheck (not on uname) to update this
> part, perhaps on top of Peff's "split default settings based on
> uname into separate file" patch?
>
> If I understood what Mark and Torsten wrote correctly, you will have
> the new win32api if you install 1.7.17 (or newer) from scratch, but
> if you are on older 1.7.x then you can update the win32api part as a
> package update (as opposed to the whole-system upgrade). A test
> based on "uname -r" cannot notice that an older 1.7.x (say 1.7.14)
> installation has a newer win32api because the user updated it from
> the package (hence the user should not define CYGWIN_V15_WIN32API).
>
> Am I on the same page as you guys, or am I still behind?
>
> In the meantime, perhaps we would need something like this?
>
>
> diff --git a/Makefile b/Makefile
> index 8e225ca..b45b06d 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -281,6 +281,9 @@ all::
> #
> # Define NO_REGEX if you have no or inferior regex support in your C library.
> #
> +# Define CYGWIN_V15_WIN32API if your Cygwin uses win32api dll older than
> +# 1.7.x (this typically is true on Cygwin older than 1.7.17)
> +#
> # Define HAVE_DEV_TTY if your system can open /dev/tty to interact with the
> # user.
> #
Hm, I haven't understood the connection between the dll (cygwin1.dll ?)
which is used in runtime, and the header files which are used when compiling.
Are they updated at the same time when updating from 1.7.16 to 1.7.17 ?
Until I updated my cygwin 1.7 (following Marks recommendation) this did the trick for me:
+ifeq ($(shell grep mingw /usr/include/w32api/winsock2.h />/dev/null 2>/dev/null && echo y),y)
+ CYGWIN_V15_WIN32API=YesPlease
+endif
As an alternative, would this be easier to read?
> +# Define CYGWIN_V15_WIN32API for Cygwin versions up to 1.7.16
^ permalink raw reply
* Re: Re: [PATCH] Remove the suggestion to use parsecvs, which is currently broken.
From: Eric S. Raymond @ 2013-01-06 20:32 UTC (permalink / raw)
To: Heiko Voigt; +Cc: git
In-Reply-To: <20130106163420.GA3378@book-mint>
Heiko Voigt <hvoigt@hvoigt.net>:
> > I'm parsecvs's maintainer now. It's not in good shape; there is at
> > least one other known showstopper besides the build issue. I would
> > strongly prefer to direct peoples' attention away from it until I
> > have time to fix it and cut a release. This is not a distant
> > prospect - two or three weeks out, maybe.
>
> So for this short amount of time you want to change gits documentation?
Yes. We should not direct people to a tool that plain doesn't work.
I'll fix parsecvs as soon as I can. Once I do, I will add support to the
new git-cvsimport to use parsecvs as a conversion engine, alongside
cvsps and cvs2git.
You may not have seen the first version of that patch, so I'll
explain. The new git-cvsimport can use multiple conversion engines;
each one is expressed as a Python class that knows how to convert
git-cvsimport options to engine options, and how to generate a
command that ships an import stream to standard output. There's
an -e option that selects an engine.
Currently there are two such classes, one for cvsps and one for cvs2git.
cvsps is the default. When parsecvs is working, it will be the work of
a few minutes to add a parsecvs class.
The architectural goal here is to make it easy for users of
git-cvsimport to be able to experiment with different engines to
get the best possible conversion, without having to fuss with
details of the engine invocation.
> Is this hint causing you trouble? Are there many people asking for
> support because of that?
No. But as a matter of principle I am against having documentation
tell pretty lies, even temporarily. It's bad craftsmanship and bad
faith to do that.
> There is no README so I am not sure how the tests are supposed to be
> build in general. Due to the lack of documentation its probably easier
> for you Eric to port my tests.
At the present state of things, I agree. I have been so busy fighting other
aspects of this problem that I have not yet had time to separate the
test suite from the cvsps code and document it properly.
> The structure of my tests is quite simple:
>
> t/ - All the tests
> t/cvsroot - A cvs module per test
> t/t[0-9]{4}*/expect - The expected cvsps output
>
> You can copy the cvs repository modules and convert the expected cvsps
> output to whatever output you want to test against. It the found
> changeset ordering that is interesting.
Noted. I have a copy and will port them.
> The fix was never clean and AFAIR the reason behind that was that the
> breakage in commit ordering is not easy to fix in cvsps.
Understood. But it's better than no fix at all.
> That and
> because there are other working tools out there was the reason why I
> stopped working on fixing cvsps.
Once I have all three tools working and can run them against a common
test suite, several interesting possibilities will open up.
--
<a href="http://www.catb.org/~esr/">Eric S. Raymond</a>
^ permalink raw reply
* Re: [PATCH] api-allocation-growing.txt: encourage better variable naming
From: Junio C Hamano @ 2013-01-06 20:29 UTC (permalink / raw)
To: Adam Spiers; +Cc: git list
In-Reply-To: <1357486505-21357-1-git-send-email-git@adamspiers.org>
Adam Spiers <git@adamspiers.org> writes:
> The documentation for the ALLOC_GROW API implicitly encouraged
> developers to use "ary" as the variable name for the array which is
> dynamically grown. However "ary" is an unusual abbreviation hardly
> used anywhere else in the source tree, and it is also better to name
> variables based on their contents not on their type.
Sounds good. To follow "not type but contents", a further rewrite
with s/array/item/ is even better, no?
I can obviously squash it in without resending, if you agree, or you
can point out why item[] is not a good idea and array[] is better.
>
> Signed-off-by: Adam Spiers <git@adamspiers.org>
> ---
> Documentation/technical/api-allocation-growing.txt | 14 ++++++++------
> 1 file changed, 8 insertions(+), 6 deletions(-)
>
> diff --git a/Documentation/technical/api-allocation-growing.txt b/Documentation/technical/api-allocation-growing.txt
> index 43dbe09..3894815 100644
> --- a/Documentation/technical/api-allocation-growing.txt
> +++ b/Documentation/technical/api-allocation-growing.txt
> @@ -5,7 +5,9 @@ Dynamically growing an array using realloc() is error prone and boring.
>
> Define your array with:
>
> -* a pointer (`ary`) that points at the array, initialized to `NULL`;
> +* a pointer (`array`) that points at the array, initialized to `NULL`
> + (although please name the variable based on its contents, not on its
> + type);
>
> * an integer variable (`alloc`) that keeps track of how big the current
> allocation is, initialized to `0`;
> @@ -13,22 +15,22 @@ Define your array with:
> * another integer variable (`nr`) to keep track of how many elements the
> array currently has, initialized to `0`.
>
> -Then before adding `n`th element to the array, call `ALLOC_GROW(ary, n,
> +Then before adding `n`th element to the array, call `ALLOC_GROW(array, n,
> alloc)`. This ensures that the array can hold at least `n` elements by
> calling `realloc(3)` and adjusting `alloc` variable.
>
> ------------
> -sometype *ary;
> +sometype *array;
> size_t nr;
> size_t alloc
>
> for (i = 0; i < nr; i++)
> - if (we like ary[i] already)
> + if (we like array[i] already)
> return;
>
> /* we did not like any existing one, so add one */
> -ALLOC_GROW(ary, nr + 1, alloc);
> -ary[nr++] = value you like;
> +ALLOC_GROW(array, nr + 1, alloc);
> +array[nr++] = value you like;
> ------------
>
> You are responsible for updating the `nr` variable.
^ permalink raw reply
* Re: [PATCH v3 11/19] dir.c: use a single struct exclude_list per source of excludes
From: Junio C Hamano @ 2013-01-06 20:25 UTC (permalink / raw)
To: Adam Spiers; +Cc: git list
In-Reply-To: <20130106152039.GA2396@pacific.linksys.moosehall>
Adam Spiers <git@adamspiers.org> writes:
> On Fri, Jan 04, 2013 at 01:03:59PM -0800, Junio C Hamano wrote:
>> Adam Spiers <git@adamspiers.org> writes:
>>
>> > diff --git a/builtin/clean.c b/builtin/clean.c
>> > index 0c7b3d0..bd18b88 100644
>> > --- a/builtin/clean.c
>> > +++ b/builtin/clean.c
>> > @@ -97,9 +97,10 @@ int cmd_clean(int argc, const char **argv, const char *prefix)
>> > if (!ignored)
>> > setup_standard_excludes(&dir);
>> >
>> > + add_exclude_list(&dir, EXC_CMDL);
>> > for (i = 0; i < exclude_list.nr; i++)
>> > add_exclude(exclude_list.items[i].string, "", 0,
>> > - &dir.exclude_list[EXC_CMDL]);
>> > + &dir.exclude_list_groups[EXC_CMDL].ary[0]);
>>
>> This looks somewhat ugly for two reasons.
>>
>> * The abstraction add_exclude() offers to its callers is just to
>> let them add one pattern to the list of patterns for the kind
>> (here, EXC_CMDL); why should they care about .ary[0] part?
>
> Because the caller has to decide which exclude list the new exclude
> should be added to; that is how it has been for a long time, and I am
> not proposing we change that.
Unless I was mistaken, I never objected to the EXC_CMDL, etc
appearing in the text of the calling site of add_exclude().
The objection was about the .ary[0] bit. From the point of view of
a caller of the API, it:
- calls add_exclude_list() to declare "I now start adding new
patterns that come from a new source of patterns"; then
- calls add_exclude() repeatedly to add the patterns that come
from that source.
no? Why does the latter has to keep repeating "Here is the new
pattern for the EXC_CMDL group; it comes from the latest source I
earlier declared, by the way", instead of just "Here is the new
pattern for the EXC_CMDL group"? The ary[0] part always using "0"
(not "4" or "ix") is what repeats that "by the way".
>> Are
>> there cases any sane caller (not the implementation of the
>> exclude_list_group machinery e.g. add_excludes_from_... function)
>> may want to call it with .ary[1]?
>
> Effectively yes, although it is not written like ".ary[1]". For
> example prep_exclude() calls add_excludes_from_file_to_list() for each
> new .gitignore file
That is part of the "implementation of the machinery". If the API
for the outside callers are to call add_exclude_list() to declare
that patterns added by subsequent calls to add_exclude() are from
one new source of the patterns (e.g. .gitignore file in a new
directory level), and then call add_exclude() to add each pattern,
then the callers to add_exclude() shouldn't have to care about the
implementation detail that individual sources in exclude_list_group
is implemented as an array in that sructure, and the latest ones
should go to its ->array[0].
The implementation of the machinery may find it more convenient if
they can add one or more "sources" to an exclude_list_group before
starting to add patterns to ->array[0] or ->array[1] or ->array[2],
and a finer grained internal API that lets the caller pass an
instance of "struct exclude_list" regardless of where in an
exclude_list_group's ary[] that instance sits may be necessary to do
so.
But that does not mean other existing callers has to be aware of
such inner detail. If the implementation of the machinery needs a
helper function that adds an element to any struct exclude_list, not
necessarily the one at the tip of an exclude_list_group, we can
still do that by having the bulk of the logic in the internal, finer
grained helper, say, add_pattern_to_exclude_list(), and keep the
external API simpler by making it a thin wrapper around it, perhaps
like:
static void add_pattern_to_exclude_list(const char *pattern,
const char *base, int baselen,
struct exclude_list *el);
void add_exclude(const char *pattern,
const char *base, int baselen,
struct exclude_list_group *group) {
add_pattern_to_exclude_list(pattern, base, baselen, &group->ary[0]);
}
no?
^ permalink raw reply
* Re: [PATCH jk/pathspec-literal] t6130-pathspec-noglob: Windows does not allow a file named "f*"
From: Junio C Hamano @ 2013-01-06 20:00 UTC (permalink / raw)
To: Jeff King; +Cc: Johannes Sixt, Git Mailing List, msysGit
In-Reply-To: <20130106143319.GA10690@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
> On Sun, Jan 06, 2013 at 03:07:43PM +0100, Johannes Sixt wrote:
>
>> Windows disallows file names that contain a star. Arrange the test setup
>> to insert the file name "f*" in the repository without the corresponding
>> file in the worktree.
>> [...]
>> - test_commit star "f*" &&
>> + # insert file "f*" in the commit, but in a way that avoids
>> + # the name "f*" in the worktree, because it is not allowed
>> + # on Windows (the tests below do not depend on the presence
>> + # of the file in the worktree)
>> + git update-index --add --cacheinfo 100644 "$(git rev-parse HEAD:foo)" "f*" &&
>> + test_tick &&
>> + git commit -m star &&
>
> Thanks, looks obviously correct to me.
>
> Sorry to break Windows again. It seems I learn about a new gotcha with
> every patch series. :)
Thanks, both.
It appears that pushing things earlier to 'next' (and unfortunately
even to 'master') rather than later seems to be the only way to help
catching little gotchas like this ;-).
--
*** Please reply-to-all at all times ***
*** (do not pretend to know who is subscribed and who is not) ***
*** Please avoid top-posting. ***
The msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.
You received this message because you are subscribed to the Google
Groups "msysGit" group.
To post to this group, send email to msysgit@googlegroups.com
To unsubscribe from this group, send email to
msysgit+unsubscribe@googlegroups.com
For more options, and view previous threads, visit this group at
http://groups.google.com/group/msysgit?hl=en_US?hl=en
^ permalink raw reply
* Re: Version 1.8.1 does not compile on Cygwin 1.7.14
From: Junio C Hamano @ 2013-01-06 19:54 UTC (permalink / raw)
To: Mark Levedahl
Cc: Jonathan Nieder, Torsten Bögershausen,
Stephen & Linda Smith, Jason Pyeron, git, Eric Blake
In-Reply-To: <20130106120917.GC22081@elie.Belkin>
Jonathan Nieder <jrnieder@gmail.com> writes:
> Mark Levedahl wrote:
>
>> However, the newer
>> win32api is provided only for the current cygwin release series, which can
>> be reliably identified by having dll version 1.7.x, while the older frozen
>> releases (dll versions 1.6.x from redhat, 1.5.x open source) still have the
>> older api as no updates are being made for the legacy version(s).
>
> Ah. That makes sense, thanks.
>
> (For the future, if we wanted to diagnose an out-of-date win32api and
> print a helpful message, I guess cygcheck would be the command to use.)
Hmph, so we might see somebody who cares about Cygwin to come up
with a solution based on cygcheck (not on uname) to update this
part, perhaps on top of Peff's "split default settings based on
uname into separate file" patch?
If I understood what Mark and Torsten wrote correctly, you will have
the new win32api if you install 1.7.17 (or newer) from scratch, but
if you are on older 1.7.x then you can update the win32api part as a
package update (as opposed to the whole-system upgrade). A test
based on "uname -r" cannot notice that an older 1.7.x (say 1.7.14)
installation has a newer win32api because the user updated it from
the package (hence the user should not define CYGWIN_V15_WIN32API).
Am I on the same page as you guys, or am I still behind?
In the meantime, perhaps we would need something like this?
diff --git a/Makefile b/Makefile
index 8e225ca..b45b06d 100644
--- a/Makefile
+++ b/Makefile
@@ -281,6 +281,9 @@ all::
#
# Define NO_REGEX if you have no or inferior regex support in your C library.
#
+# Define CYGWIN_V15_WIN32API if your Cygwin uses win32api dll older than
+# 1.7.x (this typically is true on Cygwin older than 1.7.17)
+#
# Define HAVE_DEV_TTY if your system can open /dev/tty to interact with the
# user.
#
^ permalink raw reply related
* Re: [PATCH v4] git-completion.bash: add support for path completion
From: Manlio Perillo @ 2013-01-06 18:39 UTC (permalink / raw)
To: Marc Khouzam
Cc: Junio C Hamano, git@vger.kernel.org, szeder@ira.uka.de,
felipe.contreras@gmail.com
In-Reply-To: <E59706EF8DB1D147B15BECA3322E4BDC0681FA@eusaamb103.ericsson.se>
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
Il 05/01/2013 21:23, Marc Khouzam ha scritto:
> [...]
> Thanks for this, it improves the situation dramatically.
> I did further testing with your patch and found some less obvious
> issues. I didn't debug the script myself as I'm not that familiar with
> it either, but I think the testcases below should help Manlio or
> someone else look into some regressions.
>
> 1- Using .. or . breaks completion when after the '/':
> [...]
> 2- Maybe related to problem 1. Using .. breaks completion in other ways:
> [...]
> 3- Also probably related to problems 1 and 2. Using absolute paths behaves wierdly and
> worse than before:
> In my opinion, the above three cases are regressions.
>
Yes.
I did not considered this use case, thanks!
I have never done something like this when working with Mercurial.
The problem is caused by the __git_index_file_list_filter function.
The job of this function is to stop path completion at directory
boundary (in order to avoid to suggest files in child
directories [1]), and to make paths relative to current directory.
Unfortunately, what it does is to simply remove the prefix string from
the path name; of course this will not work when the prefix is a non
canonical path name.
The solution is quite simple: canonicalize both the prefix path and each
of the path name returned by git.
This can be done using `readlink -f "$path"` or `realpath $path`, but
the problem is that it is inefficient to execute an external command for
each of the path returned by git; moreover readlink and realpath are not
POSIX and may not be supported on all platforms where git works (but I
found a portable implementation using pushd, popd, `pwd`, `dirname`,
`basename` -- not very efficient).
IMHO, the best solution is to recode __git_index_file_list_filter in Perl.
Another possible solution (as suggested by Junio) is to use the
- --relative option; unfortunately this is only supported by
`git diff-index` and not by `git ls-files`.
And it will not solve the problem when using absolute path names (but
this case can be handled by leaving path completion to bash).
> 4- Completion choices include their entire path, which is not what bash does by default. For example:
>> cd git/contrib
>> ls completion/git-<tab>
> git-completion.bash git-completion.tcsh git-completion.zsh git-prompt.sh
> but
>> git rm completion/git-<tab>
> completion/git-completion.bash completion/git-completion.tcsh completion/git-completion.zsh completion/git-prompt.sh
> notice the extra 'completion/' before each completion.
This is another thing I missed.
The problem is that only the current directory is removed from the path
names returned by git.
> This can get pretty large when completing with
> many directory prefixes. The current tcsh completion has the same problem which I couldn't fix. However, I am
> not sure if it can be fixed for bash.
>
The fix was very easy, and it seems to work.
The problem is in the __git_complete_index_files and
__git_complete_diff_index_files function.
When calling the __git_index_files and git_index_files, the "$cur"
variable should be used, instead of the computed "$pfx".
Not sure if this is correct.
I will post the patch, so you can test it.
> I personally don't think this is regression, just an slight annoyance.
>
> 5- With this feature git-completion.bash will return directories as completions. This is something
> git-completion.tcsh is not handling very well. I will post a patch to fix that.
>
I'll pass on this, thanks.
> Below are two suggestions that are in line with this effort but that are not regressions.
>
> A) It would be nice if
> git commit -a <TAB>
> also completed with untracked files
>
I agree.
And there are other places when it may be useful to check the passed
options (see the comments).
But I think it is better to leave these issues for the future.
I will just add a comment to take note of this use case.
> B) Now that, for example, 'git rm' completion is smart about path completion
> it would be nice to somehow not trigger bash default file completion
> when 'git rm' does not report any completions.
>
Not sure how this can be done, but it is possible and should be easy.
> For example, if I have a file called zz.tar.gz (which is an ignored file)
> and I do 'git rm <tab>', I will get the proper list of files that can be
> removed by git, excluding zz.tar.gz. But if I complete
> 'git rm zz.tar.<tab>' then the completion script will return nothing,
> since git cannot remove that ignored file, but we will then fall-back
> to the bash default completion, which will complete the file to zz.tar.gz.
>
> Although there are some issues, I think this feature will greatly benefit the user
> and is worth the time needed to fix.
>
> Thanks!
>
> Marc
Thanks to you for the review!
Regards Manlio
[1] this is what the Mercurial bash completion script does
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.10 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/
iEYEARECAAYFAlDpxNEACgkQscQJ24LbaURZEgCcD2Uc+7/W+RCrMk3j+vrd5w36
6ogAn1ou4pOarBSMywaQ3zQKdZmofyKA
=iU13
-----END PGP SIGNATURE-----
^ permalink raw reply
* Re: [PATCH 2/4] t0024, t5000: use test_lazy_prereq for UNZIP
From: Matt Kraai @ 2013-01-06 18:06 UTC (permalink / raw)
To: René Scharfe; +Cc: git discussion list, Junio C Hamano
In-Reply-To: <50E9B90C.2060200@lsrfire.ath.cx>
On Sun, Jan 06, 2013 at 06:49:00PM +0100, René Scharfe wrote:
> This change makes the code smaller and we can put it at the top of
> the script, its rightful place as setup code.
Would it be better to add the setting of GIT_UNZIP and
test_lazy_prereq to test-lib.sh so they aren't duplicated in both
t0024-crlf-archive.sh and t5000-tar-tree.sh, something like the
following (modulo UNZIP/GIT_UNZIP)?
--
Matt Kraai
https://ftbfs.org/kraai
diff --git a/t/t0024-crlf-archive.sh b/t/t0024-crlf-archive.sh
index ec6c1b3..084f33c 100755
--- a/t/t0024-crlf-archive.sh
+++ b/t/t0024-crlf-archive.sh
@@ -3,7 +3,6 @@
test_description='respect crlf in git archive'
. ./test-lib.sh
-UNZIP=${UNZIP:-unzip}
test_expect_success setup '
@@ -26,13 +25,6 @@ test_expect_success 'tar archive' '
'
-"$UNZIP" -v >/dev/null 2>&1
-if [ $? -eq 127 ]; then
- say "Skipping ZIP test, because unzip was not found"
-else
- test_set_prereq UNZIP
-fi
-
test_expect_success UNZIP 'zip archive' '
git archive --format=zip HEAD >test.zip &&
diff --git a/t/t5000-tar-tree.sh b/t/t5000-tar-tree.sh
index ecf00ed..85b64ae 100755
--- a/t/t5000-tar-tree.sh
+++ b/t/t5000-tar-tree.sh
@@ -25,7 +25,6 @@ commit id embedding:
'
. ./test-lib.sh
-UNZIP=${UNZIP:-unzip}
GZIP=${GZIP:-gzip}
GUNZIP=${GUNZIP:-gzip -d}
@@ -201,13 +200,6 @@ test_expect_success \
test_cmp a/substfile2 g/prefix/a/substfile2
'
-$UNZIP -v >/dev/null 2>&1
-if [ $? -eq 127 ]; then
- say "Skipping ZIP tests, because unzip was not found"
-else
- test_set_prereq UNZIP
-fi
-
test_expect_success \
'git archive --format=zip' \
'git archive --format=zip HEAD >d.zip'
diff --git a/t/test-lib.sh b/t/test-lib.sh
index 8a12cbb..4ceabad 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -752,6 +752,13 @@ test_lazy_prereq AUTOIDENT '
git var GIT_AUTHOR_IDENT
'
+UNZIP=${UNZIP:-unzip}
+
+test_lazy_prereq UNZIP '
+ "$UNZIP" -v >/dev/null 2>&1
+ test $? -ne 127
+'
+
# When the tests are run as root, permission tests will report that
# things are writable when they shouldn't be.
test -w / || test_set_prereq SANITY
^ permalink raw reply related
* Re: [PATCH v4] git-completion.bash: add support for path completion
From: Manlio Perillo @ 2013-01-06 18:00 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Marc Khouzam, git, szeder, felipe.contreras
In-Reply-To: <7vehi0qh4x.fsf@alter.siamese.dyndns.org>
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
Il 05/01/2013 07:27, Junio C Hamano ha scritto:
> Junio C Hamano <gitster@pobox.com> writes:
>
>> Marc Khouzam <marc.khouzam@ericsson.com> writes:
>>
>>> I've been playing with it but I'm not getting the expected
>>> behavior when I cd to a sub-directory.
>>
>> Thanks for testing. Manlio?
>
> Can you try the attached patch?
>
Thanks, it seems to fix the problem.
> As I am not familiar with the completion machinery, take this with a
> large grain of salt. Here is my explanation of what is going on in
> this "how about this" fixup:
>
> * Giving --git-dir from the command line (or GIT_DIR environment)
> without specifying GIT_WORK_TREE is to signal Git that you are at
> the top of the working tree. "git ls-files" will then show the
> full tree even outside the real $cwd because you are lying to
> Git.
>
I was not aware of this, and blindly copied the code from the other
existing functions.
However the other completion functions never have to deal with paths in
the working directory.
I have applied the patch to my local branch.
> [...]
Regards Manlio
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.10 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/
iEYEARECAAYFAlDpu7UACgkQscQJ24LbaUSmUACgl+OKUyvpp183kFZGmBpOfqm1
yqEAnjxcqmZYvWSeIpOo6cNFl/dnMH76
=oE/+
-----END PGP SIGNATURE-----
^ 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