* Re: [PATCH] Handle UNC paths everywhere
From: Erik Faye-Lund @ 2010-01-25 17:57 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: Robin Rosenberg, git, Johannes Sixt, Junio C Hamano
In-Reply-To: <alpine.DEB.1.00.1001251553150.8733@intel-tinevez-2-302>
On Mon, Jan 25, 2010 at 6:34 PM, Johannes Schindelin
<Johannes.Schindelin@gmx.de> wrote:
> Hi,
>
> On Mon, 25 Jan 2010, Robin Rosenberg wrote:
>
>> >From 37a74ccd395d91e5662665ca49d7f4ec49811de0 Mon Sep 17 00:00:00 2001
>> From: Robin Rosenberg <robin.rosenberg@dewire.com>
>> Date: Mon, 25 Jan 2010 01:41:03 +0100
>> Subject: [PATCH] Handle UNC paths everywhere
>>
>> In Windows paths beginning with // are knows as UNC paths. They are
>> absolute paths, usually referring to a shared resource on a server.
>
> And even a simple "cd" with them does not work.
>
But it does, at least for me - both in bash and cmd.exe. I just need
to log on to the server first, unless it's a public share.
--
Erik "kusma" Faye-Lund
^ permalink raw reply
* [RFC/H] "git diff --submodule" showing submodule work tree dirtiness
From: Junio C Hamano @ 2010-01-25 18:05 UTC (permalink / raw)
To: Jens Lehmann; +Cc: git
In-Reply-To: <4B5C547C.7000604@web.de>
Jens Lehmann <Jens.Lehmann@web.de> writes:
> Right now "git diff --submodule" doesn't show the dirty status of a
> submodule at all (like it does when using it without that option and
> having paid the cost to get the necessary information). So IMHO something
> like the patch below should go into 1.7.0 to fix that. When applied the
> output looks like this:
>
> Submodule sub 3f35670..3f35670-dirty:
>
> which is now consistent with the output of "git diff" without that option:
>
> diff --git a/sub b/sub
> --- a/sub
> +++ b/sub
> @@ -1 +1 @@
> -Subproject commit 3f356705649b5d566d97ff843cf193359229a453
> +Subproject commit 3f356705649b5d566d97ff843cf193359229a453-dirty
I think this is sensible; I queued it to 'pu' to give chance to submodule
users comment on it, but I am inclined to say it should be part of 1.7.0
for consistency.
^ permalink raw reply
* Re: git notes: notes
From: John Koleszar @ 2010-01-25 18:08 UTC (permalink / raw)
To: Johan Herland; +Cc: Joey Hess, git@vger.kernel.org, Johannes.Schindelin@gmx.de
In-Reply-To: <201001210305.05309.johan@herland.net>
On Wed, 2010-01-20 at 21:05 -0500, Johan Herland wrote:
> On Wednesday 20 January 2010, Joey Hess wrote:
> > Johan Herland wrote:
> > > > PS, Has anyone thought about using notes to warn bisect away from
> > > > commits that are known to be unbuildable or otherwise cause bisection
> > > > trouble?
> > >
> > > No, I haven't thought of that specific use case. Great idea! :)
> >
[...]
>
> In any case, I would not use "git notes" to maintain the bisect hints.
> Rather, I'd add subcommands to "git bisect" that would take care of
> maintaining the notes tree @ "refs/notes/bisect". Much more user-friendly
> than telling the user to write their own bisect-notes by hand.
>
I haven't read up on notes more than enough to know its in the pipe, but
I had a similar idea for using them to store bisect hints. I've been
doing a lot of bisecting lately into a range that had a couple dormant
bugs where I'm trying to bisect bug B but bug A prevents me from making
a determination. Rather than skip what I know is an interesting commit,
I cherry-pick the bugfix commit(s) A' and test that, then reset and
continue bisecting.
Teaching bisect to consistently skip a commit, or to automatically
squash in A' if we have A and not A', would be a desirable feature. I
will have to read up some more on notes.
^ permalink raw reply
* Issues that need to be resolved before 1.7.0-rc1
From: Junio C Hamano @ 2010-01-25 18:29 UTC (permalink / raw)
To: git
In-Reply-To: <7vfx5u6bn9.fsf@alter.siamese.dyndns.org>
Right now 'next' is empty and there are a handful topics in 'pu':
- Jens Lehmann's updates to omit expensive is_submodule_modified() call
when running "diff --ignore-submodules" and to make "diff --submodule"
to show the "dirty worktree" status. I think they should be in 1.7.0
as part of the series to show work tree/index dirtiness in "git diff"
output from the superproject.
- Christian Couder's "reset --keep". I am inclined to exclude it from
1.7.0 as I cannot make a convincing argument to sell this to users
without confusing them (even I am not convinced even though I do fairly
straight line development at day-job), but hopefully we can keep
cooking it in 'pu/next'.
- Johan Herland's notes updates. Won't be ready for 1.7.0 but hopefully
we can keep cooking in 'pu/next'.
- John Hawley's gitweb caching layer. Jakub is splitting them into
smaller pieces and they are still generating discussions.
- My "how about this" patch to make 'grep --author=foo --grep=bar' to
implicitly run --all-match; this is not yet readyand needs reworking.
There also is a discussion on //server/share path on msysgit, that was
started since 288123f (ignore duplicated slashes in make_relative_path(),
2010-01-21). If that commit _breaks_ use case on msysgit that used to
work, I think the sanest course of action is to revert it and ship 1.7.0
with the same breakage we've had forever when the user gives extra slashes
in the paths to --git-dir or --work-tree options. We could instead apply
Thomas's patch
Message-ID: <379d55c6a4110736aadb8ace3b050de879a9deab.1264118830.git.trast@student.ethz.ch>
that changed behaviour in much narrower case, which should be safer. In
any case, a more intrusive change for UNC is outside the scope of 1.7.0
(discussion and preparation can continue on 'pu/next').
I saw a few patches and discussion to git-p4 in contrib from Pal-Kristian
Engstad and I understand that we are waiting for revised version before I
should take any action on them.
The above are the current set of unresolved topics and their status but I
am sure I've missed some others. Please remind me and the list if there
are other important ones that need to be discussed.
^ permalink raw reply
* Re: [PATCH] Handle UNC paths everywhere
From: Johannes Schindelin @ 2010-01-25 18:19 UTC (permalink / raw)
To: kusmabite; +Cc: Robin Rosenberg, git, Johannes Sixt, Junio C Hamano
In-Reply-To: <40aa078e1001250957h292f8b01me8f7dec4ba2b425b@mail.gmail.com>
Hi,
On Mon, 25 Jan 2010, Erik Faye-Lund wrote:
> On Mon, Jan 25, 2010 at 6:34 PM, Johannes Schindelin
> <Johannes.Schindelin@gmx.de> wrote:
> > Hi,
> >
> > On Mon, 25 Jan 2010, Robin Rosenberg wrote:
> >
> >> >From 37a74ccd395d91e5662665ca49d7f4ec49811de0 Mon Sep 17 00:00:00 2001
> >> From: Robin Rosenberg <robin.rosenberg@dewire.com>
> >> Date: Mon, 25 Jan 2010 01:41:03 +0100
> >> Subject: [PATCH] Handle UNC paths everywhere
> >>
> >> In Windows paths beginning with // are knows as UNC paths. They are
> >> absolute paths, usually referring to a shared resource on a server.
> >
> > And even a simple "cd" with them does not work.
> >
>
> But it does, at least for me - both in bash and cmd.exe. I just need
> to log on to the server first, unless it's a public share.
I love it when people say "it works for me, so let's do it".
_My_ _only_ instance of Windows cmd says this:
C:\Blah> cd \\localhost
'\\localhost'
CMD does not support UNC paths as current directories.
C:\Blah>
So.
Besides, the patch was not in a form where I can say that it was obviously
fixing the issue. It was rather in a form where I would have to have set
aside a substantial amount of time to verify that nothing undesired was
introduced as a side effect.
Ciao,
Dscho
^ permalink raw reply
* Re: [PATCH] rebase -p: Preserve fast-forwardable merges
From: Alex Scarborough @ 2010-01-25 18:53 UTC (permalink / raw)
To: Johannes Schindelin
Cc: git, Junio C Hamano, Michael Haggerty, Alex Scarborough
In-Reply-To: <alpine.DEB.1.00.1001251508400.8733@intel-tinevez-2-302>
On Mon, Jan 25, 2010, Johannes Schindelin wrote:
> On Fri, 22 Jan 2010, Alex Scarborough wrote:
>
> > Previously, rebase -p would not preserve a merge commit if the merge
> > could be resolved as a fast-forward. rebase -p now passes --no-ff to
> > git merge when recreating a merge commit, which ensures that merge
> > commits created with git merge --no-ff are preserved.
>
> For my use case (well, it used to be my use case), namely keeping a number
> of topic branches on top of an upstream up-to-date, this is not the
> desired action. In my use case, merged topic branches should just vanish,
> and not even leave a merge commit.
I see. In that use case this patch would be rather irritating :)
What do you think of adding a --no-ff option to git rebase which, when used
with -p, will recreate merge commits even if they could be resolved as a
fast-forward? That would leave your use case unchanged while giving my
use case a way out, so to speak.
Either way, I suggest we change the documentation for rebase -p to state
that it does not preserve merge commits that can be fast-forwarded after
rebasing.
If it sounds good, I should be able to roll some patches by the end of the
week.
-Alex Scarborough
^ permalink raw reply
* Re: [PATCH 01/18] rebase -i: Make the condition for an "if" more transparent
From: Johannes Schindelin @ 2010-01-25 18:28 UTC (permalink / raw)
To: Michael Haggerty; +Cc: git, gitster
In-Reply-To: <aa37ee8a68d460df172b23b4999fbe4ce7d77c1e.1263447037.git.mhagger@alum.mit.edu>
Hi,
On Thu, 14 Jan 2010, Michael Haggerty wrote:
> @@ -166,7 +166,8 @@ pick_one () {
> parent_sha1=$(git rev-parse --verify $sha1^) ||
> die "Could not get the parent of $sha1"
> current_sha1=$(git rev-parse --verify HEAD)
> - if test "$no_ff$current_sha1" = "$parent_sha1"; then
> + if test -z "$no_ff" -a "$current_sha1" = "$parent_sha1"
Rather use &&, right?
Ciao,
Dscho
^ permalink raw reply
* Re: mesa_7_7_branch -> master merges
From: tom fogal @ 2010-01-25 19:04 UTC (permalink / raw)
To: José Fonseca; +Cc: git, mesa3d-dev
In-Reply-To: <1264443264.3029.255.camel@jfonseca-laptop>
I think we've touched on a core git workflow issue here, and its likely
others have hit this && have a solution, so I've added the git ML to
the CC list.
Git: the situation in this repo is a fast-moving master that is
including many changes to internal interfaces. Stable branches just
get bugfixes, and are periodically merged to master. However, the more
the heads diverge, the more difficult it is for a bugfix to merge into
the head. The major issue is that more experienced developers should
really weigh in on these merges, because they tend to automagically
undo some of the interface changes. Yet during such a delay, master
inevitably moves, and the bugfixer has to do even more work to "redo"
the merge (and potentially get more review!).
Of course, if there are two bugfixers trying to make separate changes
in the same time period, this gets worse.
Is there a workflow that can solve this issue?
writes:
> On Mon, 2010-01-25 at 09:52 -0800, tom fogal wrote:
> > writes:
> > [snip]
> > > The ideal would be to peer-review the merges before committing,
> > > but it seems difficult to do that with git, while preserving merge
> > > history and not redoing merges.
> >
> > Google has developed an infrastructure to do peer review using git.
> > `Gerrit':
[snip]
> Review infrastructures are nice. I'd have some bias towards
> http://www.reviewboard.org/ by the similar reasons ;)
Heh, yeah I can understand the bias ;)
Personally, I'm not keen on a review tool I can't use from the command
line, or at least not-the-web. Then again, my reviews wouldn't really
be important in Mesa, so my opinion is irrelevant here ;)
> But automated infrastructures aside, my worry with reviewing merges is
> the actual constraints that git has. For example, let's suppose the
> following scenario:
>
> 1) Developer A merges a stable branch into master.
> 2) After spending a bunch of time fixing conflicts the best he can, he
> emails the patch to mesa3d-dev for peer review.
> 3) Developer B checks in a change into master.
> 4) Developer A takes feedback from list, updates the code, and commits.
> 5) Developer A cannot push because remote head has moved.
>
> So what can Developer A do now?
>
> a) Redo the merge, using the new master head.
> b) Rebase the merge on top of the new head (I'm not sure it works, or
> that it preserves branch history)
> c) Double merge, i.e., merge its local head with the new master head.
Hrm, I was thinking of some sort of staging branch, but I can't think
of a good way to make it work. The crux of the issue seems to be that
a developer needs to somehow give a version control promise that they
will do the merge, even if the merge isn't done yet, because otherwise
anyone else coming afterwards will duplicate the work (potentially
incorrectly). That would mean some kind of lock though, which sounds
like a terrible idea...
-tom
------------------------------------------------------------------------------
The Planet: dedicated and managed hosting, cloud storage, colocation
Stay online with enterprise data centers and the best network in the business
Choose flexible plans and management services without long-term contracts
Personal 24x7 support from experience hosting pros just a phone call away.
http://p.sf.net/sfu/theplanet-com
^ permalink raw reply
* Re: [PATCH 00/18] rebase -i: For pure fixups, do not start log message editor
From: Johannes Schindelin @ 2010-01-25 18:38 UTC (permalink / raw)
To: Michael Haggerty; +Cc: git, gitster
In-Reply-To: <cover.1263447037.git.mhagger@alum.mit.edu>
Hi,
On Thu, 14 Jan 2010, Michael Haggerty wrote:
> This patch series is a successor to mh/rebase-fixup, which causes
> "rebase -i" to skip opening the log message editor when processing a
> block of "fixup" commands that does not include any "squash"es.
I knew why I kept this patch series as the last thing I would do before
going offline: it is real elegantly done.
Ciao,
Dscho
^ permalink raw reply
* Re: [PATCH] Handle UNC paths everywhere
From: Robin Rosenberg @ 2010-01-25 19:37 UTC (permalink / raw)
To: kusmabite; +Cc: Johannes Schindelin, git, Johannes Sixt, Junio C Hamano
In-Reply-To: <40aa078e1001250957h292f8b01me8f7dec4ba2b425b@mail.gmail.com>
måndagen den 25 januari 2010 18.57.06 skrev Erik Faye-Lund:
> On Mon, Jan 25, 2010 at 6:34 PM, Johannes Schindelin
>
> <Johannes.Schindelin@gmx.de> wrote:
> > Hi,
> >
> > On Mon, 25 Jan 2010, Robin Rosenberg wrote:
> >> >From 37a74ccd395d91e5662665ca49d7f4ec49811de0 Mon Sep 17 00:00:00 2001
> >>
> >> From: Robin Rosenberg <robin.rosenberg@dewire.com>
> >> Date: Mon, 25 Jan 2010 01:41:03 +0100
> >> Subject: [PATCH] Handle UNC paths everywhere
> >>
> >> In Windows paths beginning with // are knows as UNC paths. They are
> >> absolute paths, usually referring to a shared resource on a server.
> >
> > And even a simple "cd" with them does not work.
>
> But it does, at least for me - both in bash and cmd.exe. I just needt.
In cmd,exe surprises me a bit. pushd \\server\share is not the same
as it maps a drive and then uses it to cd.
-- robin
^ permalink raw reply
* Re: [Mesa3d-dev] mesa_7_7_branch -> master merges
From: tom fogal @ 2010-01-25 19:14 UTC (permalink / raw)
To: git; +Cc: jfonseca
This bounced, it seems because Jose's [1] name is not representable in
8bit ASCII (some header wasn't, at least).
I'm not cc'ing Mesa to avoid spamming everyone. I'm not sure
non-subscribers can post there anyway. Jose or myself will forward
along any relevant discussion...
[1] sorry for misspelling it there..
------- Forwarded Message
From: tom fogal <tfogal@alumni.unh.edu>
To: José Fonseca <jfonseca@vmware.com>
cc: mesa3d-dev <mesa3d-dev@lists.sourceforge.net>, git@vger.kernel.org
Subject: Re: [Mesa3d-dev] mesa_7_7_branch -> master merges
In-Reply-To: Your message of "Mon, 25 Jan 2010 18:14:24 GMT."
<1264443264.3029.255.camel@jfonseca-laptop>
References: <1264424650.3029.155.camel@jfonseca-laptop> <auto-000021765525@sci.utah.edu> <1264443264.3029.255.camel@jfonseca-laptop>
Date: Mon, 25 Jan 2010 12:04:00 -0700
I think we've touched on a core git workflow issue here, and its likely
others have hit this && have a solution, so I've added the git ML to
the CC list.
Git: the situation in this repo is a fast-moving master that is
including many changes to internal interfaces. Stable branches just
get bugfixes, and are periodically merged to master. However, the more
the heads diverge, the more difficult it is for a bugfix to merge into
the head. The major issue is that more experienced developers should
really weigh in on these merges, because they tend to automagically
undo some of the interface changes. Yet during such a delay, master
inevitably moves, and the bugfixer has to do even more work to "redo"
the merge (and potentially get more review!).
Of course, if there are two bugfixers trying to make separate changes
in the same time period, this gets worse.
Is there a workflow that can solve this issue?
writes:
> On Mon, 2010-01-25 at 09:52 -0800, tom fogal wrote:
> > writes:
> > [snip]
> > > The ideal would be to peer-review the merges before committing,
> > > but it seems difficult to do that with git, while preserving merge
> > > history and not redoing merges.
> >
> > Google has developed an infrastructure to do peer review using git.
> > `Gerrit':
[snip]
> Review infrastructures are nice. I'd have some bias towards
> http://www.reviewboard.org/ by the similar reasons ;)
Heh, yeah I can understand the bias ;)
Personally, I'm not keen on a review tool I can't use from the command
line, or at least not-the-web. Then again, my reviews wouldn't really
be important in Mesa, so my opinion is irrelevant here ;)
> But automated infrastructures aside, my worry with reviewing merges is
> the actual constraints that git has. For example, let's suppose the
> following scenario:
>
> 1) Developer A merges a stable branch into master.
> 2) After spending a bunch of time fixing conflicts the best he can, he
> emails the patch to mesa3d-dev for peer review.
> 3) Developer B checks in a change into master.
> 4) Developer A takes feedback from list, updates the code, and commits.
> 5) Developer A cannot push because remote head has moved.
>
> So what can Developer A do now?
>
> a) Redo the merge, using the new master head.
> b) Rebase the merge on top of the new head (I'm not sure it works, or
> that it preserves branch history)
> c) Double merge, i.e., merge its local head with the new master head.
Hrm, I was thinking of some sort of staging branch, but I can't think
of a good way to make it work. The crux of the issue seems to be that
a developer needs to somehow give a version control promise that they
will do the merge, even if the merge isn't done yet, because otherwise
anyone else coming afterwards will duplicate the work (potentially
incorrectly). That would mean some kind of lock though, which sounds
like a terrible idea...
- -tom
------- End of Forwarded Message
^ permalink raw reply
* Re: [PATCH] Handle UNC paths everywhere
From: Robin Rosenberg @ 2010-01-25 19:45 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: git, Johannes Sixt, Junio C Hamano
In-Reply-To: <alpine.DEB.1.00.1001251553150.8733@intel-tinevez-2-302>
måndagen den 25 januari 2010 18.34.01 skrev Johannes Schindelin:
> Hi,
>
> On Mon, 25 Jan 2010, Robin Rosenberg wrote:
> > >From 37a74ccd395d91e5662665ca49d7f4ec49811de0 Mon Sep 17 00:00:00 2001
> >
> > From: Robin Rosenberg <robin.rosenberg@dewire.com>
> > Date: Mon, 25 Jan 2010 01:41:03 +0100
> > Subject: [PATCH] Handle UNC paths everywhere
> >
> > In Windows paths beginning with // are knows as UNC paths. They are
> > absolute paths, usually referring to a shared resource on a server.
>
> And even a simple "cd" with them does not work.
>
> > Examples of legal UNC paths
> >
> > \\hub\repos\repo
> > \\?\unc\hub\repos
> > \\?\d:\repo
> >
> > Signed-off-by: Robin Rosenberg <robin.rosenberg@dewire.com>
> > ---
> > cache.h | 2 +-
> > compat/basename.c | 2 +-
> > compat/mingw.h | 8 +++++++-
> > connect.c | 2 +-
> > git-compat-util.h | 9 +++++++++
> > path.c | 2 +-
> > setup.c | 2 +-
> > sha1_file.c | 20 ++++++++++++++++++++
> > transport.c | 2 +-
> > 9 files changed, 42 insertions(+), 7 deletions(-)
>
> Ouch. You should know better than to clutter non-Windows-specific parts
> with that ugly kludge.
>
> > diff --git a/cache.h b/cache.h
> > index 767a50e..8f63640 100644
> > --- a/cache.h
> > +++ b/cache.h
> > @@ -648,7 +648,7 @@ int safe_create_leading_directories_const(const char
> > *path);
> > char *enter_repo(char *path, int strict);
> > static inline int is_absolute_path(const char *path)
> > {
> > - return path[0] == '/' || has_dos_drive_prefix(path);
> > + return path[0] == '/' || has_win32_abs_prefix(path);
>
> Why? We can still keep the name. Well, maybe not, see below.
I do think function names should imply something about their behaviour.
>
> > diff --git a/compat/mingw.h b/compat/mingw.h
> > index 1b528da..d1aa8be 100644
> > --- a/compat/mingw.h
> > +++ b/compat/mingw.h
> > @@ -210,7 +210,13 @@ int winansi_fprintf(FILE *stream, const char
> > *format, ...) __attribute__((format
> > * git specific compatibility
> > */
> >
> > -#define has_dos_drive_prefix(path) (isalpha(*(path)) && (path)[1] ==
> > ':') +#define has_dos_drive_prefix(path) \
> > + (isalpha(*(path)) && (path)[1] == ':')
>
> Why?
To avoid very long lines and format this (now) set of related macros
uniformely.
> > +#define has_unc_prefix(path) \
> > + (is_dir_sep((path)[0]) && is_dir_sep((path)[1]))
> > +#define has_win32_abs_prefix(path) \
> > + (has_dos_drive_prefix(path) || has_unc_prefix(path))
>
> "c:hello.txt" is not an absolute path.
Ok. Nevertheless that was how it was treated before, It's not relative,
either, but some quasirelative thing. has_win32_quasi_abs_prefix?
> > diff --git a/connect.c b/connect.c
> > index 7945e38..9d4556c 100644
> > --- a/connect.c
> > +++ b/connect.c
> > @@ -535,7 +535,7 @@ struct child_process *git_connect(int fd[2], const
> > char *url_orig,
> > end = host;
> >
> > path = strchr(end, c);
> > - if (path && !has_dos_drive_prefix(end)) {
> > + if (path && !has_win32_abs_prefix(end)) {
> > if (c == ':') {
>
> Why? Do we really have to exclude UNC paths from that ":" handling?
That colon is about URL-ish things... Right.
> > diff --git a/git-compat-util.h b/git-compat-util.h
> > index ef60803..0de9dac 100644
> > --- a/git-compat-util.h
> > +++ b/git-compat-util.h
> > @@ -170,6 +170,15 @@ extern char *gitbasename(char *);
> > #define has_dos_drive_prefix(path) 0
> > #endif
> >
> > +#ifndef has_unc_prefix
> > +#define has_unc_prefix(path) 0
> > +#endif
> > +
> > +#ifndef has_win32_abs_prefix
> > +#error no abs
ouch, a leftover from trying to figure out a complation message.
>
> Yeah, sure. I do have abs, thank you very much.
>
> In general, I am _very_ worried about your patch. It does not acknowledge
> that there is a fundamental difference between DOS drive prefixes and UNC
> paths, and not being able to "cd" to the latter is just a symptom.
As I said. Most programs including bash, but excluding cmd.exe can set the
working directory to an UNC path. I cannot fix cmd.exe and rarely use it
with git, but the patch helps even if you cannot cd from a UNC challenged
shell.
> I am also not quite sure if you can get away with having the same offset
> for both: if I have "C:\blah" and strip off "C:", I always have a
> directory separator to bounce against, whereas I do not have that if I
> strip off the two "\\" of a UNC path. Besides, I maintain that the host
> name, and maybe even the share name, should not ever be stripped off!
When creating directoties you only strip them off for the purpose of finding
paths to mkdir. The server and share part you cannot mkdir anyway, they
must exist before attempting to create a directory, hence I skip past those
portions. As for the \-less path beginning with a drive I'll reconsider. I did
not test that one.
-- robin
^ permalink raw reply
* Re: [PATCH] Handle UNC paths everywhere
From: Erik Faye-Lund @ 2010-01-25 19:45 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: Robin Rosenberg, git, Johannes Sixt, Junio C Hamano
In-Reply-To: <alpine.DEB.1.00.1001251916030.8733@intel-tinevez-2-302>
On Mon, Jan 25, 2010 at 7:19 PM, Johannes Schindelin
<Johannes.Schindelin@gmx.de> wrote:
> Hi,
>
> On Mon, 25 Jan 2010, Erik Faye-Lund wrote:
>
>> On Mon, Jan 25, 2010 at 6:34 PM, Johannes Schindelin
>> <Johannes.Schindelin@gmx.de> wrote:
>> > Hi,
>> >
>> > On Mon, 25 Jan 2010, Robin Rosenberg wrote:
>> >
>> >> >From 37a74ccd395d91e5662665ca49d7f4ec49811de0 Mon Sep 17 00:00:00 2001
>> >> From: Robin Rosenberg <robin.rosenberg@dewire.com>
>> >> Date: Mon, 25 Jan 2010 01:41:03 +0100
>> >> Subject: [PATCH] Handle UNC paths everywhere
>> >>
>> >> In Windows paths beginning with // are knows as UNC paths. They are
>> >> absolute paths, usually referring to a shared resource on a server.
>> >
>> > And even a simple "cd" with them does not work.
>> >
>>
>> But it does, at least for me - both in bash and cmd.exe. I just need
>> to log on to the server first, unless it's a public share.
>
> I love it when people say "it works for me, so let's do it".
>
> _My_ _only_ instance of Windows cmd says this:
>
> C:\Blah> cd \\localhost
> '\\localhost'
> CMD does not support UNC paths as current directories.
>
> C:\Blah>
>
> So.
Actually, you're right about cmd.exe - I somehow mixed that up.
However, it works fine in bash (and simply by doing chdir() from a
normal C-program), as long as I've logged on in advance.
This applies to my Vista 64 and XP installations.
>
> Besides, the patch was not in a form where I can say that it was obviously
> fixing the issue. It was rather in a form where I would have to have set
> aside a substantial amount of time to verify that nothing undesired was
> introduced as a side effect.
>
This I can agree on. I just wanted to clear up the situation about
cd'ing. But I failed - hopefully that's corrected now.
--
Erik "kusma" Faye-Lund
^ permalink raw reply
* Re: [PATCH] Handle UNC paths everywhere
From: Erik Faye-Lund @ 2010-01-25 19:48 UTC (permalink / raw)
To: Robin Rosenberg; +Cc: Johannes Schindelin, git, Johannes Sixt, Junio C Hamano
In-Reply-To: <201001252037.41497.robin.rosenberg@dewire.com>
On Mon, Jan 25, 2010 at 8:37 PM, Robin Rosenberg
<robin.rosenberg@dewire.com> wrote:
> måndagen den 25 januari 2010 18.57.06 skrev Erik Faye-Lund:
>> On Mon, Jan 25, 2010 at 6:34 PM, Johannes Schindelin
>>
>> <Johannes.Schindelin@gmx.de> wrote:
>> >
>> > And even a simple "cd" with them does not work.
>>
>> But it does, at least for me - both in bash and cmd.exe. I just needt.
>
> In cmd,exe surprises me a bit. pushd \\server\share is not the same
> as it maps a drive and then uses it to cd.
>
> -- robin
>
My guess would be that Dscho mentioned this because git internally
does a chdir() to the path that is cloned, so currently chdir'ing must
be supported for clone to work with a "local repo".
--
Erik "kusma" Faye-Lund
^ permalink raw reply
* Re: [PATCH] Handle UNC paths everywhere
From: Robin Rosenberg @ 2010-01-25 20:04 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: git, Johannes Sixt, Junio C Hamano
In-Reply-To: <alpine.DEB.1.00.1001251553150.8733@intel-tinevez-2-302>
måndagen den 25 januari 2010 18.34.01 skrev Johannes Schindelin:
> I am also not quite sure if you can get away with having the same offset
> for both: if I have "C:\blah" and strip off "C:", I always have a
> directory separator to bounce against, whereas I do not have that if I
> strip off the two "\\" of a UNC path. Besides, I maintain that the host
> name, and maybe even the share name, should not ever be stripped off!
Advices needed:
d:somedir (when cwd=d:\msysgit, == /) may be tricky to fix.
Msysgit seems confused by the syntax and treats it as d:\
roro@SIENA / (master)
$ cmd
Microsoft Windows [Version 5.2.3790]
(C) Copyright 1985-2003 Microsoft Corp.
D:\msysgit>exit
roro@SIENA / (master)
$ mkdir d:x
mkdir: cannot create directory `d:x': File exist
roro@SIENA / (master)
$ cd d:x
roro@SIENA /d
$ ls -l x
ls: x: No such file or directory
roro@SIENA /d
$
From that I think that even if we try to make git handle d:path, msys
will break regardless. We can fix truly absolute and normal relative paths.
-- robin
^ permalink raw reply
* Re: [PATCH] Handle UNC paths everywhere
From: Johannes Sixt @ 2010-01-25 20:07 UTC (permalink / raw)
To: Robin Rosenberg; +Cc: git, Junio C Hamano
In-Reply-To: <201001250155.47664.robin.rosenberg@dewire.com>
On Montag, 25. Januar 2010, Robin Rosenberg wrote:
> In Windows paths beginning with // are knows as UNC paths. They are
> absolute paths, usually referring to a shared resource on a server.
>
> Examples of legal UNC paths
>
> \\hub\repos\repo
> \\?\unc\hub\repos
> \\?\d:\repo
I agree that that the problem that you are addressing needs a solution.
However, the solution is not a whole-sale replacement of
have_dos_drive_prefix() by a function that is only a tiny bit fancier.
Accompanying changes are needed, and perhaps more code locations need change.
> @@ -648,7 +648,7 @@ int safe_create_leading_directories_const(const char
> *path);
> char *enter_repo(char *path, int strict);
> static inline int is_absolute_path(const char *path)
> {
> - return path[0] == '/' || has_dos_drive_prefix(path);
> + return path[0] == '/' || has_win32_abs_prefix(path);
Perhaps we need is_dir_sep(path[0]) here? But since I have not observed any
breakage in connection with this code, I think that all callers feed only
normalized paths (i.e. with forward slash). (Note that our getcwd()
implementation converts backslashes to forward slashes.) This means that a
full-fledged check is not needed.
> @@ -5,7 +5,7 @@ char *gitbasename (char *path)
> {
> const char *base;
> /* Skip over the disk name in MSDOS pathnames. */
> - if (has_dos_drive_prefix(path))
> + if (has_win32_abs_prefix(path))
> path += 2;
This change is unnecessary; it really is only to skip an initial driver
prefix. If you want to support \\?\X: style paths, more work is needed here
so that you do not return X: or ? as the basename.
> +#define has_win32_abs_prefix(path) \
Do we really have to name everything "win32" when it is about Windows?
> @@ -535,7 +535,7 @@ struct child_process *git_connect(int fd[2], const char
> *url_orig,
> end = host;
>
> path = strchr(end, c);
> - if (path && !has_dos_drive_prefix(end)) {
> + if (path && !has_win32_abs_prefix(end)) {
This change is wrong because the check is really only about the drive prefix:
It checks that we do not mistake c:/foo as a ssh connection to host c,
path /foo. Yes, it does mean that on Windows we cannot have remotes to hosts
whose name consists only of a single letter using the rcp notation (you must
say ssh://c/foo if you mean it).
> @@ -409,7 +409,7 @@ int normalize_path_copy(char *dst, const char *src)
> {
> char *dst0;
>
> - if (has_dos_drive_prefix(src)) {
> + if (has_win32_abs_prefix(src)) {
> *dst++ = *src++;
> *dst++ = *src++;
> }
Is skipping just two characters for \\ or \\?\whatever paths the right thing?
> @@ -342,7 +342,7 @@ const char *setup_git_directory_gently(int *nongit_ok)
> die_errno("Unable to read current working directory");
>
> ceil_offset = longest_ancestor_length(cwd, env_ceiling_dirs);
> - if (ceil_offset < 0 && has_dos_drive_prefix(cwd))
> + if (ceil_offset < 0 && has_win32_abs_prefix(cwd))
> ceil_offset = 1;
I doubt that this is correct. The purpose of this check is that "c:/" is the
last directory that is checked (on Unix it would be "/") when path components
are stripped from cwd. For UNC paths this must be adjusted depending on how
you want to support \\server\share and \\?\c:\paths: You do not want to check
whether \\server\.git or \\.git or \\?\.git are git directories.
> --- a/transport.c
> +++ b/transport.c
> @@ -797,7 +797,7 @@ static int is_local(const char *url)
> const char *colon = strchr(url, ':');
> const char *slash = strchr(url, '/');
> return !colon || (slash && slash < colon) ||
> - has_dos_drive_prefix(url);
> + has_win32_abs_prefix(url);
This check is again to not mistake c:/foo as rcp style connection. No change
needed.
As I said, changes to other parts are perhaps also needed, most prominently,
make_relative_path() that prompted this patch. What about
make_absolute_path() and make_non_relative_path()?
-- Hannes
^ permalink raw reply
* Re: [PATCH] Handle UNC paths everywhere
From: Robin Rosenberg @ 2010-01-25 20:15 UTC (permalink / raw)
To: kusmabite; +Cc: Johannes Schindelin, git, Johannes Sixt, Junio C Hamano
In-Reply-To: <40aa078e1001251148p6263feeevc85a3223f85873@mail.gmail.com>
måndagen den 25 januari 2010 20.48.22 skrev Erik Faye-Lund:
> On Mon, Jan 25, 2010 at 8:37 PM, Robin Rosenberg
>
> <robin.rosenberg@dewire.com> wrote:
> > måndagen den 25 januari 2010 18.57.06 skrev Erik Faye-Lund:
> >> On Mon, Jan 25, 2010 at 6:34 PM, Johannes Schindelin
> >>
> >> <Johannes.Schindelin@gmx.de> wrote:
> >> > And even a simple "cd" with them does not work.
> >>
> >> But it does, at least for me - both in bash and cmd.exe. I just needt.
> >
> > In cmd,exe surprises me a bit. pushd \\server\share is not the same
> > as it maps a drive and then uses it to cd.
> >
> > -- robin
>
> My guess would be that Dscho mentioned this because git internally
> does a chdir() to the path that is cloned, so currently chdir'ing must
chdir doesn't call cmd.exe and does not suffer from cmd's limitations.
-- robin
^ permalink raw reply
* Re: [RFC PATCH 10/10] gitweb: Show appropriate "Generating..." page when regenerating cache (WIP)
From: J.H. @ 2010-01-25 20:32 UTC (permalink / raw)
To: Petr Baudis; +Cc: Jakub Narebski, git, John 'Warthog9' Hawley
In-Reply-To: <20100125135653.GN4159@machine.or.cz>
On 01/25/2010 05:56 AM, Petr Baudis wrote:
> On Mon, Jan 25, 2010 at 02:48:26PM +0100, Jakub Narebski wrote:
>> Now those patches (mine and J.H. both) make gitweb use locking
>> (it is IIRC configurable in J.H. patch) to make only one process
>> generate the page if it is missing from cache, or is stale. Now
>> if it is missing, we have to wait until it is generated in full
>> before being able to show it to client. While it is possible to
>> "tee" output (using PerlIO::tee, or Capture::Tiny, or tie like
>> CGI::Cache) writing it simultaneously to browser and to cache for
>> the process that is generating data, it is as far as I understand
>> it impossible for processes which are waiting for data. Therefore
>> the need for "Generating..." page, so the user does not think that
>> web server hung or something, and is not generating output.
>
> Ah, ok, so the message is there to cover up for a technical problem. ;-)
> I didn't quite realize. Then, it would be great to tweak the mechanisms
> so that the user does not really have to wait.
No, that is an incorrect assumption on how the 'Generating...' page
works, and your missing a bit of the point.
(1) The message itself 'Generating...' is a que to the user that
something is happening and that the browser is not actually hanging.
Web users are at the point where if things are not instantaneous and
show immediately they will either browse away completely or hit the
refresh button incessantly until content does appear. While the page is
usually only seen for about a second, and I'll admit it can be annoying,
it's nothing more than a 'sit tight a second'. For things like the
front page it can take upwards of 7 seconds to generate for a single
user, a lot to ask for a no response scenario.
(2) It prevents the stampeding herd problem, which was very vehemently
discussed 4 years ago by HPA and myself and roughly boils down to this:
When a single user comes into the site, in particular the front page, it
kicks off a process that will start to generate at it, causing a huge
amount of git requests into individual repositories and a lot of disk
i/o. A second user will then come in and the same requests will start
to be done from the beginning again, and so on until you basically kill
the machine because the disk i/o goes up enough that it can't ever
service the requests fast enough.
This does 2 things in the end:
1) means there's only 1 copy of the page ever being generated, thus
meaning there isn't extraneous and dangerous disk i/o going on on the system
2) prevents a user from reporting to the website that it's broken by
giving them a visual que that things aren't broken.
> So, I wonder about two things:
>
> (i) How often does it happen that two requests for the same page are
> received? Has anyone measured it? Or is at least able to make
> a minimally educated guess? IOW, isn't this premature optimization?
For most pages, not many but it happens more often than you think. The
data I have is much too old to be useful now but the front page could,
at times, have up to 30 people waiting for it without caching. This is
a very important patch believe it or not. For a site the size of
kernel.org it cannot exist without this.
But here's a quick stat, in 36 hours git.kernel.org has had
156099 accesses world wide or about 1.2 accesses a second.
android.git.kernel.org, in the same time period has had 115818 accesses.
If the first request takes 7 seconds to generate, by the time it's done
there are now 3 additional requests running. If it again takes 7
seconds to generate there are now another 3 requests running, etc. Very
quickly you've got so much i/o running the box more or less is useless.
> (ii) Can't the locked gitwebs do the equivalent of tail -f?
Not really going to help much, most of the gitweb operations won't
output much of anything beyond the header until it's collected all of
the data it needs anyway and then there will be a flurry of output. It
also means that this 'Generating...' page will only work for caching
schemes that tail can read out of, which I'm not sure it would work all
that well with things like memcached or a non-custom caching layer where
we don't necessarily have direct access to the file being written to.
At least the way I had it (and I'll admit I haven't read through Jakub's
re-working of my patches so I don't know if it's still there) is that
with background caching you only get the 'Generating...' page if it's
new or the content is grossly out of data. If it's a popular page and
it's not grossly out of date it shows you the 'stale' data while it
generates the new content in the background anyway, only locking you out
when the new file is being written. Or at least that's how I had it.
- John 'Warthog9' Hawley
^ permalink raw reply
* Re: [RFC PATCH 10/10] gitweb: Show appropriate "Generating..." page when regenerating cache (WIP)
From: J.H. @ 2010-01-25 20:41 UTC (permalink / raw)
To: Jakub Narebski; +Cc: Petr Baudis, git, John 'Warthog9' Hawley
In-Reply-To: <201001251448.27513.jnareb@gmail.com>
On 01/25/2010 05:48 AM, Jakub Narebski wrote:
> On Mon, Jan 25, 2010, Petr Baudis wrote:
>> On Mon, Jan 25, 2010 at 12:46:39PM +0100, Jakub Narebski wrote:
>>> On Sun, 24 Jan 2010, Petr Baudis wrote:
>
>>>> I have stupid question, common to both the original patch and this
>>>> RFC.
>>>>
>>>>> [RFC PATCH 10/10] gitweb: Show appropriate "Generating..." page when
>>>>> regenerating cache (WIP)
>>>>
>>>> Just why is a "Generating..." page appropriate?
>>>>
>>>> I have to admit I hate it; can you please at least make it
>>>> configurable? Why is it needed at all? It [...] confuses
>>>> non-interactive HTTP clients [...]
>
>>> Second, gitweb can always check User-Agent header, and serve
>>> "Generating..." page only to web browsers:
>>>
>>> unless (defined $cgi->user_agent() &&
>>> $cgi->user_agent() =~ /\b(Mozilla|Opera)\b/i) {
>>> return;
>>> }
>>>
>>> or something like that.
>>
>> I'm not too happy with this. What about Safari? Opera? ELinks? There's a
>> lot of web browsers.
>
> The "Mozilla" part would catch all "Mozilla compatibile" web browsers,
> including Firefox (and other Gecko-based web browsers), Internet Explorer,
> WebKit based browsers including Safari and Chrome and Konqueror.
> The "Opera" part would catch Opera.
> http://www.nczonline.net/blog/2010/01/12/history-of-the-user-agent-string/
>
> As to other web browsers like Elinks, Lynx, w3m, Dillo, etc.: the issue
> is whether they honor '<meta http-equiv="refresh" content="0" />'.
> I think it is better to stay on the safe side; it is not disaster if web
> browser is not shown "Generating..." page where it could (but see
> explanation below).
Most of them do, that particular tag has been around for a long time and
since it doesn't require Javascript to do the page refresh it's pretty
much universal.
The problem is going to be with things like wget when someone wants to
snag a binary file. This works fine if the file is already cached, but
the user doesn't get what they are expecting if they get a blob that
isn't the final file, but the html contents of the page. I don't know
of any hint that things like wget would send to the server that you
could switch based on, but it would be more or less the non-background
caching state.
>> Most of the issues can be worked around, but I'm not sure why to go
>> through all the trouble. I just personally don't see the value in having
>> the placeholder in there at all, to me it is distracting UI even if all
>> the technicalities are put aside.
>
> The issue that "Generating..." page tries to solve is, I think, the
> following.
>
> Some actions, like 'blame' view or pickaxe search, or grep search,
> can take quite a long time to generate, with times counted in
> tens of seconds. It is not that visible for non-caching, because
> gitweb streams output so we have at least _some_ output upfront quite
> fast.
>
> Now those patches (mine and J.H. both) make gitweb use locking
> (it is IIRC configurable in J.H. patch) to make only one process
> generate the page if it is missing from cache, or is stale. Now
> if it is missing, we have to wait until it is generated in full
> before being able to show it to client. While it is possible to
> "tee" output (using PerlIO::tee, or Capture::Tiny, or tie like
> CGI::Cache) writing it simultaneously to browser and to cache for
> the process that is generating data, it is as far as I understand
> it impossible for processes which are waiting for data. Therefore
> the need for "Generating..." page, so the user does not think that
> web server hung or something, and is not generating output.
>
> We can try to reduce occurrences of cache miss stampedes by using
> 'expires_variance' feature[1] from CHI - Unified caching interface.
> We can also turn off locking and tee output to have some output upfront
> as an activity indicator instead of this "Generating..." page.
>
> [1]: http://search.cpan.org/~jswartz/CHI-0.33/lib/CHI.pm#set
>
>> But if it will be possible to turn this off eventually, it's all your
>> call whether to bother implementing it. :-)
>
> In my implementation it is (or rather would be) as simple as just
> not passing 'generating_info' => \&git_generating_data_html in the
> GitwebCache::SimpleFileCache constructor.
>
At least in mine it was don't allow background caching. It would force
everyone to wait on the one process that was actually generating
content. But it means a few blank pages with a spinning working icon
until the cache releases it's exclusive lock.
- John 'Warthog9' Hawley
^ permalink raw reply
* Re: [PATCH 8/9] gitweb: Convert output to using indirect file handle
From: J.H. @ 2010-01-25 20:48 UTC (permalink / raw)
To: Jakub Narebski; +Cc: Petr Baudis, John 'Warthog9' Hawley, git
In-Reply-To: <201001250247.13101.jnareb@gmail.com>
> So there is a bit of rule of preservation of difficulty at work. Either
> we have large patch adding explicit filehandle to all print statements
> 'print <sth> -> print $out <sth>' but simple code, or have smaller patch
> but complicated *STDOUT manipulation, or have small patch but rely on
> non-core CPAN modules present.
I think depending in non-core CPAN modules is a really bad idea, and
will cause some concern and consternation with mainline distributions,
besides making it more complicated for users to get this up and running
quickly and easily. While I agree there are other ways of handling this
I think just adding the filehandle to the print statements provides the
easiest cross section of usability and functionality for everything
involved.
- John 'Warthog9' Hawley
^ permalink raw reply
* Re: [RFC PATCH 10/10] gitweb: Show appropriate "Generating..." page when regenerating cache (WIP)
From: Jakub Narebski @ 2010-01-25 20:58 UTC (permalink / raw)
To: Petr Baudis
Cc: git, John 'Warthog9' Hawley,
John 'Warthog9' Hawley
In-Reply-To: <20100125135653.GN4159@machine.or.cz>
On Mon, Jan 25, 2010 at 14:56 +0100, Petr Baudis wrote:
> On Mon, Jan 25, 2010 at 02:48:26PM +0100, Jakub Narebski wrote:
> > Now those patches (mine and J.H. both) make gitweb use locking
> > (it is IIRC configurable in J.H. patch) to make only one process
> > generate the page if it is missing from cache, or is stale. Now
> > if it is missing, we have to wait until it is generated in full
> > before being able to show it to client. While it is possible to
> > "tee" output (using PerlIO::tee, or Capture::Tiny, or tie like
> > CGI::Cache) writing it simultaneously to browser and to cache for
> > the process that is generating data, it is as far as I understand
> > it impossible for processes which are waiting for data. Therefore
> > the need for "Generating..." page, so the user does not think that
> > web server hung or something, and is not generating output.
>
> Ah, ok, so the message is there to cover up for a technical problem. ;-)
> I didn't quite realize. Then, it would be great to tweak the mechanisms
> so that the user does not really have to wait.
Well, the mechanism would certainly be configurable in final version
(current split version is more of proof of concept of splitting).
>
> So, I wonder about two things:
>
> (i) How often does it happen that two requests for the same page are
> received? Has anyone measured it? Or is at least able to make
> a minimally educated guess? IOW, isn't this premature optimization?
To be more exact the question is how often second request for the same
page appears when earlier request didn't finished processing. It is
the matter of both frequency of given requests, and time it takes to
generate request (which grows with growing load on server).
As to measurements: Pasky, do you have access logs, or their analysis
a la AWStats, Webalizer and the like, for repo.or.cz? Warthog9, do
you have access logs or analysis for git.kernel.org? Can you get similar
from fedorahosted?
> (ii) Can't the locked gitwebs do the equivalent of tail -f?
Well, it could, in principle, but it would need some changes.
First, instead of using temporary file to create cache entry atomically
(write to temporary file, then rename) the process generating data would
have to write to file other processes can read from. It could be e.g.
lockfile.
Second, there would be needed extended cache API so that generated data
is streamed to cache file, ->set($key, $data) ==> ->set($key, $fh) or
->set_io($key, $fh). This would mean some complications, but what might
be more important is that this trick would not work as far as I can see
with other caching backends / caching engines that the one from
gitweb/cache.pm (like memcached or mmap based ones).
Then the code could look like the following (in pseudocode):
try to acquire writers lock
if (acquired writers lock) {
generate and "tee" response
create cache entry
} else {
# <<<<<<
while (not acquired writers lock &&
sysread something) {
print <data>;
}
# >>>>>>
retrieve and print (rest) of data
}
where parts between <<<<<< and >>>>>> are new.
But there is another complication: gitweb needs to be able to deal with
the situation where process generating data got interrupted before
creating full output, or process generating data ran die_error which
does not generate any cache entry (e.g. if the URL we are trying to
access returns 404 not found - the check for existence of object can
take a while if the system is busy, I think).
Now in current implementation either cache entry is written in full, or it
is not written at all. It would be, I think, fairly easy to check with the
current code whether cache entry got generated when we acquired readers
lock (when the process get terminated, the lock gets released, which
is advantage over using atomic creating file with O_EXCL for locking),
and if we didn't repeat the whole process. With the "tee"/"tail"
solution if the process generating data got interrupted before end,
we can detect such situation, but currently I have no idea what should
be done in such situation. We can as easily as for the current solution
(which needs "Generating..." page for activity indicator) to detect
die_error situation, and with some care i.e. with not writing to cache
file directly we can ensure that cache entries contain full, correctly
generated data.
>
> P.S.: Again the disclaimer - if this is "too hard", it's better to
> accept patches like they are, then improve this later. But perhaps
> a better solution would be not to clutter the code by optimizing this
> case at all if it's not clear it really matters in the real world.
See above.
P.S. I have noticed that with current implementation (well, I am not sure
if it is true also for J.H. implementation) there is problem if there is
more than one process trying to request URL which result in die_error being
called. The design decision, present in original patch, was to not cache
"die_error" / non-"200 OK" pages; it seems sane, but I don't know if it
was a correct decision. The solution for interrupted generating process,
described above, works also for die_error pages, although it makes
die_error pages slower for such (hopefully rare) situation of simultaneous
errorneous request.
P.P.S. Both Pasky's approach to caching projects_list page, and Lea Wiemann
work on "gitweb caching" project for Google Summer of Code 2008 approached
caching in different way: by caching (parsed) data, not by caching output.
Note however that for some actions like 'snapshot' we would probably want
to have response/output caching anyway. Also for output caching we can
use X-Sendfile (or like) extension.
--
Jakub Narebski
Poland
^ permalink raw reply
* Re: Issues that need to be resolved before 1.7.0-rc1
From: Johannes Sixt @ 2010-01-25 21:17 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7vsk9u2g3k.fsf@alter.siamese.dyndns.org>
On Montag, 25. Januar 2010, Junio C Hamano wrote:
> There also is a discussion on //server/share path on msysgit, that was
> started since 288123f (ignore duplicated slashes in make_relative_path(),
> 2010-01-21). If that commit _breaks_ use case on msysgit that used to
> work, I think the sanest course of action is to revert it...
IFAICS, this patch is not a regression w.r.t. //server/share paths.
The discussion is about that support of //server/share paths in msysgit is
poor in general. For example, it is not possible to
git clone //server/share/repo.git
with different failure modes depending on whether forward slashes or
backslashes are used.
-- Hannes
^ permalink raw reply
* Re: Perforce vcs-helper
From: Daniel Barkalow @ 2010-01-25 21:25 UTC (permalink / raw)
To: Tor Arvid Lund; +Cc: Git mailing list
In-Reply-To: <1a6be5fa1001250257s333339fdq9a08e91fdb84fd3d@mail.gmail.com>
On Mon, 25 Jan 2010, Tor Arvid Lund wrote:
> Hi, Daniel.
>
> I remember that you posted a vcs-p4 transport helper last year... And
> I basically wondered whether or not that is something that you're
> still working on. As I understand it, the transport helper framework
> will be part of git 1.7, and I thought it would be interesting to test
> your code.
I've updated it slightly for changes in the transport helper framework,
but I haven't done too much else on it. In particular, it's still doing
the push side in a way I now don't like, and, in general, relies a bit too
much on the local side going into a git database.
> The current git-p4 script is nice, of course, but I would much like to
> try the "detect branches and integrations" stuff :-)
Sure, I'll post the latest version shortly.
-Daniel
*This .sig left intentionally blank*
^ permalink raw reply
* [PATCH not-for-mainline] Implement git-vcs-p4
From: Daniel Barkalow @ 2010-01-25 21:35 UTC (permalink / raw)
To: git; +Cc: Tor Arvid Lund
This is probably not particularly appropriate for mainline
application, and is somewhat buggy, not extensively tested, and
incomplete. The push support is also currently based on a transport helper
export design that isn't upstream and I don't like any more; a better
design is probably to have the core send an "export" command and then a
gfi stream, but I haven't worked on this.
It has two implementations of the interaction with the Perforce
server: one that uses the command-line client (and therefore makes a
ton of separate connections to the server) and one that uses the
(closed source, vaguely licensed) C++ API. The former does not support
everything used in push/submit correctly at this point.
It also adds support to the Makefile for building C++ object files and
linking with a C++ linker. It should be easy to omit entirely for
builds that don't use p4, and it's at least somewhat out of the way.
The biggest flaw currently is that it doesn't save its analysis of the
structure of the history, and doesn't have a way to push it out of memory,
so a long or complex history will run you out of memory or will take a
long time to do an incremental fetch.
Fetch features:
- following integrations (with some guessing)
- finding other branches of a codeline
Push features (only with the C++ API):
- works if you don't do anything at all complicated
Signed-off-by: Daniel Barkalow <barkalow@iabervon.org>
---
Documentation/git-vcs-p4.txt | 47 ++
Makefile | 23 +
vcs-p4/p4client-api.cc | 455 +++++++++++++++
vcs-p4/p4client.c | 158 ++++++
vcs-p4/p4client.h | 74 +++
vcs-p4/vcs-p4.c | 1250 ++++++++++++++++++++++++++++++++++++++++++
vcs-p4/vcs-p4.h | 135 +++++
7 files changed, 2142 insertions(+), 0 deletions(-)
create mode 100644 Documentation/git-vcs-p4.txt
create mode 100644 vcs-p4/p4client-api.cc
create mode 100644 vcs-p4/p4client.c
create mode 100644 vcs-p4/p4client.h
create mode 100644 vcs-p4/vcs-p4.c
create mode 100644 vcs-p4/vcs-p4.h
diff --git a/Documentation/git-vcs-p4.txt b/Documentation/git-vcs-p4.txt
new file mode 100644
index 0000000..61da8c1
--- /dev/null
+++ b/Documentation/git-vcs-p4.txt
@@ -0,0 +1,47 @@
+Config
+------
+
+vcs-p4.port::
+ The value to use for P4PORT
+
+vcs-p4.client::
+ The value to use for P4CLIENT
+
+vcs-p4.codelineformat::
+ A regular expression to match valid codelines; a codeline is a
+ directory that contains exactly those files that belong to a
+ version of a project. Importing history with integrations will
+ generally discover codelines not explicitly marked to be
+ imported, found when a file in a known codeline, whose full
+ path is therefore the codeline path plus a relative path, is
+ integrated from a file with a name that ends with that
+ relative path. However, files will sometimes be integrated
+ from non-codelines (that is, from a directory that contains
+ unrelated files whose history should not be tracked), and this
+ option can be used to ignore some directories.
+
+ Note that, properly, the history of the individual files from
+ a non-codeline which got integrated into a codeline should
+ contribute but that this is not presently supported.
+
+vcs-p4.findbranches::
+ If true, attempt to find branches of the codeline(s) specified
+ by looking for integrations out of these codelines.
+
+vcs-p4.ignorecodeline::
+ A perforce location which is a codeline, but is not relevant
+ to this project. This only applies to finding branches; a
+ codeline containing ancestors of the current codelines is
+ always imported, although it won't be given as a remote head
+ if it is ignored.
+
+remotes.*.vcs::
+ The string "p4" to use this importer.
+
+remotes.*.codeline::
+ The perforce location of a codeline to track. Other codelines
+ may be discovered by git-vcs-p4, but it will make no attempt
+ to get versions in these locations more recent than the last
+ versions that contribute at present to the tracked codelines,
+ and it will not make them available for matching in "fetch"
+ patterns.
diff --git a/Makefile b/Makefile
index 0ebf9dd..638127a 100644
--- a/Makefile
+++ b/Makefile
@@ -364,6 +364,7 @@ PROGRAMS += git-unpack-file$X
PROGRAMS += git-update-server-info$X
PROGRAMS += git-upload-pack$X
PROGRAMS += git-var$X
+PROGRAMS += git-remote-p4$X
# List built-in command $C whose implementation cmd_$C() is not in
# builtin-$C.o but is linked in as part of some other command.
@@ -1252,6 +1253,7 @@ endif
ifneq ($(findstring $(MAKEFLAGS),s),s)
ifndef V
QUIET_CC = @echo ' ' CC $@;
+ QUIET_CXX = @echo ' ' CXX $@;
QUIET_AR = @echo ' ' AR $@;
QUIET_LINK = @echo ' ' LINK $@;
QUIET_BUILT_IN = @echo ' ' BUILTIN $@;
@@ -1448,12 +1450,16 @@ git.o git.spec \
$(patsubst %.perl,%,$(SCRIPT_PERL)) \
: GIT-VERSION-FILE
+vcs-p4/%.o: ALL_CFLAGS += -I.
+
%.o: %.c GIT-CFLAGS
$(QUIET_CC)$(CC) -o $*.o -c $(ALL_CFLAGS) $<
%.s: %.c GIT-CFLAGS
$(QUIET_CC)$(CC) -S $(ALL_CFLAGS) $<
%.o: %.S
$(QUIET_CC)$(CC) -o $*.o -c $(ALL_CFLAGS) $<
+%.o: %.cc GIT-CFLAGS
+ $(QUIET_CXX)$(CXX) -o $*.o -c $(ALL_CFLAGS) $<
exec_cmd.o: exec_cmd.c GIT-CFLAGS
$(QUIET_CC)$(CC) -o $*.o -c $(ALL_CFLAGS) \
@@ -1498,6 +1504,22 @@ git-remote-http$X git-remote-https$X git-remote-ftp$X: remote-curl.o http.o http
$(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) $(filter %.o,$^) \
$(LIBS) $(CURL_LIBCURL) $(EXPAT_LIBEXPAT)
+ifdef P4API_BASE
+P4_IMPL=p4client-api
+
+vcs-p4/p4client-api.o: ALL_CFLAGS += -I$(P4API_BASE)/include
+P4_LINK=$(CXX)
+P4LIBS=-L$(P4API_BASE)/lib -lclient -lrpc -lsupp
+else
+P4_IMPL=p4client
+P4_LINK=$(CC)
+endif
+
+git-remote-p4$X: LIBS += $(P4LIBS)
+git-remote-p4$X: vcs-p4/vcs-p4.o vcs-p4/$(P4_IMPL).o $(GITLIBS)
+ $(QUIET_LINK)$(P4_LINK) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) $(filter %.o,$^) \
+ $(LIBS)
+
$(LIB_OBJS) $(BUILTIN_OBJS): $(LIB_H)
$(patsubst git-%$X,%.o,$(PROGRAMS)) git.o: $(LIB_H) $(wildcard */*.h)
builtin-revert.o wt-status.o: wt-status.h
@@ -1759,6 +1781,7 @@ distclean: clean
clean:
$(RM) *.o mozilla-sha1/*.o arm/*.o ppc/*.o compat/*.o compat/*/*.o xdiff/*.o \
+ vcs-p4/*.o \
$(LIB_FILE) $(XDIFF_LIB)
$(RM) $(ALL_PROGRAMS) $(BUILT_INS) git$X
$(RM) $(TEST_PROGRAMS)
diff --git a/vcs-p4/p4client-api.cc b/vcs-p4/p4client-api.cc
new file mode 100644
index 0000000..3ff5962
--- /dev/null
+++ b/vcs-p4/p4client-api.cc
@@ -0,0 +1,455 @@
+extern "C" {
+#include "p4client.h"
+}
+
+#include <p4/clientapi.h>
+
+class VCSClientUser : public ClientUser {
+ virtual void OutputInfo(char level, const char *data);
+ virtual void OutputBinary(const char *data, int length);
+ virtual void OutputText(const char *data, int length);
+ // Stuff that shouldn't happen
+ virtual void InputData(StrBuf *strbuf, Error *e);
+ virtual void OutputError(const char *errBuf);
+ virtual void OutputStat(StrDict *varList);
+ virtual void Prompt(const StrPtr &msg, StrBuf &rsp, int noEcho, Error *e);
+ virtual void ErrorPause(char *errBuf, Error *e);
+ virtual void Edit(FileSys *f1, Error *e);
+ virtual void Diff(FileSys *f1, FileSys *f2, int doPage,
+ char *diffFlags, Error *e);
+ virtual void Merge(FileSys *base, FileSys *leg1, FileSys *leg2,
+ FileSys *result, Error *e);
+ virtual int Resolve(ClientMerge *m, Error *e);
+ virtual void Help(const char *const *help);
+ virtual FileSys *File(FileSysType type);
+
+public:
+ void *data;
+ void (*info_cb)(void *, int, const char *);
+ void (*form_cb)(void *, const char *, const char *);
+ const char *(*form_io_cb)(void *, const char *, const char *);
+
+ void (*buffer_cb)(void *, const char *buffer, int length);
+
+ void clear() {
+ info_cb = NULL;
+ form_cb = NULL;
+ form_io_cb = NULL;
+ buffer_cb = NULL;
+ }
+
+ VCSClientUser() {
+ strbuf_init(&input, 0);
+ tree = NULL;
+ }
+
+ const unsigned char *tree;
+ const char *base;
+
+private:
+ struct strbuf input;
+};
+
+class PhonyFileSys : public FileSys {
+ virtual void Open(FileOpenMode mode, Error *e);
+ virtual void Write(const char *buf, int len, Error *e);
+ virtual int Read(char *buf, int len, Error *e);
+ virtual void Close(Error *e);
+ virtual int Stat();
+ virtual int StatModTime();
+ virtual void Truncate(Error *e);
+ virtual void Unlink(Error *e);
+ virtual void Rename(FileSys *target, Error *e);
+ virtual void Chmod(FilePerm perms, Error *e);
+ virtual void ChmodTime(Error *e);
+public:
+ PhonyFileSys(VCSClientUser *user, FileSysType type) {
+ this->user = user;
+ this->type = type;
+ }
+private:
+ VCSClientUser *user;
+ int mode;
+ const char *buffer;
+ unsigned long posn;
+ unsigned long size;
+};
+
+static ClientApi client;
+static VCSClientUser ui;
+
+void p4_init(const char *const *env)
+{
+ Error e;
+ StrBuf msg;
+
+ while (*env) {
+ if (!strncmp(*env, "P4PORT=", 7))
+ client.SetPort((*env) + 7);
+ if (!strncmp(*env, "P4CLIENT=", 7))
+ client.SetClient((*env) + 9);
+ env++;
+ }
+
+ client.Init(&e);
+ if (e.Test()) {
+ e.Fmt(&msg);
+ fprintf(stderr, msg.Text());
+ exit(1);
+ }
+}
+
+void VCSClientUser::OutputBinary(const char *buffer, int length)
+{
+ if (buffer_cb) {
+ buffer_cb(data, buffer, length);
+ } else
+ fprintf(stderr, "Unexpected binary of length %d\n", length);
+}
+
+void VCSClientUser::OutputText(const char *buffer, int length)
+{
+ if (buffer_cb) {
+ buffer_cb(data, buffer, length);
+ } else
+ fprintf(stderr, "Unexpected text of length %d\n", length);
+}
+
+void VCSClientUser::OutputInfo(char level, const char *line)
+{
+ if (info_cb)
+ info_cb(data, level - '0', line);
+ else if (form_cb || form_io_cb) {
+ struct strbuf key;
+ struct strbuf value;
+
+ strbuf_init(&key, 0);
+ strbuf_init(&value, 0);
+
+ const char *eol = NULL;
+ for (; *line; line = eol + 1) {
+ const char *eok;
+
+ eol = strchr(line, '\n');
+ if (!eol)
+ break;
+ if (eol == line || line[0] == '#')
+ continue;
+ eok = strchr(line, ':');
+ if (!eok)
+ continue;
+ strbuf_reset(&key);
+ strbuf_reset(&value);
+ strbuf_add(&key, line, eok - line);
+ if (eok[1] == '\t') {
+ strbuf_add(&value, eok + 2, eol - (eok + 2));
+ } else if (eok[1] == '\n') {
+ for (line = eol + 1; *line && line[0] != '\n'; line = eol + 1) {
+ eol = strchr(line, '\n');
+ strbuf_add(&value, line + 1, eol - (line + 1) + 1);
+ }
+ }
+ if (form_cb)
+ form_cb(data, key.buf, value.buf);
+ else {
+ const char *new_value = form_io_cb(data, key.buf, value.buf);
+ if (new_value) {
+ strbuf_addbuf(&input, &key);
+ strbuf_addch(&input, ':');
+ if (strchr(new_value, '\n')) {
+ const char *posn = new_value;
+ while (posn) {
+ const char *eol = strchr(posn, '\n');
+ strbuf_addstr(&input, "\n\t");
+ if (eol) {
+ strbuf_add(&input, posn, eol - posn);
+ posn = eol + 1;
+ } else {
+ strbuf_addstr(&input, posn);
+ break;
+ }
+ }
+ } else {
+ strbuf_addch(&input, ' ');
+ strbuf_addstr(&input, new_value);
+ }
+ strbuf_addch(&input, '\n');
+ }
+ }
+ }
+ strbuf_release(&key);
+ strbuf_release(&value);
+ } else
+ fprintf(stderr, "Unexpected info: %c ... %s\n", level, line);
+}
+
+void VCSClientUser::InputData(StrBuf *strbuf, Error *e)
+{
+ strbuf->Append(input.buf, input.len);
+ strbuf_reset(&input);
+ //fprintf(stderr, "Unexpected input data\n");
+}
+
+void VCSClientUser::OutputError(const char *errBuf)
+{
+ fprintf(stderr, "Error output: %s\n", errBuf);
+}
+
+void VCSClientUser::OutputStat(StrDict *varList)
+{
+ fprintf(stderr, "Unexpected stat\n");
+}
+
+void VCSClientUser::Prompt(const StrPtr &msg, StrBuf &rsp,
+ int noEcho, Error *e)
+{
+ fprintf(stderr, "Prompt\n");
+}
+
+void VCSClientUser::ErrorPause(char *errBuf, Error *e)
+{
+ fprintf(stderr, "Error pause from p4\n");
+}
+
+void VCSClientUser::Edit(FileSys *f1, Error *e)
+{
+ fprintf(stderr, "Edit request from p4\n");
+}
+
+void VCSClientUser::Diff(FileSys *f1, FileSys *f2, int doPage,
+ char *diffFlags, Error *e)
+{
+ fprintf(stderr, "Diff from p4\n");
+}
+
+void VCSClientUser::Merge(FileSys *base, FileSys *leg1, FileSys *leg2,
+ FileSys *result, Error *e)
+{
+ fprintf(stderr, "Merge from p4\n");
+}
+
+int VCSClientUser::Resolve(ClientMerge *m, Error *e)
+{
+ fprintf(stderr, "Resolve from p4\n");
+ m->Select(CMS_MERGED, e);
+ return CMS_MERGED;
+}
+
+void VCSClientUser::Help(const char *const *help)
+{
+ fprintf(stderr, "Help from p4\n");
+}
+
+FileSys *VCSClientUser::File(FileSysType type)
+{
+ FileSys *ret;
+ fprintf(stderr, "File from p4: %d\n", type);
+ ret = new PhonyFileSys(this, type);
+ return ret;
+}
+
+void PhonyFileSys::Open(FileOpenMode mode, Error *e)
+{
+ const char *openpath = path.Text();
+ fprintf(stderr, "Open %s for %d\n", openpath, mode);
+ if (mode == FOM_READ) {
+ if (strncmp(openpath, user->base, strlen(user->base))) {
+ e->Sys("Path outside of working directory", openpath);
+ return;
+ }
+ openpath = openpath + strlen(user->base);
+ if (openpath[0] == '/')
+ openpath++;
+ unsigned filemode;
+ if (!user->tree)
+ e->Sys("Unable to read file", openpath);
+ buffer = get_tree_path(user->tree, openpath, &filemode, &size);
+ if (!buffer)
+ e->Sys("File not found", openpath);
+ posn = 0;
+ }
+}
+
+void PhonyFileSys::Write(const char *buf, int len, Error *e)
+{
+}
+
+int PhonyFileSys::Read(char *buf, int len, Error *e)
+{
+ if (len + posn > size)
+ len = size - posn;
+ memcpy(buf, buffer + posn, len);
+ posn += len;
+ return len;
+}
+
+void PhonyFileSys::Close(Error *e)
+{
+}
+
+int PhonyFileSys::Stat()
+{
+ const char *openpath = path.Text();
+ unsigned filemode;
+ fprintf(stderr, "Stat %s\n", openpath);
+ if (!user->tree)
+ return 0;
+ if (strncmp(openpath, user->base, strlen(user->base))) {
+ return 0;
+ }
+ openpath = openpath + strlen(user->base);
+ if (openpath[0] == '/')
+ openpath++;
+ buffer = get_tree_path(user->tree, openpath, &filemode, &size);
+ if (!buffer)
+ return 0;
+ fprintf(stderr, "Exists\n");
+ return FSF_EXISTS;
+}
+
+int PhonyFileSys::StatModTime()
+{
+ const char *openpath = path.Text();
+ fprintf(stderr, "Stat modtime %s\n", openpath);
+ return 0;
+}
+
+void PhonyFileSys::Truncate(Error *e)
+{
+ fprintf(stderr, "truncate\n");
+}
+
+void PhonyFileSys::Unlink(Error *e)
+{
+ fprintf(stderr, "unlink\n");
+}
+
+void PhonyFileSys::Rename(FileSys *target, Error *e)
+{
+ fprintf(stderr, "rename %s to %s\n", path.Text(), target->Path()->Text());
+}
+
+void PhonyFileSys::Chmod(FilePerm perms, Error *e)
+{
+ fprintf(stderr, "chmod\n");
+}
+
+void PhonyFileSys::ChmodTime(Error *e)
+{
+ fprintf(stderr, "chmodtime\n");
+}
+
+int p4_call(int fds[], const char *arg0, int argc, char *const *argv)
+{
+ ui.data = NULL;
+ ui.clear();
+ client.SetArgv(argc, argv);
+ client.Run(arg0, &ui);
+ p4_fini();
+ exit(1);
+ return 0;
+}
+
+int _p4_call_unknown(const char *arg0, int argc, char *const *argv)
+{
+ ui.clear();
+ ui.data = NULL;
+ client.SetArgv(argc, argv);
+ client.Run(arg0, &ui);
+ return 0;
+}
+
+int _p4_call_info(const char *arg0, int argc, char *const *argv,
+ void *data,
+ void (*cb)(void *data, int level, const char *line))
+{
+ ui.clear();
+ ui.data = data;
+ ui.info_cb = cb;
+ client.SetArgv(argc, argv);
+ client.Run(arg0, &ui);
+ return 0;
+}
+
+int _p4_call_form(const char *arg0, int argc, char *const *argv,
+ void *data,
+ void (*cb)(void *data, const char *key, const char *value))
+{
+ char **new_argv = (char **)calloc(argc + 2, sizeof(*new_argv));
+ int i;
+ new_argv[0] = "-o";
+ for (i = 0; i < argc; i++)
+ new_argv[i + 1] = argv[i];
+ new_argv[argc + 1] = NULL;
+ ui.clear();
+ ui.data = data;
+ ui.form_cb = cb;
+ client.SetArgv(argc + 1, new_argv);
+ client.Run(arg0, &ui);
+ return 0;
+}
+
+int _p4_call_form_io(const char *arg0, int argc, char *const *argv, void *data,
+ const char *(*cb)(void *data, const char *key, const char *value))
+{
+ char **new_argv = (char **)calloc(argc + 2, sizeof(*new_argv));
+ int i;
+ new_argv[0] = "-o";
+ for (i = 0; i < argc; i++)
+ new_argv[i + 1] = argv[i];
+ new_argv[argc + 1] = NULL;
+ ui.clear();
+ ui.data = data;
+ ui.form_io_cb = cb;
+ client.SetArgv(argc + 1, new_argv);
+ client.Run(arg0, &ui);
+ return 0;
+}
+
+int _p4_call_buffer(const char *arg0, int argc, char *const *argv,
+ void *data,
+ void (*cb)(void *data, const char *buffer, int length))
+{
+ ui.clear();
+ ui.data = data;
+ ui.buffer_cb = cb;
+ client.SetArgv(argc, argv);
+ client.Run(arg0, &ui);
+ return 0;
+}
+
+void p4_write_blob(const char *base, unsigned mode, const unsigned char *sha1,
+ const char *path)
+{
+}
+
+void p4_write_tree(const char *base, const unsigned char *sha1)
+{
+ ui.base = base;
+ ui.tree = sha1;
+}
+
+void p4_release_tree(void)
+{
+ ui.base = NULL;
+ ui.tree = NULL;
+}
+
+int p4_complete()
+{
+ return 0;
+}
+
+int p4_fini()
+{
+ Error e;
+ StrBuf msg;
+
+ client.Final(&e);
+ if (e.Test()) {
+ e.Fmt(&msg);
+ fprintf(stderr, msg.Text());
+ exit(1);
+ }
+ return 0;
+}
diff --git a/vcs-p4/p4client.c b/vcs-p4/p4client.c
new file mode 100644
index 0000000..44f21da
--- /dev/null
+++ b/vcs-p4/p4client.c
@@ -0,0 +1,158 @@
+#include "p4client.h"
+
+#include "cache.h"
+#include "run-command.h"
+
+static const char *const *envp;
+
+void p4_init(const char *const *env)
+{
+ envp = env;
+}
+
+static struct child_process child;
+
+int p4_call(int fds[], const char *arg0, int argc, char *const *argv)
+{
+ int i;
+ memset(&child, 0, sizeof(child));
+ if (fds) {
+ child.in = -1;
+ child.out = -1;
+ } else {
+ child.no_stdin = 1;
+ child.no_stdout = 1;
+ }
+ child.err = 0;
+ child.argv = xcalloc(argc + 3, sizeof(*argv));
+ child.argv[0] = "p4";
+ child.argv[1] = arg0;
+ child.env = envp;
+ for (i = 0; i < argc; i++)
+ child.argv[i + 2] = argv[i];
+ child.argv[argc + 2] = NULL;
+ start_command(&child);
+ if (fds) {
+ fds[0] = child.in;
+ fds[1] = child.out;
+ }
+ return 0;
+}
+
+int _p4_call_info(const char *arg0, int argc, char *const *argv, void *data,
+ void (*cb)(void *data, int level, const char *line))
+{
+ int fds[2];
+ struct strbuf line;
+ FILE *input;
+
+ if (p4_call(fds, arg0, argc, argv))
+ return -1;
+
+ strbuf_init(&line, 0);
+ input = fdopen(fds[1], "r");
+ while (!strbuf_getline(&line, input, '\n')) {
+ int level = 0;
+ char *posn = line.buf;
+ while (!prefixcmp(posn, "... ")) {
+ posn += 4;
+ level++;
+ }
+ cb(data, level, posn);
+ }
+ p4_complete();
+ return 0;
+}
+
+int _p4_call_form(const char *arg0, int argc, char *const *argv, void *data,
+ void (*cb)(void *data, const char *key, const char *value))
+{
+ int fds[2];
+ struct strbuf line;
+ struct strbuf key;
+ struct strbuf value;
+ FILE *input;
+
+ if (p4_call(fds, arg0, argc, argv))
+ return -1;
+
+ strbuf_init(&line, 0);
+ strbuf_init(&key, 0);
+ strbuf_init(&value, 0);
+ input = fdopen(fds[1], "r");
+ for (; !strbuf_getline(&line, input, '\n'); strbuf_reset(&line)) {
+ const char *eok;
+
+ if (!line.buf[0] || line.buf[0] == '#')
+ continue;
+ eok = strchr(line.buf, ':');
+ if (!eok)
+ continue;
+ strbuf_reset(&key);
+ strbuf_reset(&value);
+ strbuf_add(&key, line.buf, eok - line.buf);
+ if (eok[1] == '\t')
+ strbuf_addstr(&value, eok + 2);
+ else {
+ strbuf_reset(&line);
+ while (!strbuf_getline(&line, input, '\n') && line.len) {
+ strbuf_addstr(&value, line.buf + 1);
+ strbuf_addch(&value, '\n');
+ strbuf_reset(&line);
+ }
+ }
+ cb(data, key.buf, value.buf);
+ }
+ p4_complete();
+ return 0;
+}
+
+int _p4_call_buffer(const char *arg0, int argc, char *const *argv, void *data,
+ void (*cb)(void *data, const char *buffer, int len))
+{
+ int fds[2];
+ struct strbuf block;
+ if (p4_call(fds, arg0, argc, argv))
+ return -1;
+ strbuf_init(&block, 0);
+ strbuf_read(&block, fds[1], 0);
+ cb(data, block.buf, block.len);
+ p4_complete();
+ return 0;
+}
+
+
+void p4_write_blob(const char *base, unsigned mode, const unsigned char *sha1,
+ const char *path)
+{
+ struct strbuf buf;
+ void *content;
+ enum object_type type;
+ unsigned long size;
+ int fd;
+
+ strbuf_init(&buf, 0);
+ strbuf_addf(&buf, "%s/%s", base, path);
+ content = read_sha1_file(sha1, &type, &size);
+ fd = open(buf.buf, O_WRONLY | O_CREAT, (mode & 0100) ? 0666 : 0777);
+ if (fd < 0) {
+ die("Got err %d", errno);
+ }
+ write_or_die(fd, content, size);
+ return 0;
+}
+
+int p4_complete(void)
+{
+ if (!child.no_stdin)
+ close(child.in);
+ if (!child.no_stdout)
+ close(child.out);
+ finish_command(&child);
+ return 0;
+}
+
+int p4_fini(void)
+{
+ return 0;
+}
diff --git a/vcs-p4/p4client.h b/vcs-p4/p4client.h
new file mode 100644
index 0000000..f0d0ded
--- /dev/null
+++ b/vcs-p4/p4client.h
@@ -0,0 +1,74 @@
+#ifndef P4CLIENT_H
+#define P4CLIENT_H
+
+#include <stdlib.h>
+#include <stdio.h>
+#include <string.h>
+#include "strbuf.h"
+
+/**
+ * buffer: print
+ * form: change
+ * info: filelog, where, sync
+ **/
+
+void p4_init(const char *const *env);
+
+int p4_call(int fds[], const char *arg0, int argc, char *const *argv);
+
+/** Calls back with a bunch of (data, level, line) **/
+#define p4_call_info(arg0, argc, argv, data, cb) \
+ (0 ? ((*(cb))((data), 0, NULL), 1) : \
+ _p4_call_info(arg0, argc, argv, (void *)data, (void (*)(void *, int, const char *)) cb))
+
+/** Calls back with a bunch of (data, key, value); implies "-o" **/
+#define p4_call_form(arg0, argc, argv, data, cb) \
+ (0 ? ((*(cb))((data), NULL, NULL), 1) : \
+ _p4_call_form(arg0, argc, argv, (void *)data, (void (*)(void *, const char *, const char *)) cb))
+
+/** Calls back with a bunch of (data, key, value); implies "-o", and
+ * is followed by sending back form data with "-i" automatically.
+ **/
+#define p4_call_form_io(arg0, argc, argv, data, cb) \
+ (0 ? ({ __attribute__((unused)) const char *r = (*(cb))((data), NULL, NULL); 1;}) : \
+ _p4_call_form_io(arg0, argc, argv, (void *)data, (const char *(*)(void *, const char *, const char *)) cb))
+
+/** Calls back with a bunch of (data, buffer, len) **/
+#define p4_call_buffer(arg0, argc, argv, data, cb) \
+ (0 ? ((*(cb))((data), NULL, 0), 1) : \
+ _p4_call_buffer(arg0, argc, argv, (void *)data, (void (*)(void *, const char *, int)) cb))
+
+#define p4_call_unknown(arg0, argc, argv) _p4_call_unknown(arg0, argc, argv)
+
+/** One or the other of p4_write_tree and p4_write_blob will
+ * actually be effective.
+ **/
+void p4_write_tree(const char *base, const unsigned char *sha1);
+
+const char *get_tree_path(const unsigned char *sha1, const char *path,
+ unsigned *mode, unsigned long *size);
+
+void p4_write_blob(const char *base, unsigned mode, const unsigned char *sha1,
+ const char *path);
+
+void p4_release_tree(void);
+
+int p4_complete();
+
+int p4_fini();
+
+int _p4_call_info(const char *arg0, int argc, char *const *argv, void *data,
+ void (*cb)(void *data, int level, const char *line));
+
+int _p4_call_form(const char *arg0, int argc, char *const *argv, void *data,
+ void (*cb)(void *data, const char *key, const char *value));
+
+int _p4_call_form_io(const char *arg0, int argc, char *const *argv, void *data,
+ const char *(*cb)(void *data, const char *key, const char *value));
+
+int _p4_call_buffer(const char *arg0, int argc, char *const *argv, void *data,
+ void (*cb)(void *data, const char *buffer, int len));
+
+int _p4_call_unknown(const char *arg0, int argc, char *const *argv);
+
+#endif
diff --git a/vcs-p4/vcs-p4.c b/vcs-p4/vcs-p4.c
new file mode 100644
index 0000000..1b9147c
--- /dev/null
+++ b/vcs-p4/vcs-p4.c
@@ -0,0 +1,1250 @@
+#include "cache.h"
+#include "vcs-p4.h"
+#include "strbuf.h"
+#include "remote.h"
+#include "commit.h"
+#include "tree.h"
+#include "tree-walk.h"
+#include "diff.h"
+
+#include "p4client.h"
+
+#include <string.h>
+
+/** Should we try to find codelines that branch off of the relevant
+ * ones, for future reference? This lets us find new things in
+ * ls-remote without making the user tell us.
+ **/
+static int find_new_codelines;
+
+static int ignore_codeline_nr;
+static int ignore_codeline_alloc;
+static char **ignore_codelines;
+
+static int prints_done = 0;
+
+static regex_t *codeline_regex;
+
+#define CODELINE_TAG "Codeline: "
+#define CHANGE_TAG "Changelist: "
+
+#define LIST_P4_OPERATIONS 0
+
+/** List functions **/
+
+static void add_to_revision_list(struct p4_revision_list **list,
+ struct p4_revision *revision)
+{
+ while (*list)
+ list = &(*list)->next;
+ *list = xcalloc(1, sizeof(**list));
+ (*list)->revision = revision;
+}
+
+static struct p4_revision_list *copy_revision_list(struct p4_revision_list *lst)
+{
+ struct p4_revision_list *ret, **posn = &ret;
+ while (lst) {
+ *posn = xcalloc(1, sizeof(**posn));
+ (*posn)->revision = lst->revision;
+ posn = &((*posn)->next);
+ lst = lst->next;
+ }
+ return ret;
+}
+
+/** Functions to find or create representations **/
+
+static struct p4_depot *get_depot(void)
+{
+ struct p4_depot *depot = xcalloc(1, sizeof(*depot));
+ depot->next_mark = 1;
+ return depot;
+}
+
+static void add_mapped_changeset(struct p4_depot *depot, struct commit *commit,
+ struct p4_changeset *change)
+{
+ change->original = commit;
+ ALLOC_GROW(depot->added, depot->added_nr + 1, depot->added_alloc);
+ depot->added[depot->added_nr++] = change;
+}
+
+static struct p4_changeset *get_changeset(struct p4_codeline *codeline,
+ long number);
+
+static char *codeline_to_refname(const char *path) {
+ struct strbuf buf;
+ if (prefixcmp(path, "//"))
+ return NULL;
+ strbuf_init(&buf, 0);
+ strbuf_addf(&buf, "refs/p4/%s", path + 2);
+ return strbuf_detach(&buf, NULL);
+}
+
+static char *refname_to_codeline(const char *refname) {
+ struct strbuf buf;
+ if (prefixcmp(refname, "refs/p4/"))
+ return NULL;
+ strbuf_init(&buf, 0);
+ strbuf_addf(&buf, "//%s", refname + strlen("refs/p4/"));
+ return strbuf_detach(&buf, NULL);
+}
+
+static struct p4_codeline *get_codeline(struct p4_depot *depot, const char *path)
+{
+ struct p4_codeline **posn, *codeline;
+ int i;
+ unsigned char sha1[20];
+
+ if (codeline_regex && regexec(codeline_regex, path, 0, NULL, 0))
+ return NULL;
+
+ for (posn = &depot->codelines; *posn; posn = &(*posn)->next)
+ if (!strcmp(path, (*posn)->path))
+ return *posn;
+ codeline = xcalloc(1, sizeof(*codeline));
+ codeline->depot = depot;
+ codeline->path = xstrdup(path);
+
+ for (i = 0; i < ignore_codeline_nr; i++)
+ if (!strcmp(path, ignore_codelines[i]))
+ codeline->ignore = 1;
+
+ codeline->refname = codeline_to_refname(path);
+ if (!get_sha1(codeline->refname, sha1)) {
+ struct commit *commit = lookup_commit(sha1);
+ char *field;
+ parse_commit(commit);
+ field = strstr(commit->buffer, CHANGE_TAG);
+ if (!field) {
+ fprintf(stderr, "Couldn't find changeset line in commit\n");
+ } else {
+ struct p4_changeset *changeset;
+ codeline->finished_changeset =
+ atoi(field + strlen(CHANGE_TAG));
+ changeset = get_changeset(codeline, codeline->finished_changeset);
+ changeset->commit = commit;
+ codeline->history = changeset;
+ }
+ }
+ *posn = codeline;
+ return codeline;
+}
+
+static struct p4_codeline *find_codeline(struct p4_depot *depot, const char *path)
+{
+ struct p4_codeline **posn;
+ for (posn = &depot->codelines; *posn; posn = &(*posn)->next)
+ if (!prefixcmp(path, (*posn)->path))
+ return *posn;
+ return NULL;
+}
+
+/** Inserts the changeset at the right place in order for the codeline **/
+static struct p4_changeset *get_changeset(struct p4_codeline *codeline,
+ long number)
+{
+ struct p4_changeset **posn = &codeline->changesets;
+ struct p4_changeset *changeset, *prev = NULL;
+ while (*posn && (*posn)->number < number) {
+ prev = *posn;
+ posn = &(*posn)->next;
+ }
+ if (*posn && (*posn)->number == number)
+ return *posn;
+ //printf("# add changeset %lu in %s\n", number, codeline->path);
+ changeset = xcalloc(1, sizeof(*changeset));
+ changeset->codeline = codeline;
+ changeset->next = *posn;
+ changeset->previous = prev;
+ if (changeset->next)
+ changeset->next->previous = changeset;
+ else
+ codeline->head = changeset;
+ *posn = changeset;
+ changeset->number = number;
+ codeline->num_changesets++;
+ return changeset;
+}
+
+static struct p4_changeset *changeset_from_commit(struct p4_depot *depot,
+ struct commit *commit)
+{
+ int i;
+ unsigned long number = 0;
+ char *codeline = NULL, *field;
+ parse_commit(commit);
+ field = strstr(commit->buffer, CHANGE_TAG);
+ if (field)
+ number = atoi(field + strlen(CHANGE_TAG));
+ field = strstr(commit->buffer, CODELINE_TAG);
+ if (field) {
+ char *end;
+ codeline = field + strlen(CODELINE_TAG);
+ end = strchr(codeline, '\n');
+ if (end)
+ *end = '\0';
+ }
+ if (number && codeline)
+ return get_changeset(get_codeline(depot, codeline), number);
+ for (i = 0; i < depot->added_nr; i++) {
+ if (depot->added[i]->original == commit)
+ return depot->added[i];
+ }
+ return NULL;
+}
+
+static struct p4_file *get_file_by_full(struct p4_codeline *codeline,
+ const char *fullpath)
+{
+ const char *rel = fullpath + strlen(codeline->path);
+ struct p4_file **posn;
+ for (posn = &codeline->files; *posn; posn = &(*posn)->next) {
+ if (!strcmp((*posn)->name, rel))
+ return *posn;
+ }
+ *posn = xcalloc(1, sizeof(**posn));
+ (*posn)->codeline = codeline;
+ (*posn)->name = xstrdup(rel);
+ return *posn;
+}
+
+static struct p4_file *get_related_file(struct p4_file *base, const char *path)
+{
+ int basenamelen = strlen(base->name);
+ int reldirlen = strlen(path) - basenamelen;
+ struct p4_codeline *codeline;
+ if (reldirlen > 0 && !strcmp(path + reldirlen, base->name)) {
+ /* File with the same name in another codeline */
+ char *other = xstrndup(path, reldirlen);
+ //printf("# find %s in %s\n", path, other);
+ codeline = get_codeline(base->codeline->depot, other);
+ free(other);
+ if (codeline)
+ return get_file_by_full(codeline, path);
+ return NULL;
+ }
+ codeline = find_codeline(base->codeline->depot, path);
+ if (codeline) {
+ /* File with a different name in some known codeline */
+ return get_file_by_full(codeline, path);
+ }
+ fprintf(stderr, "Failed to identify %s\n", path);
+ /* Not in any known codeline; need to recheck this after
+ * discovering codelines completes.
+ */
+ return NULL;
+}
+
+static struct p4_revision *get_revision(struct p4_file *file, unsigned number)
+{
+ struct p4_revision **posn;
+ struct p4_revision *revision;
+ for (posn = &file->revisions; *posn && (*posn)->number < number;
+ posn = &(*posn)->next)
+ ;
+ if (!*posn || (*posn)->number != number) {
+ revision = xcalloc(1, sizeof(*revision));
+ revision->next = *posn;
+ *posn = revision;
+ revision->number = number;
+ revision->file = file;
+ }
+ return *posn;
+}
+
+static int parse_p4_date(const char *date)
+{
+ struct tm tm;
+ memset(&tm, 0, sizeof(tm));
+ tm.tm_year = strtol(date, NULL, 10) - 1900;
+ tm.tm_mon = strtol(date + 5, NULL, 10) - 1;
+ tm.tm_mday = strtol(date + 8, NULL, 10);
+ tm.tm_hour = strtol(date + 11, NULL, 10);
+ tm.tm_min = strtol(date + 14, NULL, 10);
+ tm.tm_sec = strtol(date + 17, NULL, 10);
+ return mktime(&tm);
+}
+
+static int is_keyword(const char *text, int keywords)
+{
+ if (!prefixcmp(text, "Id: ") ||
+ !prefixcmp(text, "Header: "))
+ return 1;
+ if (keywords == 1)
+ return 0;
+ return !prefixcmp(text, "Date: ") ||
+ !prefixcmp(text, "DateTime: ") ||
+ !prefixcmp(text, "Change: ") ||
+ !prefixcmp(text, "File: ") ||
+ !prefixcmp(text, "Revision: ") ||
+ !prefixcmp(text, "Author: ");
+}
+
+static void handle_keywords(struct strbuf *buf, int keywords)
+{
+ int posn = 0;
+ char *keyword;
+
+ if (!keywords)
+ return;
+
+ do {
+ keyword = strchr(buf->buf + posn, '$');
+ if (!keyword)
+ break;
+ if (!is_keyword(keyword + 1, keywords)) {
+ posn = keyword - buf->buf + 1;
+ continue;
+ }
+ char *eok = strchr(keyword + 1, ':');
+ size_t kwl = strcspn(eok, "$\n");
+ if (!eok[kwl])
+ break;
+ if (eok[kwl] == '$') {
+ strbuf_remove(buf, eok - buf->buf, kwl);
+ } else {
+ posn = eok - buf->buf + kwl + 1;
+ }
+ } while (1);
+}
+
+static const char *get_file_type(const char *text, size_t len)
+{
+ if (len == 5 && !memcmp(text, "ktext", 5))
+ return "text+k";
+ if (len == 5 && !memcmp(text, "xtext", 5))
+ return "text+x";
+ if (len == 6 && !memcmp(text, "kxtext", 6))
+ return "text+kx";
+ return xstrndup(text, len);
+}
+
+static const char *get_file_mode(const char *type)
+{
+ char *p = strchr(type, '+');
+ if (!strcmp(type, "symlink"))
+ return "120000";
+ if (p && strchr(p, 'x'))
+ return "100755";
+ return "100644";
+}
+
+static int keywords(const char *type)
+{
+ char *p = strchr(type, '+');
+ if (p) {
+ char *k = strchr(p, 'k');
+ if (k) {
+ if (k[1] == 'o')
+ return 1;
+ return 2;
+ }
+ }
+ return 0;
+}
+
+static void output_data(struct strbuf *buf)
+{
+ printf("data %d\n", buf->len);
+ fwrite(buf->buf, 1, buf->len, stdout);
+ printf("\n");
+}
+
+/** P4 operations **/
+
+static void set_working(struct p4_codeline *codeline, int level, const char *line)
+{
+ char *working = strrchr(line, ' ');
+ if (working)
+ codeline->working = xstrdup(working + 1);
+}
+
+static int p4_where(struct p4_codeline *codeline)
+{
+ char *argv[1];
+ struct strbuf buf;
+
+ strbuf_init(&buf, 0);
+ strbuf_addstr(&buf, codeline->path);
+ argv[0] = buf.buf;
+ p4_call_info("where", 1, argv, codeline, set_working);
+ return codeline->working ? 0 : -1;
+}
+
+static void sync_cb(void *data, int level, const char *line)
+{
+ //fprintf(stderr, "%s\n", line);
+}
+
+static void p4_sync(struct p4_changeset *changeset)
+{
+ char *argv[1];
+ struct strbuf buf;
+
+ printf("progress syncing %s/...\n", changeset->codeline->working);
+ strbuf_init(&buf, 0);
+ strbuf_addf(&buf, "%s/...@%lu",
+ changeset->codeline->working, changeset->codeline->head->number);
+ argv[0] = buf.buf;
+ p4_call_info("sync", 1, argv, NULL, sync_cb);
+}
+
+static void p4_integrate(struct p4_codeline *codeline,
+ struct p4_changeset *side)
+{
+ char *argv[3];
+ struct strbuf buf;
+ strbuf_init(&buf, 0);
+ strbuf_addf(&buf, "%s/...@%lu", side->codeline->path, side->number);
+ argv[0] = "-d";
+ argv[1] = strbuf_detach(&buf, 0);
+ strbuf_addf(&buf, "%s/...", codeline->path);
+ argv[2] = buf.buf;
+ p4_call_unknown("integrate", 3, argv);
+ free(argv[1]);
+ strbuf_release(&buf);
+}
+
+static void p4_resolve(void)
+{
+ p4_call_unknown("resolve", 0, NULL);
+}
+
+static void p4_edit(struct p4_codeline *codeline, const char *path)
+{
+ char *argv[1];
+ struct strbuf buf;
+
+ strbuf_init(&buf, 0);
+ strbuf_addf(&buf, "%s%s", codeline->working, path);
+ argv[0] = buf.buf;
+ p4_call_unknown("edit", 1, argv);
+ strbuf_release(&buf);
+}
+
+static void p4_add(struct p4_codeline *codeline, const char *path)
+{
+ char *argv[1];
+ struct strbuf buf;
+
+ strbuf_init(&buf, 0);
+ strbuf_addf(&buf, "%s%s", codeline->working, path);
+ argv[0] = buf.buf;
+ p4_call(NULL, "add", 1, argv);
+ strbuf_release(&buf);
+ p4_complete();
+}
+
+static void p4_delete(struct p4_codeline *codeline, const char *path)
+{
+ char *argv[1];
+ struct strbuf buf;
+
+ strbuf_init(&buf, 0);
+ strbuf_addf(&buf, "%s%s", codeline->working, path);
+ argv[0] = buf.buf;
+ p4_call(NULL, "delete", 1, argv);
+ strbuf_release(&buf);
+ p4_complete();
+}
+
+static const char *change_cb(struct commit *commit, const char *key, const char *value)
+{
+ fprintf(stderr, "Form for %s\n", key);
+ if (!strcmp(key, "Description")) {
+ const char *message = strstr(commit->buffer, "\n\n");
+ if (message)
+ message += 2;
+ fprintf(stderr, "Return %s\n", message);
+ return message;
+ }
+ return value;
+}
+
+static void submit_cb(unsigned long *data, int level, const char *line)
+{
+ int len = strlen(line);
+ if (!prefixcmp(line, "Change ") && len > 18 &&
+ !strncmp(line + len - strlen(" submitted."), " submitted.",
+ strlen(" submitted.")))
+ *data = atoi(line + 7);
+}
+
+static unsigned long p4_submit(struct commit *commit)
+{
+ unsigned long ret;
+ char *argv[1];
+ p4_call_form_io("change", 0, NULL, commit, change_cb);
+ argv[0] = "-i";
+ p4_call_info("submit", 1, argv, &ret, submit_cb);
+ return ret;
+}
+
+static void p4_print(struct p4_revision *revision)
+{
+ char *argv[2];
+ struct strbuf line;
+ strbuf_init(&line, 0);
+ strbuf_addf(&line, "%s%s#%lu",
+ revision->file->codeline->path,
+ revision->file->name, revision->number);
+ argv[1] = strdup(line.buf);
+ argv[0] = "-q";
+
+ if (LIST_P4_OPERATIONS)
+ fprintf(stderr, "p4 print\n");
+
+ strbuf_reset(&line);
+
+ p4_call_buffer("print", 2, argv, &line, strbuf_add);
+
+ free(argv[1]);
+
+ handle_keywords(&line, keywords(revision->type));
+
+ /* Perforce puts a newline at the end when printing symlinks */
+ if (!strcmp(revision->type, "symlink"))
+ line.len--;
+
+ output_data(&line);
+
+ strbuf_release(&line);
+
+ prints_done++;
+}
+
+struct p4_change_data {
+ struct p4_changeset *changeset;
+ int date;
+ char *user;
+ struct strbuf message;
+};
+
+static void p4_change_cb(struct p4_change_data *data, const char *key,
+ const char *value)
+{
+ if (!strcmp(key, "User"))
+ data->user = xstrdup(value);
+ else if (!strcmp(key, "Date"))
+ data->date = parse_p4_date(value);
+ else if (!strcmp(key, "Description"))
+ strbuf_addstr(&data->message, value);
+}
+
+static void p4_change(struct p4_changeset *changeset)
+{
+ char *argv[1];
+ struct strbuf line;
+
+ struct p4_change_data data = {
+ .changeset = changeset,
+ .date = 0,
+ .user = NULL,
+ };
+
+ if (LIST_P4_OPERATIONS)
+ fprintf(stderr, "p4 change\n");
+
+ strbuf_init(&data.message, 0);
+
+ strbuf_init(&line, 0);
+ strbuf_addf(&line, "%lu", changeset->number);
+ argv[0] = line.buf;
+ p4_call_form("change", 1, argv, &data, p4_change_cb);
+ strbuf_release(&line);
+
+ printf("committer %s <%s> %d +0000\n",
+ data.user, data.user, data.date);
+ free(data.user);
+
+ strbuf_addf(&data.message,
+ "\n" CODELINE_TAG "%s\n" CHANGE_TAG "%lu\n",
+ changeset->codeline->path, changeset->number);
+ output_data(&data.message);
+ strbuf_release(&data.message);
+}
+
+struct p4_filelog_data {
+ struct p4_codeline *codeline;
+ struct p4_file *file;
+ struct p4_revision *revision;
+};
+
+static void p4_filelog_cb(struct p4_filelog_data *data,
+ char level, const char *line)
+{
+ if (level == 0) {
+ data->file = get_file_by_full(data->codeline, line);
+ } else if (level == 1) {
+ int rev, change, delete = 0, branch = 0;
+ char *posn;
+ rev = strtoul(line + 1, &posn, 10); /* skip the '#' */
+ posn += strlen(" change ");
+ change = strtoul(posn, &posn, 10);
+ if (!prefixcmp(posn, " delete"))
+ delete = 1;
+ if (!prefixcmp(posn, " branch"))
+ branch = 1;
+ posn = strchr(posn, '(') + 1;
+ data->revision = get_revision(data->file, rev);
+ data->revision->changeset =
+ get_changeset(data->codeline, change);
+ data->revision->type = get_file_type(posn,
+ strchr(posn, ')') - posn);
+ data->revision->delete = delete;
+ data->revision->branch = branch;
+ add_to_revision_list(&data->revision->changeset->revisions,
+ data->revision);
+ } else if (level == 2) {
+ const char *path;
+ int rev, from = 0;
+ char *type = xstrdup(line);
+ char *posn = strrchr(type, ' ') + 1;
+
+ from = (!prefixcmp(type, "ignored") &&
+ posn == type + strlen("ignored") + 1) ||
+ !prefixcmp(strchr(type, ' '), " from");
+
+ path = posn;
+ posn = strchr(posn, '#');
+ *(posn++) = '\0';
+ do {
+ /* ???? What does a list of revisions mean? */
+ rev = strtoul(posn, &posn, 10);
+ if (*posn != ',')
+ break;
+ posn += 2;
+ } while (1);
+ if (from) {
+ struct p4_file *rel_file =
+ get_related_file(data->file, path);
+ if (!rel_file) {
+ /*
+ printf("# Couldn't find %s related to %s %s\n",
+ path, data->file->codeline->path,
+ data->file->name);
+ */
+ }
+ if (rel_file && rel_file->codeline != data->codeline)
+ add_to_revision_list(&data->revision->integrated,
+ get_revision(rel_file, rev));
+ } else if (find_new_codelines) {
+ /* This is an "<op> into <path>#<rev>" line.
+ * We just want to try to create a codeline.
+ */
+ get_related_file(data->file, path);
+ }
+ free(type);
+ }
+}
+
+/** Finds all files in the codeline, and all revisions of those files,
+ * and all of the changesets they are from, and looks up the codelines
+ * and files they integrate or branch.
+ **/
+static void p4_filelog(struct p4_codeline *codeline)
+{
+ struct strbuf line;
+
+ struct p4_filelog_data data = {
+ .codeline = codeline,
+ .file = NULL,
+ .revision = NULL
+ };
+ char *arg;
+
+ if (codeline->filelog_done)
+ return;
+
+ if (LIST_P4_OPERATIONS)
+ fprintf(stderr, "p4 filelog %s\n", codeline->path);
+
+ strbuf_init(&line, 0);
+ strbuf_addstr(&line, codeline->path);
+ strbuf_addstr(&line, "/...");
+ arg = line.buf;
+ p4_call_info("filelog", 1, &arg, &data, p4_filelog_cb);
+ strbuf_release(&line);
+ if (codeline->history)
+ codeline->unreported = codeline->history->next;
+ else
+ codeline->unreported = codeline->changesets;
+ codeline->filelog_done = 1;
+}
+
+/** Functions to import things (i.e., fill out the representations) **/
+
+static struct p4_changeset_list *
+find_codeline_changeset(struct p4_changeset_list **list,
+ struct p4_codeline *codeline)
+{
+ while (*list) {
+ if ((*list)->changeset->codeline == codeline)
+ return *list;
+ list = &(*list)->next;
+ }
+ *list = xcalloc(1, sizeof(**list));
+ return *list;
+}
+
+static void resolve_codeline_contents(struct p4_codeline *codeline)
+{
+ struct p4_revision_list *prevrevs = NULL;
+ struct p4_changeset *changeset = codeline->changesets;
+ while (changeset) {
+ struct p4_revision_list *changes =
+ copy_revision_list(changeset->revisions);
+ changeset->contents = changes;
+ while (prevrevs) {
+ struct p4_revision_list *posn;
+ int found = 0;
+ for (posn = changes; posn; posn = posn->next) {
+ if (prevrevs->revision->file ==
+ posn->revision->file) {
+ found = 1;
+ break;
+ }
+ }
+ if (!found) {
+ struct p4_revision_list *item =
+ xcalloc(1, sizeof(*item));
+ item->revision = prevrevs->revision;
+ item->next = changeset->contents;
+ changeset->contents = item;
+ }
+ prevrevs = prevrevs->next;
+ }
+
+ prevrevs = changeset->contents;
+ changeset = changeset->next;
+ }
+}
+
+static void resolve_changeset_integrates(struct p4_changeset *changeset)
+{
+ struct p4_revision_list *posn;
+ struct p4_changeset_list *changesets = NULL;
+ /* For each codeline, we want the highest numbered changeset
+ * that introduced a revision that has been integrated.
+ */
+ for (posn = changeset->revisions; posn; posn = posn->next) {
+ struct p4_revision_list *rev_ints = posn->revision->integrated;
+ while (rev_ints) {
+ struct p4_changeset_list *item;
+ if (rev_ints->revision->file->codeline == changeset->codeline) {
+ rev_ints = rev_ints->next;
+ continue;
+ }
+ /* The revision doesn't have the changeset
+ * filled out unless we call this.
+ */
+ p4_filelog(rev_ints->revision->file->codeline);
+ if (!rev_ints->revision->changeset) {
+ rev_ints = rev_ints->next;
+ continue;
+ }
+ item = find_codeline_changeset(&changesets,
+ rev_ints->revision->file->codeline);
+ if (!item->changeset ||
+ item->changeset->number < rev_ints->revision->changeset->number) {
+ if (0)
+ printf("progress %lu integrates %s#%lu from %lu\n",
+ changeset->number,
+ rev_ints->revision->file->name,
+ rev_ints->revision->number,
+ rev_ints->revision->changeset->number);
+ item->changeset = rev_ints->revision->changeset;
+ }
+ rev_ints = rev_ints->next;
+ }
+ }
+ /* We could issue a warning if the state of other files didn't
+ * match and yet didn't get integrated, but that's a lot of
+ * work and there's no good way to represent the case of a
+ * commit contributing to but not being completely obsoleted
+ * by another commit.
+ */
+ changeset->integrated = changesets;
+ while (changesets) {
+ //printf("# integrate %lu from %lu\n", changeset->number, changesets->changeset->number);
+ changesets = changesets->next;
+ }
+}
+
+static void follow_codeline(struct p4_codeline *target)
+{
+ struct p4_codeline *posn;
+ if (target->filelog_done)
+ return;
+
+ p4_filelog(target);
+
+ if (0)
+ printf("progress resolving integrates\n");
+
+ /* Now resolve all the integrates in changesets */
+ for (posn = target->depot->codelines; posn; posn = posn->next) {
+ struct p4_changeset *changeset;
+ for (changeset = posn->unreported; changeset; changeset = changeset->next) {
+ resolve_changeset_integrates(changeset);
+ }
+ resolve_codeline_contents(posn);
+ }
+}
+
+static struct p4_codeline *import_depot(struct p4_depot *depot, const char *refname)
+{
+ struct p4_codeline *target;
+ char *path = refname_to_codeline(refname);
+ target = get_codeline(depot, path);
+
+ if (!target)
+ die("Invalid codeline: %s", path);
+
+ free(path);
+
+ follow_codeline(target);
+
+ return target;
+}
+
+static void name_changeset(struct p4_changeset *changeset)
+{
+ if (changeset->commit)
+ printf("%s\n", sha1_to_hex(changeset->commit->object.sha1));
+ else
+ printf(":%d\n", changeset->mark);
+}
+
+static void lookup_git_changeset(struct p4_codeline *codeline,
+ struct p4_changeset *changeset)
+{
+ while (!changeset->commit) {
+ struct commit *parent = codeline->history->commit->parents->item;
+ parse_commit(parent);
+ codeline->history->previous->commit = parent;
+ codeline->history = codeline->history->previous;
+ }
+}
+
+static void report_codeline(struct p4_codeline *codeline,
+ struct p4_changeset *until);
+
+static void identify_changeset(struct p4_changeset *changeset)
+{
+ if (changeset->mark || changeset->commit)
+ return;
+ if (changeset->codeline->finished_changeset >= changeset->number)
+ lookup_git_changeset(changeset->codeline, changeset);
+ else
+ report_codeline(changeset->codeline, changeset);
+}
+
+static int skip_found(struct p4_revision *revision,
+ struct p4_revision_list **origin) {
+ struct p4_revision *orev = NULL;
+ struct p4_revision_list *i;
+ while (*origin) {
+ if (!strcmp((*origin)->revision->file->name,
+ revision->file->name)) {
+ struct p4_revision_list *oitem = *origin;
+ *origin = oitem->next;
+ orev = oitem->revision;
+ free(oitem);
+ break;
+ } else {
+ origin = &((*origin)->next);
+ }
+ }
+ if (!revision->branch) /* It's changed anyway */
+ return 0;
+ for (i = revision->integrated; i; i = i->next) {
+ if (i->revision == orev)
+ return 1;
+ }
+ return 0;
+}
+
+static void report_codeline(struct p4_codeline *codeline, struct p4_changeset *until)
+{
+ struct p4_changeset *changeset;
+ struct p4_revision_list *rev;
+
+ printf("progress import codeline %s (%lu-%lu)\n", codeline->path,
+ codeline->unreported->number, until->number);
+
+ for (changeset = codeline->unreported; changeset; changeset = changeset->next) {
+ struct p4_changeset_list *integrated = changeset->integrated;
+ struct p4_revision_list *origin = NULL;
+
+ while (integrated) {
+ identify_changeset(integrated->changeset);
+ integrated = integrated->next;
+ }
+ printf("progress import changeset %lu (%s)\n",
+ changeset->number, changeset->codeline->path);
+ printf("# changeset %lu\n", changeset->number);
+ printf("commit %s\n", codeline->refname);
+ changeset->mark = codeline->depot->next_mark++;
+ printf("mark :%d\n", changeset->mark);
+ p4_change(changeset);
+ integrated = changeset->integrated;
+ if (changeset->previous) {
+ printf("from ");
+ name_changeset(changeset->previous);
+ } else if (integrated) {
+ printf("from ");
+ origin = copy_revision_list(integrated->changeset->contents);
+ name_changeset(integrated->changeset);
+ integrated = integrated->next;
+ }
+
+ while (integrated) {
+ printf("merge ");
+ name_changeset(integrated->changeset);
+ integrated = integrated->next;
+ }
+
+ for (rev = changeset->revisions; rev; rev = rev->next) {
+ if (rev->revision->delete) {
+ printf("D %s\n", rev->revision->file->name + 1);
+ } else if (!skip_found(rev->revision, &origin)) {
+ printf("M %s inline %s\n",
+ get_file_mode(rev->revision->type),
+ rev->revision->file->name + 1);
+ p4_print(rev->revision);
+ }
+ }
+ while (origin) {
+ struct p4_revision_list *old;
+ printf("D %s\n", origin->revision->file->name + 1);
+ old = origin;
+ origin = origin->next;
+ free(old);
+ }
+ printf("\n");
+ codeline->unreported = changeset->next;
+ if (changeset == until)
+ break;
+ }
+ printf("checkpoint\n");
+}
+
+static void export_change(struct diff_options *options,
+ unsigned old_mode, unsigned new_mode,
+ const unsigned char *old_sha1,
+ const unsigned char *new_sha1,
+ const char *path)
+{
+ struct p4_codeline *codeline = options->format_callback_data;
+ p4_edit(codeline, path);
+ p4_write_blob(codeline->working, new_mode, new_sha1, path);
+}
+
+static void export_add_remove(struct diff_options *options,
+ int addremove, unsigned mode,
+ const unsigned char *sha1,
+ const char *path)
+{
+ struct p4_codeline *codeline = options->format_callback_data;
+ if (addremove == '+') {
+ p4_write_blob(codeline->working, mode, sha1, path);
+ p4_add(codeline, path);
+ } else if (addremove == '-') {
+ p4_delete(codeline, path);
+ }
+}
+
+static void export_p4(struct p4_depot *depot, unsigned char *sha1, const char *ref)
+{
+ struct p4_codeline *target;
+ struct strbuf buf;
+
+ // check client
+
+ fprintf(stderr, "Exporting %s to %s\n", sha1_to_hex(sha1), ref);
+
+ target = import_depot(depot, ref);
+
+ strbuf_init(&buf, 0);
+
+ struct p4_changeset *parent = NULL, *integrate = NULL;
+ struct commit *commit, *git_parent = NULL;
+ struct commit_list *parents;
+ commit = lookup_commit(sha1);
+ parse_commit(commit);
+ for (parents = commit->parents; parents; parents = parents->next) {
+ struct p4_changeset *p4_parent =
+ changeset_from_commit(depot, parents->item);
+ if (p4_parent) {
+ if (p4_parent->codeline == target) {
+ parent = p4_parent;
+ git_parent = parents->item;
+ } else
+ integrate = p4_parent;
+ } else {
+ fprintf(stderr, "Unknown parent\n");
+ return;
+ }
+ }
+ if (target->head != parent) {
+ if (!parent) {
+ printf("progress Couldn't find parent\n");
+ return;
+ }
+ printf("progress not up-to-date\n");
+ return;
+ }
+ if (p4_where(target))
+ return;
+ p4_sync(parent);
+
+ if (!parent) {
+ // Need to start new codeline
+ return;
+ }
+ p4_write_tree(target->working, commit->tree->object.sha1);
+ if (integrate) {
+ p4_integrate(target, integrate);
+ fprintf(stderr, "Exporting merge\n");
+ // Dunno how to do this
+ p4_resolve();
+ //return;
+ }
+ struct tree_desc pre, post;
+ struct diff_options opts;
+ memset(&opts, 0, sizeof(opts));
+ parse_tree(commit->tree);
+ parse_tree(git_parent->tree);
+ init_tree_desc(&pre, git_parent->tree->buffer, git_parent->tree->size);
+ init_tree_desc(&post, commit->tree->buffer, commit->tree->size);
+ opts.change = export_change;
+ opts.add_remove = export_add_remove;
+ opts.format_callback_data = target;
+ opts.flags = DIFF_OPT_RECURSIVE;
+ diff_tree(&pre, &post, "/", &opts);
+
+ unsigned long change = p4_submit(commit);
+
+ p4_release_tree();
+
+ target->filelog_done = 0;
+ follow_codeline(target);
+ struct p4_changeset *changeset = get_changeset(target, change);
+ add_mapped_changeset(depot, commit, changeset);
+ report_codeline(target, changeset);
+}
+
+static const char **env;
+static int env_nr;
+static int env_alloc;
+
+static int connected;
+
+static const char **codelines;
+static int codeline_nr;
+static int codeline_alloc;
+
+static int handle_config(const char *key, const char *value, void *cb)
+{
+ struct remote *remote = cb;
+ struct strbuf buf;
+ const char *subkey = NULL;
+
+ if (!prefixcmp(key, "vcs-p4."))
+ subkey = key + strlen("vcs-p4.");
+
+ if (remote && !prefixcmp(key, "remote.") &&
+ !prefixcmp(key + strlen("remote."), remote->name))
+ subkey = key + strlen("remote.") + strlen(remote->name) + 1;
+
+ if (!subkey)
+ return 0;
+
+ if (!strcmp(subkey, "findbranches")) {
+ find_new_codelines = git_config_bool(key, value);
+ }
+ if (!strcmp(subkey, "ignorecodeline")) {
+ ALLOC_GROW(ignore_codelines, ignore_codeline_nr + 1,
+ ignore_codeline_alloc);
+ ignore_codelines[ignore_codeline_nr++] = xstrdup(value);
+ }
+ if (!strcmp(subkey, "port")) {
+ strbuf_init(&buf, 0);
+ strbuf_addf(&buf, "P4PORT=%s", value);
+
+ ALLOC_GROW(env, env_nr + 1, env_alloc);
+ env[env_nr++] = strbuf_detach(&buf, NULL);
+ }
+ if (!strcmp(subkey, "client")) {
+ strbuf_init(&buf, 0);
+ strbuf_addf(&buf, "P4CLIENT=%s", value);
+
+ ALLOC_GROW(env, env_nr + 1, env_alloc);
+ env[env_nr++] = strbuf_detach(&buf, NULL);
+ }
+ if (!strcmp(subkey, "codelineformat")) {
+ codeline_regex = (regex_t*)xmalloc(sizeof(regex_t));
+ if (regcomp(codeline_regex, value, REG_EXTENDED)) {
+ free(codeline_regex);
+ fprintf(stderr, "Invalid codeline pattern: %s",
+ value);
+ }
+ }
+ if (!strcmp(subkey, "codeline")) {
+ ALLOC_GROW(codelines, codeline_nr + 1, codeline_alloc);
+ codelines[codeline_nr++] = xstrdup(value);
+ }
+ return 0;
+}
+
+static void connect_to_p4(void)
+{
+ if (!connected)
+ p4_init(env);
+ connected = 1;
+}
+
+static void disconnect_from_p4(void)
+{
+ if (LIST_P4_OPERATIONS)
+ fprintf(stderr, "Prints done: %d\n", prints_done);
+ if (connected)
+ p4_fini();
+ connected = 0;
+}
+
+const char *get_tree_path(const unsigned char *sha1, const char *path,
+ unsigned *mode, unsigned long *size)
+{
+ unsigned char blob_sha1[20];
+ enum object_type type;
+ if (get_tree_entry(sha1, path, blob_sha1, mode)) {
+ error("Couldn't find %s in %s", path, sha1_to_hex(sha1));
+ return NULL;
+ }
+ return read_sha1_file(blob_sha1, &type, size);
+}
+
+int main(int argc, const char **argv)
+{
+ struct remote *remote;
+ struct strbuf buf;
+ struct p4_depot *depot = NULL;
+
+ setup_git_directory();
+
+ if (argc < 1) {
+ fprintf(stderr, "Remote needed");
+ return 1;
+ }
+
+ remote = remote_get(argv[1]);
+ git_config(handle_config, remote);
+
+ ALLOC_GROW(env, env_nr + 1, env_alloc);
+ env[env_nr++] = NULL;
+
+ strbuf_init(&buf, 0);
+ do {
+ if (strbuf_getline(&buf, stdin, '\n') == EOF)
+ break;
+ if (!*buf.buf)
+ break;
+ if (!strcmp(buf.buf, "capabilities")) {
+ printf("import\n");
+ printf("export\n");
+ printf("\n");
+ fflush(stdout);
+ } else if (!prefixcmp(buf.buf, "import ")) {
+ save_commit_buffer = 1;
+
+ find_new_codelines = 0;
+
+ connect_to_p4();
+
+ if (!depot)
+ depot = get_depot();
+
+ identify_changeset(import_depot(depot, buf.buf + strlen("import "))->head);
+ } else if (!strcmp(buf.buf, "list")) {
+ int i;
+
+ git_config(handle_config, remote);
+
+ ALLOC_GROW(env, env_nr + 1, env_alloc);
+ env[env_nr++] = NULL;
+
+ if (find_new_codelines) {
+ struct p4_codeline *codeline;
+ save_commit_buffer = 1;
+
+ connect_to_p4();
+
+ if (!depot)
+ depot = get_depot();
+
+ for (i = 0; i < codeline_nr; i++)
+ import_depot(depot,
+ codeline_to_refname(codelines[i]));
+
+ for (codeline = depot->codelines; codeline;
+ codeline = codeline->next) {
+ if (codeline->ignore)
+ continue;
+ follow_codeline(codeline);
+ printf("? %s %s\n", codeline->refname,
+ codeline->head == codeline->history ?
+ "unchanged" : "");
+ }
+ } else {
+ for (i = 0; i < codeline_nr; i++)
+ printf("? %s\n",
+ codeline_to_refname(codelines[i]));
+ }
+ printf("\n");
+ fflush(stdout);
+ } else if (!prefixcmp(buf.buf, "export ")) {
+ unsigned char sha1[20];
+ char *hash = buf.buf + strlen("export ");
+ char *ref = strchr(hash, ' ');
+ *(ref++) = '\0';
+ get_sha1(hash, sha1);
+
+ connect_to_p4();
+
+ if (!depot)
+ depot = get_depot();
+
+ export_p4(depot, sha1, ref);
+ // 1: check whether the import of the target location
+ // is up-to-date
+
+ // 2: find the target location in the client view
+
+ // 3: bring the client view up-to-date with the target
+ // location
+
+ // 4: recheck that this matches the tree
+
+ // 5: open the necessary files in the client
+
+ // 6: replace the necessary files in the filesystem
+
+ // 7: submit
+
+ // 8: reimport
+
+ // 9: go back to (3)
+ } else {
+ disconnect_from_p4();
+ fprintf(stderr, "Unrecognized command %s\n", buf.buf);
+ return 1;
+ }
+ strbuf_reset(&buf);
+ } while (1);
+ disconnect_from_p4();
+ return 0;
+}
diff --git a/vcs-p4/vcs-p4.h b/vcs-p4/vcs-p4.h
new file mode 100644
index 0000000..104ef90
--- /dev/null
+++ b/vcs-p4/vcs-p4.h
@@ -0,0 +1,135 @@
+#ifndef VCS_P4_H
+#define VCS_P4_H
+
+struct p4_depot {
+ struct p4_codeline *codelines;
+
+ int next_mark;
+
+ struct p4_changeset **added;
+ int added_nr;
+ int added_alloc;
+};
+
+/** Note that multiple codelines can have changesets with the same
+ * number.
+ **/
+struct p4_changeset {
+ struct p4_codeline *codeline;
+
+ unsigned long number;
+
+ /** Used only if a previous import found this changeset **/
+ struct commit *commit;
+
+ /** Used for the original git commit this was exported as **/
+ struct commit *original;
+
+ /** Used only if this changeset is newly imported in this operation. **/
+ int mark;
+
+ const char *message;
+
+ /** These are the revisions introduced in the changeset **/
+ struct p4_revision_list *revisions;
+
+ /** These are the revisions which are current as of the changeset **/
+ struct p4_revision_list *contents;
+
+ /** Not explicit in p4 **/
+ struct p4_changeset_list *integrated;
+
+ /** Next and previous in codeline **/
+ struct p4_changeset *next;
+ struct p4_changeset *previous;
+};
+
+struct p4_changeset_list {
+ struct p4_changeset *changeset;
+ struct p4_changeset_list *next;
+};
+
+struct p4_revision {
+ unsigned long number;
+
+ unsigned delete : 1;
+ unsigned branch : 1; /* unchanged against something integrated */
+
+ const char *type;
+
+ struct p4_file *file;
+ struct p4_changeset *changeset;
+
+ struct p4_revision_list *integrated;
+
+ /** Next in file **/
+ struct p4_revision *next;
+};
+
+/** Represents a collection of revisions of different files
+ **/
+struct p4_revision_list {
+ struct p4_revision *revision;
+ struct p4_revision_list *next;
+};
+
+struct p4_file {
+ struct p4_codeline *codeline;
+ const char *name;
+
+ unsigned head_number;
+
+ struct p4_revision *revisions;
+
+ /** Next file in codeline **/
+ struct p4_file *next;
+};
+
+/** perforce doesn't record codelines; we have to reverse-engineer
+ * them from how people seem to be branching.
+ **/
+struct p4_codeline {
+ unsigned ignore : 1;
+
+ struct p4_depot *depot;
+
+ /** Base path of codeline **/
+ const char *path;
+
+ /** git refname to import into **/
+ const char *refname;
+
+ struct p4_file *files;
+ struct p4_changeset *changesets;
+
+ int filelog_done;
+
+ /* The incremental state is that we have some changeset that
+ * we previously imported up to, and we have git history going
+ * back from that point, of which we've looked up some and
+ * could look up more as needed. Also, there's p4-only history
+ * going forward after the common history, and we've imported
+ * some of that, and could import more as needed. Since
+ * codelines are sorted by changeset number, we can tell which
+ * way to go to get a name for a changeset.
+ */
+ struct p4_changeset *history;
+ struct p4_changeset *unreported;
+
+ struct p4_changeset *head;
+
+ unsigned long finished_changeset;
+
+ /** For reporting **/
+ unsigned long num_changesets;
+
+ /** Next codeline in depot **/
+ struct p4_codeline *next;
+
+ /** Filesystem location of working directory for this codeline
+ * on the client.
+ **/
+ char *working;
+};
+
+#endif
--
1.6.4.32.gf5148
^ permalink raw reply related
* Re: [PATCH] Handle UNC paths everywhere
From: Robin Rosenberg @ 2010-01-25 21:42 UTC (permalink / raw)
To: Johannes Sixt; +Cc: git, Junio C Hamano
In-Reply-To: <201001252107.45745.j6t@kdbg.org>
måndagen den 25 januari 2010 21.07.44 skrev Johannes Sixt:
> On Montag, 25. Januar 2010, Robin Rosenberg wrote:
> > In Windows paths beginning with // are knows as UNC paths. They are
> > absolute paths, usually referring to a shared resource on a server.
> >
> > Examples of legal UNC paths
> >
> > \\hub\repos\repo
> > \\?\unc\hub\repos
> > \\?\d:\repo
>
> I agree that that the problem that you are addressing needs a solution.
>
> However, the solution is not a whole-sale replacement of
> have_dos_drive_prefix() by a function that is only a tiny bit fancier.
> Accompanying changes are needed, and perhaps more code locations need
> change.
I was hoping to get help in identifying these and perhaps more cases to test
than then ones I thought of at first.
> > @@ -648,7 +648,7 @@ int safe_create_leading_directories_const(const char
> > *path);
> > char *enter_repo(char *path, int strict);
> > static inline int is_absolute_path(const char *path)
> > {
> > - return path[0] == '/' || has_dos_drive_prefix(path);
> > + return path[0] == '/' || has_win32_abs_prefix(path);
>
> Perhaps we need is_dir_sep(path[0]) here? But since I have not observed any
> breakage in connection with this code, I think that all callers feed only
> normalized paths (i.e. with forward slash). (Note that our getcwd()
probably true.
> implementation converts backslashes to forward slashes.) This means that a
> full-fledged check is not needed.
ack.
> > @@ -5,7 +5,7 @@ char *gitbasename (char *path)
> > {
> > const char *base;
> > /* Skip over the disk name in MSDOS pathnames. */
> > - if (has_dos_drive_prefix(path))
> > + if (has_win32_abs_prefix(path))
> > path += 2;
>
> This change is unnecessary; it really is only to skip an initial driver
> prefix. If you want to support \\?\X: style paths, more work is needed here
> so that you do not return X: or ? as the basename.
late night hacks aren't always good.
> > +#define has_win32_abs_prefix(path) \
>
> Do we really have to name everything "win32" when it is about Windows?
hmm
> > @@ -535,7 +535,7 @@ struct child_process *git_connect(int fd[2], const
> > char *url_orig,
> > end = host;
> >
> > path = strchr(end, c);
> > - if (path && !has_dos_drive_prefix(end)) {
> > + if (path && !has_win32_abs_prefix(end)) {
>
> This change is wrong because the check is really only about the drive
> prefix: It checks that we do not mistake c:/foo as a ssh connection to
> host c, path /foo. Yes, it does mean that on Windows we cannot have
> remotes to hosts whose name consists only of a single letter using the rcp
> notation (you must say ssh://c/foo if you mean it).
right.
> > @@ -409,7 +409,7 @@ int normalize_path_copy(char *dst, const char *src)
> > {
> > char *dst0;
> >
> > - if (has_dos_drive_prefix(src)) {
> > + if (has_win32_abs_prefix(src)) {
> > *dst++ = *src++;
> > *dst++ = *src++;
> > }
>
> Is skipping just two characters for \\ or \\?\whatever paths the right
> thing?
I shouldn't skip anything. I wasn't converting the first two \'s to //.
> > @@ -342,7 +342,7 @@ const char *setup_git_directory_gently(int
> > *nongit_ok) die_errno("Unable to read current working directory");
> >
> > ceil_offset = longest_ancestor_length(cwd, env_ceiling_dirs);
> > - if (ceil_offset < 0 && has_dos_drive_prefix(cwd))
> > + if (ceil_offset < 0 && has_win32_abs_prefix(cwd))
> > ceil_offset = 1;
>
> I doubt that this is correct. The purpose of this check is that "c:/" is
> the last directory that is checked (on Unix it would be "/") when path
> components are stripped from cwd. For UNC paths this must be adjusted
> depending on how you want to support \\server\share and \\?\c:\paths: You
> do not want to check whether \\server\.git or \\.git or \\?\.git are git
> directories.
\\server\.git seems valid. Probably not a good idea, but who am I to judge?
>
> > --- a/transport.c
> > +++ b/transport.c
> > @@ -797,7 +797,7 @@ static int is_local(const char *url)
> > const char *colon = strchr(url, ':');
> > const char *slash = strchr(url, '/');
> > return !colon || (slash && slash < colon) ||
> > - has_dos_drive_prefix(url);
> > + has_win32_abs_prefix(url);
>
> This check is again to not mistake c:/foo as rcp style connection. No
> change needed.
>
> As I said, changes to other parts are perhaps also needed, most
> prominently, make_relative_path() that prompted this patch. What about
> make_absolute_path() and make_non_relative_path()?
Thanks for the feedback.
-- robin
^ 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