Summary: | resolve-ChangeLogs doesn't work with relative paths | ||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Eric Seidel (no email) <eric> | ||||||||||||
Component: | Tools / Tests | Assignee: | David Kilzer (:ddkilzer) <ddkilzer> | ||||||||||||
Status: | RESOLVED FIXED | ||||||||||||||
Severity: | Normal | CC: | aroben, ddkilzer, evan, pkasting | ||||||||||||
Priority: | P2 | ||||||||||||||
Version: | 528+ (Nightly build) | ||||||||||||||
Hardware: | All | ||||||||||||||
OS: | All | ||||||||||||||
Attachments: |
|
Description
Eric Seidel (no email)
2008-04-18 14:47:32 PDT
Could probably use some combination of chdirWebKit() and/or determineSourceDir() from webkitdirs.pm, and fix up any relative paths passed in on the command line. However, this assumes resolve-ChangeLogs resides in the same WebKitTools directory that would be (in Eric's example) at ../WebKitTools. *** Bug 25456 has been marked as a duplicate of this bug. *** Created attachment 29878 [details]
Potential fix
This needs testing. If you try it out, please report back on your results.
The basic idea is that the resolve-ChangeLogs script will always chdir to the root of the source directory for you (if you don't run it from there), so that this assumption (that it's run from the root source dir) is no longer violated.
What I don't like about this patch is that it makes it impossible to use resolve-ChangeLogs from any project other than WebKit due to the way chdirWebKit() works. It would be better to use git prefix (and something equivalent for svn--anyone? Bueller?) to chdir() instead.
Created attachment 33910 [details] Add error checking to git ls-files command on close() Reviewed by NOBODY (OOPS!). Item 2 of <https://bugs.webkit.org/show_bug.cgi?id=18599#c0>. * Scripts/resolve-ChangeLogs: Added error checking to close() after running git ls-files. Added error checking to all system() calls by checking for a non-zero WEXITSTATUS($?). Changed "|| die;" expressions to "or die $!;". --- 2 files changed, 34 insertions(+), 15 deletions(-) Created attachment 33911 [details]
resolve-ChangeLogs doesn't work with relative paths
Reviewed by NOBODY (OOPS!).
* Scripts/resolve-ChangeLogs: Used chdirReturningRelativePath()
and determineVCSRoot() to change directories to the root of the
project before running the command and to provide a path for
removeChangeLogArguments() to make sure any ChangeLog arguments
on the command line are still found.
(canonicalRelativePath): Added. Returns a canonical path (e.g.,
stripping 'dir/../' from the path) relative to the current
directory.
(removeChangeLogArguments): Added argument which contains a
relative path that must be prepended to any ChangeLog arguments.
Used canonicalRelativePath() and File::Spec->catfile() to
construct a normalized, relative path to each file.
---
2 files changed, 34 insertions(+), 4 deletions(-)
Created attachment 33912 [details] Implement VCSUtils::chdirReturningRelativePath() Reviewed by NOBODY (OOPS!). Step 2 to fix: <http://webkit.org/b/18599> resolve-ChangeLogs doesn't work with relative paths * Scripts/VCSUtils.pm: (VCSUtils::chdirReturningRelativePath): Moved here from chdirAndGetDifference() in svn-create-patch. * Scripts/svn-create-patch: Switched to use chdirReturningRelativePath() instead of chdirAndGetDifference(). (chdirAndGetDifference): Removed. --- 3 files changed, 29 insertions(+), 13 deletions(-) Created attachment 33913 [details] Implement VCSUtils::determineVCSRoot() Reviewed by NOBODY (OOPS!). Step 1 to fix: <http://webkit.org/b/18599> resolve-ChangeLogs doesn't work with relative paths * Scripts/VCSUtils.pm: Removed reference to webkitdirs module. (VCSUtils::EXPORT): Added &determineVCSRoot. Realphabetized. (VCSUtils::determineGitRoot): Added. Based on code in commit-log-editor. (VCSUtils::determineVCSRoot): Implemented using determineGitRoot() and determineSVNRoot(). * Scripts/commit-log-editor: Replaced use of topLevelSourceDirectory() with determineVCSRoot(). Resorted use statements. (topLevelSourceDirectory): Removed. --- 3 files changed, 44 insertions(+), 16 deletions(-) Odd...I expected the patches to be posted in the reverse order from bugzilla-tool! Comment on attachment 33910 [details]
Add error checking to git ls-files command on close()
Perl is so disgusting.
Comment on attachment 33911 [details]
resolve-ChangeLogs doesn't work with relative paths
I don't fully understand, but so long as it works, rs=me. :)
Comment on attachment 33912 [details]
Implement VCSUtils::chdirReturningRelativePath()
you don't need qw() just (), iirc qw() == (), or?
I've never seen this line actually do anything:
@EXPORT = qw(&chdirReturningRelativePath &determineSVNRoot &determineVCSRoot &isGit &isGitDirectory &isSVN &isSVNDirectory &makeFilePathRelative
I think we just have it to comply with perl hear-say. ;)
LGTM.
Comment on attachment 33913 [details]
Implement VCSUtils::determineVCSRoot()
LGTM. Shame we have the same logic in both our python libraries and our perl libraries (yes, I know the perl ones came first). :)
(In reply to comment #11) > (From update of attachment 33912 [details]) > you don't need qw() just (), iirc qw() == (), or? Nope. The qw() or quote word operator is different from just adding parenthesis and making a list. To change qw(alpha beta gamma) to drop the qw operator, you would have to write ("alpha", "beta", "gamma"). See "man perlop" and search for "qw" for more details. See also the q, qq, qr and qx operators. > I've never seen this line actually do anything: > @EXPORT = qw(&chdirReturningRelativePath &determineSVNRoot > &determineVCSRoot &isGit &isGitDirectory &isSVN &isSVNDirectory > &makeFilePathRelative > I think we just have it to comply with perl hear-say. ;) Then you've never tried removing a method from the list and then running a script that uses the method. This line tells the perl module which methods are implicitly exported by default when the module is 'use'd in another module or script. This is the "opposite" of explicitly importing a list of variables or methods or :groups in a use statement, e.g.: use Getopt::Long qw(:config pass_through); See "man perlmod" for more details about how to write Perl modules. See "man perlfunc" for more information about "use". Comment on attachment 33913 [details] Implement VCSUtils::determineVCSRoot() Clearing review+ flag. Landed in r46669. <http://trac.webkit.org/changeset/46669> Comment on attachment 33913 [details] Implement VCSUtils::determineVCSRoot() Clearing review+ flag. Landed in r46669. <http://trac.webkit.org/changeset/46669> Committing to http://svn.webkit.org/repository/webkit/trunk ... M WebKitTools/ChangeLog M WebKitTools/Scripts/VCSUtils.pm M WebKitTools/Scripts/svn-create-patch Committed r46670 M WebKitTools/ChangeLog M WebKitTools/Scripts/VCSUtils.pm M WebKitTools/Scripts/svn-create-patch r46670 = f28f8d26c5ba542a9e276de8ecc1ba1eb85ec487 (trunk) No changes between current HEAD and refs/remotes/trunk Resetting to the latest refs/remotes/trunk http://trac.webkit.org/changeset/46670 All reviewed patches have been landed. Closing bug. Committing to http://svn.webkit.org/repository/webkit/trunk ... M WebKitTools/ChangeLog M WebKitTools/Scripts/resolve-ChangeLogs Committed r46671 M WebKitTools/ChangeLog M WebKitTools/Scripts/resolve-ChangeLogs r46671 = 368a45ec528e8feb5b11df9c57b625a8d371e678 (trunk) No changes between current HEAD and refs/remotes/trunk Resetting to the latest refs/remotes/trunk http://trac.webkit.org/changeset/46671 All reviewed patches have been landed. Closing bug. Committing to http://svn.webkit.org/repository/webkit/trunk ... M WebKitTools/ChangeLog M WebKitTools/Scripts/resolve-ChangeLogs Committed r46672 M WebKitTools/ChangeLog M WebKitTools/Scripts/resolve-ChangeLogs r46672 = e8e8619072e33a2a51ac3dc86e988e2b41ea1570 (trunk) No changes between current HEAD and refs/remotes/trunk Resetting to the latest refs/remotes/trunk http://trac.webkit.org/changeset/46672 All reviewed patches have been landed. Closing bug. Hmm...my local 'land-commits' patch to bugzilla-tool needs a lot more work when landing a patch series on a single bug. Additional fix (work around abs2rel() bug with symlinks) committed as r46988. |