Bug 175468 - Paths in generated diffs should be from VCS root (if any)
Paths in generated diffs should be from VCS root (if any)
Status: NEW
Product: utilities
Classification: Unclassified
Component: Diff
6.x
All All
: P2 (vote)
: 6.x
Assigned To: Tomas Stupka
diff-issues@utilities
PLAN
:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2009-10-26 19:17 UTC by _ tboudreau
Modified: 2010-10-07 15:17 UTC (History)
1 user (show)

See Also:
Issue Type: ENHANCEMENT
:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description _ tboudreau 2009-10-26 19:17:05 UTC
Currently, if someone makes changes in, say, javacard.project, and right-clicks Source Files in the project and creates
a diff, entries in that diff will look like:

Index: org/netbeans/modules/javacard/constants/ActionNames.java
--- org/netbeans/modules/javacard/constants/ActionNames.java Base (BASE)
+++ org/netbeans/modules/javacard/constants/ActionNames.java Locally Modified (Based On LOCAL)

When they send that diff to me, I have to figure out what directory the person right-clicked.

It would be much more useful, and save someone reviewing or applying such a diff, if all paths in the generated diff had
the path to the version control root prepended, e.g.

Index: org/netbeans/modules/javacard/constants/ActionNames.java
--- javacard.project/src/org/netbeans/modules/javacard/constants/ActionNames.java Base (BASE)
+++ javacard.project/src/org/netbeans/modules/javacard/constants/ActionNames.java Locally Modified (Based On LOCAL)

In that case nobody will ever have to guess what directory the patch was applied to, or the strip level.  This approach
seems clearly superior, since both the current approach and the suggested approach result in a diff containing the same
file changes, but with the suggested approach, the diff is unambiguous.  I cannot think of any case where having the
full path to the VCS root would do harm, but there are definitely cases where it is more difficult to read and apply
patches without it.

Similarly, Apply Diff Patch should try applying from the VCS root if the patch does not match the directory the user
invoked the action on.
Comment 1 Ondrej Vrabec 2009-10-27 10:24:27 UTC
It could be doable without problems for mercurial - paths in the exported diff could be relative to mercurial repository
root. However problems could arise e.g. for CVS or subversion. Imagine you check-out the whole repository and export a
diff (all paths will be relative to /). But someone else may check-out only /trunk/project1 and when he tries to apply
the patch, it will surely fail.
So what we're suggesting is add an option (in export uncommitted changes UI) to select the root manually.

> Similarly, Apply Diff Patch should try applying from the VCS root...
Can't be done. Apply patch functionality has no reference to any VCS support, it stands completely separated from vcs
modules. That's why you can apply patch even for non-versioned files.
However i'll check the Apply diff implementation - i think there's a simple heuristic trying to find the applicable
context. It should traverse parents of the selected file/folder and try to apply the patch on these parents. So
eventually it should end-up in the repository root for versioned files.
Comment 2 Jesse Glick 2009-10-27 16:18:08 UTC
I think what Tim was originally asking for is simply issue #169288. Tim: until that is implemented, do not use the IDE's
built-in diff and patch features for working on Mercurial repositories. Use the command line instead, and make sure you
have specified

[diff]
git=1

in your ~/.hgrc.
Comment 3 _ tboudreau 2009-10-27 18:47:36 UTC
Yes, I do that.  However, other people who are making changes and want me to apply them do not.
Comment 4 _ tboudreau 2009-10-27 18:53:07 UTC
Not sure if this is really a duplicate of issue 169288 - IIRC, CVS diff will let you create diffs that are not from the
VCS root.  

For this issue, I'm not concerned about what generates the diff (although if Hg or Git is in use, somehow generating
Git-style diffs would definitely be nice, and the solution suggested in issue 169288 may be the best way to do it), just
that the resulting diff contains complete path information, and so can always be applied from the root of the versioning
tree.
Comment 5 Jesse Glick 2009-10-27 20:38:30 UTC
You have to ask contributors to provide you with diffs in an appropriate format. For Mercurial, there is one proper
format: that produced by 'hg diff --git'.

CVS and Subversion do let you create diffs from any root. However for these systems there is not necessarily any unique
"versioning root" which your colleagues must share - it depends upon what you happen to have checked out. Both do have a
true root - the parent of CVSROOT in the case of CVS, and the "repository root" in the case of Subversion - but it is
rare for anyone to check out the true root in either system; usually only some subdir is checked out. For example, my
checkout of https://hudson.dev.java.net/svn/hudson/trunk/hudson is the /trunk/hudson subdir of the repo; there are good
reasons to have, say, /branches/concurrent-build checked out, which is a variant of /trunk/hudson/main; or I might check
out and build /trunk/hudson/plugins/mercurial as a standalone tree.

The benefit of extra path information for such VCSs is conditional. If you really have no clue what portion of the
codebase a patch is supposed to be affecting, then the full path can be useful, though often this is obvious from
context (e.g. issue tracking component where a patch was filed). On the other hand, someone creating a diff from a
higher root than you actually have checked out will produce a patch that cannot be applied without an appropriate -p
option, which can be an annoyance. I don't have a strong opinion about what the default should be. For CVS and
Subversion, there are actually several choices:

1. Relativize paths on the selected directory, as now.

2. Starting from the selection, find the topmost directory on disk which contains VCS metadata from the same repository,
and relativize paths to that directory.

3. Same as #2, but stop upon encountering a versioned parent directory whose local filename does not match the
repository name. This can happen in CVS due to "mix and match" checkouts, where subdirectories are drawn from other
points in the repository, and I think also in SVN when using svn:externals and possibly otherwise.

4. Use for every file its path relative to the server's repository root, completely ignoring local file layout. This is
perhaps the most portable but will produce odd results requiring -p by any recipient, e.g. patches all inside
'trunk/...' in the case of SVN.
Comment 6 _ tboudreau 2009-10-27 20:51:39 UTC
3. sounds like a reasonable compromise - I'd expect the common case to be that people do have similar checkouts
(although with SVN trunk/ dirs there may be more probability of difference, simply because it is the default but also
usually not what you want, so it depends on the user whether it's there or not).

The point here is that it does no harm to have the extra path metadata, but it can be more work not to have it.
Comment 7 Tomas Pavek 2009-12-08 09:34:22 UTC
Maybe also see bug 125742.
Comment 8 Ondrej Vrabec 2010-02-26 07:04:17 UTC
fix in mercurial: http://hg.netbeans.org/cdev/rev/d7f84c01fee7
all paths are relative to the repository root
Comment 9 Quality Engineering 2010-02-26 23:05:05 UTC
Integrated into 'main-golden', will be available in build *201002270200* on http://bits.netbeans.org/dev/nightly/ (upload may still be in progress)
Changeset: http://hg.netbeans.org/main/rev/d7f84c01fee7
User: Ondrej Vrabec <ovrabec@netbeans.org>
Log: Issue #175468 - Paths in generated diffs should be from VCS root (if any)
paths in the generated diff are relative to the repository root


By use of this website, you agree to the NetBeans Policies and Terms of Use. © 2012, Oracle Corporation and/or its affiliates. Sponsored by Oracle logo