From b811a3c05e3779ee343916fa8d5ec7826c7f49ce Mon Sep 17 00:00:00 2001 From: Christopher Williams Date: Wed, 4 Jun 2014 10:43:21 -0400 Subject: [PATCH] TISTUD-6376 Appcelerator Studio 3.3.0.201405211748 (RC) is very slow to work - Refactor the ChangedFile class and how we perform Gitindex refreshes - Make ChangedFile hold an IPath, not two Strings to represent portable/OS versions of path. Use IPath in API, not String. - try to encapsulate most changes to ChangedFile. It's intended to be a value object pattern, so when we want a modified version we clone or merge into new instances - try to keep immutable as much as possible. - When we do refreshes, don't use a shared list across the three jobs/callables that gather the staged/unstaged/other files. Instead have each build it's own mapping from IPath to ChangedFile, then if we have multiple entries for the same path, merge the ChangedFile instances. This should avoid synchronization issues - and hopefully should be faster. --- .../aptana/git/core/GitMoveDeleteHook.java | 2 +- .../aptana/git/core/model/ChangedFile.java | 127 ++++-- .../com/aptana/git/core/model/GitIndex.java | 373 ++++++++---------- .../aptana/git/core/model/GitRepository.java | 4 +- .../git/core/model/IndexChangedEvent.java | 2 +- .../src/com/aptana/git/ui/DiffFormatter.java | 7 +- .../git/ui/internal/actions/CommitDialog.java | 24 +- .../actions/CompareWithHEADHandler.java | 7 +- .../git/ui/internal/actions/DiffHandler.java | 7 +- .../actions/MergeConflictsHandler.java | 8 +- .../ui/internal/actions/RevertHandler.java | 3 +- .../git/core/GitMoveDeleteHookTest.java | 13 +- .../core/GitMoveDeleteIntegrationTest.java | 14 +- .../aptana/git/core/model/GitIndexTest.java | 32 +- .../git/core/model/GitRepositoryTest.java | 8 +- 15 files changed, 342 insertions(+), 289 deletions(-) diff --git a/plugins/com.aptana.git.core/src/com/aptana/git/core/GitMoveDeleteHook.java b/plugins/com.aptana.git.core/src/com/aptana/git/core/GitMoveDeleteHook.java index f381aa34db..c6e73e9608 100644 --- a/plugins/com.aptana.git.core/src/com/aptana/git/core/GitMoveDeleteHook.java +++ b/plugins/com.aptana.git.core/src/com/aptana/git/core/GitMoveDeleteHook.java @@ -58,7 +58,7 @@ public boolean deleteFile(final IResourceTree tree, final IFile file, final int tree.addToLocalHistory(file); // Delete the file through the repo - IStatus status = repo.deleteFile(changed.getPath()); + IStatus status = repo.deleteFile(changed.getRelativePath()); if (status.isOK()) { tree.deletedFile(file); diff --git a/plugins/com.aptana.git.core/src/com/aptana/git/core/model/ChangedFile.java b/plugins/com.aptana.git.core/src/com/aptana/git/core/model/ChangedFile.java index 69ee967db3..3a84ceb0c7 100644 --- a/plugins/com.aptana.git.core/src/com/aptana/git/core/model/ChangedFile.java +++ b/plugins/com.aptana.git.core/src/com/aptana/git/core/model/ChangedFile.java @@ -1,6 +1,6 @@ /** * Aptana Studio - * Copyright (c) 2005-2011 by Appcelerator, Inc. All Rights Reserved. + * Copyright (c) 2005-2014 by Appcelerator, Inc. All Rights Reserved. * Licensed under the terms of the GNU Public License (GPL) v3 (with exceptions). * Please see the license.html included with this distribution for details. * Any modifications to this file must keep this entire header intact. @@ -10,12 +10,16 @@ import java.text.MessageFormat; import org.eclipse.core.runtime.Assert; -import org.eclipse.core.runtime.Path; +import org.eclipse.core.runtime.IPath; + +import com.aptana.core.logging.IdeLog; +import com.aptana.git.core.GitPlugin; /** * A value object representing a changed file in a git repo. There can be a large number of these and they can be - * constantly sorted/hashed/compared - so we try to use direct field references over accessors. We also pre-generate - * both a portable and OS-specific version of the relative path once. + * constantly sorted/hashed/compared. Ideally we don't want to modify the state of a ChangedFile, but to generate a new + * instance with the changes we want. However, we still do cheat in a handful of cases where we mark the staged/unstaged + * booleans. * * @author cwilliams */ @@ -33,34 +37,42 @@ public enum Status * * @param other */ - public ChangedFile(ChangedFile other) + private ChangedFile(ChangedFile other) { - this(other.portablePath, other.status); - this.hasStagedChanges = other.hasStagedChanges; - this.hasUnstagedChanges = other.hasUnstagedChanges; - this.commitBlobMode = other.commitBlobMode; - this.commitBlobSHA = other.commitBlobSHA; + this(other.repo, other.path, other.status, other.commitBlobMode, other.commitBlobSHA, other.hasStagedChanges, + other.hasUnstagedChanges); } - public ChangedFile(String path, Status status) + // FIXME can we enforce that a ChangedFile cannot live outside it's repo? What sort of sanity checking can we do + // against the repo/location? + public ChangedFile(GitRepository repository, IPath path, Status status, String mode, String sha, boolean staged, + boolean unstaged) { - this.portablePath = path; + this.repo = repository; + this.path = path; this.status = status; - this.osPath = Path.fromPortableString(path).toOSString(); + this.commitBlobMode = mode; + this.commitBlobSHA = sha; + this.hasStagedChanges = staged; + this.hasUnstagedChanges = unstaged; } - private String osPath; - String portablePath; - Status status; - boolean hasStagedChanges; - boolean hasUnstagedChanges; - String commitBlobSHA; - String commitBlobMode; + private final GitRepository repo; + private final IPath path; + final Status status; + private boolean hasStagedChanges; + private boolean hasUnstagedChanges; + private final String commitBlobSHA; + private final String commitBlobMode; - // FIXME Use IPath - public String getPath() + /** + * Returns the path relative to the repo root. + * + * @return + */ + public IPath getRelativePath() { - return osPath; + return path; } public Status getStatus() @@ -68,6 +80,29 @@ public Status getStatus() return status; } + /** + * Forces this to be marked as having staged changes, no unstaged changes. + */ + public void makeStaged() + { + hasUnstagedChanges = false; + hasStagedChanges = true; + } + + /** + * Forces this to be marked as having unstaged changes, no staged changes. + */ + public void makeUnstaged() + { + this.hasUnstagedChanges = true; + this.hasStagedChanges = false; + } + + public void setUnstaged(boolean unstaged) + { + this.hasUnstagedChanges = unstaged; + } + public boolean hasStagedChanges() { return hasStagedChanges; @@ -94,17 +129,17 @@ protected String indexInfo() "File is not new, but doesn't have an index entry!"); //$NON-NLS-1$ if (commitBlobSHA == null) { - return MessageFormat.format("0 0000000000000000000000000000000000000000\t{0}\0", portablePath); //$NON-NLS-1$ + return MessageFormat.format("0 0000000000000000000000000000000000000000\t{0}\0", path.toPortableString()); //$NON-NLS-1$ } - return MessageFormat.format("{0} {1}\t{2}\0", commitBlobMode, commitBlobSHA, portablePath); //$NON-NLS-1$ + return MessageFormat.format("{0} {1}\t{2}\0", commitBlobMode, commitBlobSHA, path.toPortableString()); //$NON-NLS-1$ } @Override public String toString() { - return MessageFormat.format( - "{0} {1} (Staged? {2}, Unstaged? {3})", status, osPath, hasStagedChanges, hasUnstagedChanges); //$NON-NLS-1$ + return MessageFormat + .format("{0} {1} (Staged? {2}, Unstaged? {3})", status, path.toOSString(), hasStagedChanges, hasUnstagedChanges); //$NON-NLS-1$ } public boolean hasUnmergedChanges() @@ -114,7 +149,7 @@ public boolean hasUnmergedChanges() public int compareTo(ChangedFile o) { - return portablePath.compareTo(o.portablePath); + return path.toPortableString().compareTo(o.path.toPortableString()); } @Override @@ -124,7 +159,7 @@ public boolean equals(Object obj) { ChangedFile other = (ChangedFile) obj; return (hasStagedChanges == other.hasStagedChanges) && (hasUnstagedChanges == other.hasUnstagedChanges) - && (status == other.status) && (portablePath == other.portablePath); + && (status == other.status) && (path.equals(other.path)); } return false; } @@ -135,7 +170,39 @@ public int hashCode() int hash = 31 + Boolean.valueOf(hasStagedChanges).hashCode(); hash = hash * 31 + Boolean.valueOf(hasUnstagedChanges).hashCode(); hash = hash * 31 + status.hashCode(); - hash = hash * 31 + portablePath.hashCode(); + hash = hash * 31 + path.hashCode(); return hash; } + + public ChangedFile clone() + { + return new ChangedFile(this); + } + + public ChangedFile merge(ChangedFile other) + { + String mode = this.commitBlobMode; + if (mode == null) + { + mode = other.commitBlobMode; + } + + String sha = this.commitBlobSHA; + if (sha == null) + { + sha = other.commitBlobSHA; + } + if (status != other.status) + { + IdeLog.logWarning(GitPlugin.getDefault(), "Mismatch statuses when merging. Who wins? " + status.name() + + ", " + other.status.name()); + } + return new ChangedFile(this.repo, this.path, status, mode, sha, + this.hasStagedChanges || other.hasStagedChanges, this.hasUnstagedChanges || other.hasUnstagedChanges); + } + + public GitRepository getRepository() + { + return repo; + } } diff --git a/plugins/com.aptana.git.core/src/com/aptana/git/core/model/GitIndex.java b/plugins/com.aptana.git.core/src/com/aptana/git/core/model/GitIndex.java index c4e6ee17ee..a1e8d527b8 100644 --- a/plugins/com.aptana.git.core/src/com/aptana/git/core/model/GitIndex.java +++ b/plugins/com.aptana.git.core/src/com/aptana/git/core/model/GitIndex.java @@ -18,6 +18,7 @@ import java.util.Iterator; import java.util.List; import java.util.Map; +import java.util.Map.Entry; import java.util.Set; import java.util.concurrent.Callable; import java.util.concurrent.CancellationException; @@ -32,11 +33,13 @@ import org.eclipse.core.resources.IResource; import org.eclipse.core.resources.ResourcesPlugin; import org.eclipse.core.runtime.Assert; +import org.eclipse.core.runtime.CoreException; import org.eclipse.core.runtime.IPath; import org.eclipse.core.runtime.IProgressMonitor; import org.eclipse.core.runtime.IStatus; import org.eclipse.core.runtime.MultiStatus; import org.eclipse.core.runtime.NullProgressMonitor; +import org.eclipse.core.runtime.Path; import org.eclipse.core.runtime.Platform; import org.eclipse.core.runtime.Status; import org.eclipse.core.runtime.SubMonitor; @@ -71,10 +74,9 @@ public class GitIndex * UI views) */ private static final String[] BINARY_EXTENSIONS = new String[] { - ".pdf", ".jpg", ".jpeg", ".png", ".bmp", ".gif", ".o", ".class", ".zip", ".gz", ".tar", ".ico", ".so", ".jar" }; //$NON-NLS-1$ //$NON-NLS-2$ //$NON-NLS-3$ //$NON-NLS-4$ //$NON-NLS-5$ //$NON-NLS-6$ //$NON-NLS-7$ //$NON-NLS-8$ //$NON-NLS-9$ //$NON-NLS-10$ //$NON-NLS-11$ //$NON-NLS-12$ //$NON-NLS-13$ //$NON-NLS-14$ + "pdf", "jpg", "jpeg", "png", "bmp", "gif", "o", "class", "zip", "gz", "tar", "ico", "so", "jar" }; //$NON-NLS-1$ //$NON-NLS-2$ //$NON-NLS-3$ //$NON-NLS-4$ //$NON-NLS-5$ //$NON-NLS-6$ //$NON-NLS-7$ //$NON-NLS-8$ //$NON-NLS-9$ //$NON-NLS-10$ //$NON-NLS-11$ //$NON-NLS-12$ //$NON-NLS-13$ //$NON-NLS-14$ private GitRepository repository; - private boolean amend; /** * The list of changed files that is a copy of the above list. Only copied at the very end of the refresh, so it @@ -85,8 +87,6 @@ public class GitIndex private boolean notify; - private List files; - /** * Service which launches the refresh commands in threads. */ @@ -145,7 +145,7 @@ public IStatus refresh(IProgressMonitor monitor) * @param monitor * @return */ - IStatus refresh(boolean notify, Collection filePaths, IProgressMonitor monitor) + IStatus refresh(boolean notify, final Collection filePaths, IProgressMonitor monitor) { SubMonitor sub = SubMonitor.convert(monitor, 100); if (sub.isCanceled()) @@ -154,15 +154,6 @@ IStatus refresh(boolean notify, Collection filePaths, IProgressMonitor mo } this.notify = notify; - final Set filePathStrings = new HashSet(CollectionsUtil.map(filePaths, - new IMap() - { - public String map(IPath location) - { - return location.toPortableString(); - } - })); - // If we don't run this, we end up showing files as unstaged when they're no longer modified! IStatus result; synchronized (this) @@ -182,10 +173,19 @@ public String map(IPath location) return result; } - Set> jobs = new HashSet>(3); - jobs.add(new UntrackedFilesRefreshJob(this, filePathStrings)); - jobs.add(new UnstagedFilesRefreshJob(this, filePathStrings)); - jobs.add(new StagedFilesRefreshJob(this, filePathStrings)); + final Set portablePathStrings = new HashSet(CollectionsUtil.map(filePaths, + new IMap() + { + public String map(IPath item) + { + return item.toPortableString(); + } + })); + + Set>> jobs = new HashSet>>(3); + jobs.add(new UntrackedFilesRefreshJob(this, portablePathStrings)); + jobs.add(new UnstagedFilesRefreshJob(this, portablePathStrings)); + jobs.add(new StagedFilesRefreshJob(this, portablePathStrings)); // Last chance to cancel... if (monitor != null && monitor.isCanceled()) @@ -194,7 +194,7 @@ public String map(IPath location) } // Now create a new temporary list so we can build it up... - this.files = Collections.synchronizedList(new ArrayList()); + Map newChangedFiles = new HashMap(); // Schedule all the jobs MultiStatus errors = new MultiStatus(GitPlugin.PLUGIN_ID, 1, @@ -205,10 +205,10 @@ public String map(IPath location) { return Status.CANCEL_STATUS; } - List> futures = es.invokeAll(jobs); + List>> futures = es.invokeAll(jobs); // Now wait for them to finish - for (Future future : futures) + for (Future> future : futures) { while (!future.isDone()) { @@ -222,10 +222,29 @@ public String map(IPath location) // When done, get their result try { - IStatus futureResult = future.get(); - if (!futureResult.isOK()) + Map map = future.get(); + if (newChangedFiles.isEmpty()) + { + newChangedFiles.putAll(map); + } + else { - errors.merge(futureResult); + // we may get multiple entries for the same path, we need to be careful and merge them together + // (i.e. we may have staged and unstaged changes for same file) + for (Entry entry : map.entrySet()) + { + IPath path = entry.getKey(); + ChangedFile file = entry.getValue(); + if (newChangedFiles.containsKey(path)) + { + // Merge the two entries + newChangedFiles.put(path, file.merge(newChangedFiles.get(path))); + } + else + { + newChangedFiles.put(path, file); + } + } } } catch (CancellationException ce) @@ -235,7 +254,14 @@ public String map(IPath location) catch (ExecutionException e) { Throwable t = e.getCause(); - errors.merge(new Status(IStatus.ERROR, GitPlugin.PLUGIN_ID, t.getMessage(), t)); + if (t instanceof CoreException) + { + errors.merge(((CoreException) t).getStatus()); + } + else + { + errors.merge(new Status(IStatus.ERROR, GitPlugin.PLUGIN_ID, t.getMessage(), t)); + } } } } @@ -251,12 +277,13 @@ public String map(IPath location) Collection preRefresh; synchronized (this.changedFilesLock) { + // Make a copy of the changed file listing, pre-refresh if (this.changedFiles != null) { preRefresh = new ArrayList(this.changedFiles.size()); for (ChangedFile file : this.changedFiles) { - preRefresh.add(new ChangedFile(file)); + preRefresh.add(file.clone()); } } else @@ -266,7 +293,7 @@ public String map(IPath location) // Now wipe any existing ChangedFile entries for any of the filePaths and add the ones we generated in // dictionary - if (CollectionsUtil.isEmpty(filePathStrings)) + if (CollectionsUtil.isEmpty(filePaths)) { this.changedFiles = new ArrayList(); } @@ -276,18 +303,18 @@ public String map(IPath location) { public boolean include(ChangedFile item) { - return !filePathStrings.contains(item.portablePath); + return !portablePathStrings.contains(item.getRelativePath().toPortableString()); } }); } - if (!CollectionsUtil.isEmpty(this.files)) + if (!CollectionsUtil.isEmpty(newChangedFiles)) { - this.changedFiles.addAll(this.files); + this.changedFiles.addAll(newChangedFiles.values()); } } // Don't hold onto temp list in memory! - this.files = null; + newChangedFiles = null; postIndexChange(preRefresh, this.changedFiles); sub.done(); @@ -346,7 +373,7 @@ public List changedFiles() List copy = new ArrayList(this.changedFiles.size()); for (ChangedFile file : this.changedFiles) { - copy.add(new ChangedFile(file)); + copy.add(file.clone()); } return copy; } @@ -360,10 +387,11 @@ public IStatus stageFiles(Collection stageFiles) return Status.OK_STATUS; } - StringBuffer input = new StringBuffer(stageFiles.size() * stageFiles.iterator().next().getPath().length()); + StringBuffer input = new StringBuffer(stageFiles.size() + * stageFiles.iterator().next().getRelativePath().toPortableString().length()); for (ChangedFile file : stageFiles) { - input.append(file.getPath()).append('\n'); + input.append(file.getRelativePath()).append('\n'); } @SuppressWarnings("nls") @@ -384,7 +412,7 @@ public IStatus stageFiles(Collection stageFiles) // files. for (ChangedFile file : stageFiles) { - preFiles.add(new ChangedFile(file)); + preFiles.add(file.clone()); synchronized (changedFilesLock) { if (this.changedFiles != null) @@ -394,14 +422,12 @@ public IStatus stageFiles(Collection stageFiles) { ChangedFile orig = this.changedFiles.get(index); - orig.hasUnstagedChanges = false; - orig.hasStagedChanges = true; + orig.makeStaged(); } } } - file.hasUnstagedChanges = false; - file.hasStagedChanges = true; + file.makeStaged(); } preFiles.trimToSize(); @@ -440,7 +466,7 @@ public IStatus unstageFiles(Collection unstageFiles) ArrayList preFiles = new ArrayList(unstageFiles.size()); for (ChangedFile file : unstageFiles) { - preFiles.add(new ChangedFile(file)); + preFiles.add(file.clone()); synchronized (this.changedFilesLock) { @@ -451,14 +477,12 @@ public IStatus unstageFiles(Collection unstageFiles) { ChangedFile orig = this.changedFiles.get(index); - orig.hasUnstagedChanges = true; - orig.hasStagedChanges = false; + orig.makeUnstaged(); } } } - file.hasUnstagedChanges = true; - file.hasStagedChanges = false; + file.makeUnstaged(); } preFiles.trimToSize(); @@ -471,7 +495,7 @@ public IStatus discardChangesForFiles(Collection discardFiles) StringBuilder input = new StringBuilder(); for (ChangedFile file : discardFiles) { - input.append(file.getPath()).append(NULL_DELIMITER); + input.append(file.getRelativePath().toPortableString()).append(NULL_DELIMITER); } IStatus result = repository.executeWithInput(input.toString(), @@ -490,8 +514,8 @@ public IStatus discardChangesForFiles(Collection discardFiles) ArrayList preFiles = new ArrayList(discardFiles.size()); for (ChangedFile file : discardFiles) { - preFiles.add(new ChangedFile(file)); - file.hasUnstagedChanges = false; + preFiles.add(file.clone()); + file.setUnstaged(false); } preFiles.trimToSize(); @@ -573,7 +597,7 @@ public String diffForFile(ChangedFile file, boolean staged, int contextLines) String parameter = "-U" + contextLines; //$NON-NLS-1$ if (staged) { - String indexPath = ":0:" + file.portablePath; //$NON-NLS-1$ + String indexPath = ":0:" + file.getRelativePath().toPortableString(); //$NON-NLS-1$ if (file.status == ChangedFile.Status.NEW) { @@ -582,7 +606,7 @@ public String diffForFile(ChangedFile file, boolean staged, int contextLines) } IStatus result = repository.execute(GitRepository.ReadWrite.READ, "diff-index", parameter, "--cached", //$NON-NLS-1$ //$NON-NLS-2$ - GitRepository.HEAD, "--", file.portablePath); //$NON-NLS-1$ + GitRepository.HEAD, "--", file.getRelativePath().toPortableString()); //$NON-NLS-1$ if (result == null || !result.isOK()) { return null; @@ -595,8 +619,9 @@ public String diffForFile(ChangedFile file, boolean staged, int contextLines) { try { - return IOUtil.read(new FileInputStream(workingDirectory().append(file.portablePath).toFile()), - IOUtil.UTF_8); // $codepro.audit.disable + return IOUtil.read( + new FileInputStream(workingDirectory().append(file.getRelativePath().toPortableString()) + .toFile()), IOUtil.UTF_8); // $codepro.audit.disable // closeWhereCreated } catch (FileNotFoundException e) @@ -606,13 +631,14 @@ public String diffForFile(ChangedFile file, boolean staged, int contextLines) } IStatus result = repository.execute(GitRepository.ReadWrite.READ, - "diff-files", parameter, "--", file.portablePath); //$NON-NLS-1$ //$NON-NLS-2$ + "diff-files", parameter, "--", file.getRelativePath().toPortableString()); //$NON-NLS-1$ //$NON-NLS-2$ return result.getMessage(); } public boolean hasBinaryAttributes(ChangedFile file) { - IStatus result = repository.execute(GitRepository.ReadWrite.READ, "check-attr", "binary", file.getPath()); //$NON-NLS-1$ //$NON-NLS-2$ + IStatus result = repository.execute(GitRepository.ReadWrite.READ, + "check-attr", "binary", file.getRelativePath().toPortableString()); //$NON-NLS-1$ //$NON-NLS-2$ String output = result.getMessage(); output = output.trim(); if (output.endsWith("binary: set")) //$NON-NLS-1$ @@ -625,12 +651,16 @@ public boolean hasBinaryAttributes(ChangedFile file) } if (output.endsWith("binary: unspecified")) //$NON-NLS-1$ { - // try common filename-extensions - for (String extension : BINARY_EXTENSIONS) + String fileExtension = file.getRelativePath().getFileExtension(); + if (fileExtension != null) { - if (file.getPath().endsWith(extension)) + // try common filename-extensions + for (String extension : BINARY_EXTENSIONS) { - return true; + if (fileExtension.equalsIgnoreCase(extension)) + { + return true; + } } } } @@ -650,12 +680,12 @@ protected boolean resourceOrChildHasChanges(IResource resource) { return false; } - + // FIXME Can we sort the changed files by path or something to help speed up this lookup? IPath workingDirectory = repository.workingDirectory(); IPath resourcePath = resource.getLocation(); for (ChangedFile changedFile : changedFiles) { - IPath fullPath = workingDirectory.append(changedFile.getPath()).makeAbsolute(); + IPath fullPath = workingDirectory.append(changedFile.getRelativePath()).makeAbsolute(); if (resourcePath.isPrefixOf(fullPath)) { return true; @@ -708,7 +738,7 @@ public Set getChangedResources() IFile getResourceForChangedFile(ChangedFile changedFile) { return ResourcesPlugin.getWorkspace().getRoot() - .getFileForLocation(workingDirectory().append(changedFile.getPath())); + .getFileForLocation(workingDirectory().append(changedFile.getRelativePath())); } protected ChangedFile getChangedFileForResource(IResource resource) @@ -720,11 +750,12 @@ protected ChangedFile getChangedFileForResource(IResource resource) IPath resourcePath = resource.getLocation(); List changedFiles = changedFiles(); + // FIXME Doing searches like this every time if (!CollectionsUtil.isEmpty(changedFiles)) { for (ChangedFile changedFile : changedFiles) { - IPath fullPath = workingDirectory().append(changedFile.getPath()); + IPath fullPath = workingDirectory().append(changedFile.getRelativePath()); if (resourcePath.equals(fullPath)) { return changedFile; @@ -763,7 +794,7 @@ protected List getChangedFilesForContainer(IContainer container) IPath workingDirectory = repository.workingDirectory(); for (ChangedFile changedFile : changedFiles) { - IPath fullPath = workingDirectory.append(changedFile.getPath()).makeAbsolute(); + IPath fullPath = workingDirectory.append(changedFile.getRelativePath()).makeAbsolute(); if (resourcePath.isPrefixOf(fullPath)) { filtered.add(changedFile); @@ -788,7 +819,7 @@ public void refreshAsync(final Collection paths) } } - private abstract static class FilesRefreshJob implements Callable + private abstract class FilesRefreshJob implements Callable> { protected GitRepository repo; @@ -824,164 +855,92 @@ protected List linesFromNotification(String string) return StringUtil.tokenize(string, NULL_DELIMITER); } - protected Map> dictionaryForLines(List lines) + protected Map> dictionaryForLines(List lines) { - Map> dictionary = new HashMap>(lines.size() / 2); + Map> dictionary = new HashMap>(lines.size() / 2); // Fill the dictionary with the new information. These lines are in the form of: // :00000 :0644 OTHER INDEX INFORMATION // Filename - Assert.isTrue(lines.size() % 2 == 0, "Lines must have an even number of lines: " + lines); //$NON-NLS-1$ + Assert.isTrue(lines.size() % 2 == 0, "Must have an even number of lines: " + lines); //$NON-NLS-1$ Iterator iter = lines.iterator(); while (iter.hasNext()) { String fileStatus = iter.next(); String fileName = iter.next(); - dictionary.put(fileName, StringUtil.tokenize(fileStatus, " ")); //$NON-NLS-1$ + dictionary.put(Path.fromPortableString(fileName), StringUtil.tokenize(fileStatus, " ")); //$NON-NLS-1$ } return dictionary; } - protected void addFilesFromDictionary(Map> dictionary, boolean staged, boolean tracked) + protected Map addFilesFromDictionary(final Map> dictionary, + final boolean staged, final boolean tracked) { - if (index.files == null) - { - return; - } - // Iterate over all existing files - synchronized (index.files) - { - for (ChangedFile file : index.files) - { - synchronized (dictionary) - { - List fileStatus = dictionary.get(file.portablePath); - // Object found, this is still a cached / uncached thing - if (fileStatus != null) - { - if (tracked) - { - String mode = fileStatus.get(0).substring(1); - String sha = fileStatus.get(2); - file.commitBlobSHA = sha; - file.commitBlobMode = mode; - - if (staged) - { - file.hasStagedChanges = true; - } - else - { - file.hasUnstagedChanges = true; - } - if (fileStatus.get(4).equals(DELETED_STATUS)) - { - file.status = ChangedFile.Status.DELETED; - } - else if (fileStatus.get(4).equals(UNMERGED_STATUS)) - { - file.status = ChangedFile.Status.UNMERGED; - } - } - else - { - // Untracked file, set status to NEW, only unstaged changes - file.hasStagedChanges = false; - file.hasUnstagedChanges = true; - file.status = ChangedFile.Status.NEW; - } - - // We handled this file, remove it from the dictionary - dictionary.remove(file.portablePath); - } - else - { - // Object not found in the dictionary, so let's reset its appropriate - // change (stage or untracked) if necessary. - - // Staged dictionary, so file does not have staged changes - if (staged) - { - file.hasStagedChanges = false; - } - // Tracked file does not have unstaged changes, file is not new, - // so we can set it to No. (If it would be new, it would not - // be in this dictionary, but in the "other dictionary"). - else if (tracked && file.status != ChangedFile.Status.NEW) - { - file.hasUnstagedChanges = false; - } - // Unstaged, untracked dictionary ("Other" files), and file - // is indicated as new (which would be untracked), so let's - // remove it - else if (!tracked && file.status == ChangedFile.Status.NEW) - { - file.hasUnstagedChanges = false; - } - } - } - } - } // Do new files only if necessary - if (dictionary.isEmpty()) + if (dictionary == null || dictionary.isEmpty()) { - return; + return Collections.emptyMap(); } // All entries left in the dictionary haven't been accounted for // above, so we need to add them to the "files" array - synchronized (dictionary) + Map result = new HashMap(dictionary.size()); + for (Map.Entry> entry : dictionary.entrySet()) { - for (String path : dictionary.keySet()) + IPath path = entry.getKey(); + // try a simple check for a sane path here + if (path.isAbsolute()) { - List fileStatus = dictionary.get(path); + IdeLog.logWarning( + GitPlugin.getDefault(), + MessageFormat + .format("Found an entry for an absoolute path ({0}), won't add to our changed file listing for repo at {1}", + path.toOSString(), workingDirectory().toOSString())); + continue; + } - ChangedFile.Status status = ChangedFile.Status.MODIFIED; - if (fileStatus.get(4).equals(DELETED_STATUS)) - { - status = ChangedFile.Status.DELETED; - } - else if (fileStatus.get(4).equals(UNMERGED_STATUS)) - { - status = ChangedFile.Status.UNMERGED; - } - else if (fileStatus.get(0).equals(":000000")) //$NON-NLS-1$ - { - status = ChangedFile.Status.NEW; - } - else - { - status = ChangedFile.Status.MODIFIED; - } + List fileStatus = entry.getValue(); - ChangedFile file = new ChangedFile(path, status); - if (tracked) - { - file.commitBlobMode = fileStatus.get(0).substring(1); - file.commitBlobSHA = fileStatus.get(2); - } + ChangedFile.Status status = ChangedFile.Status.MODIFIED; + if (fileStatus.get(4).equals(DELETED_STATUS)) + { + status = ChangedFile.Status.DELETED; + } + else if (fileStatus.get(4).equals(UNMERGED_STATUS)) + { + status = ChangedFile.Status.UNMERGED; + } + else if (fileStatus.get(0).equals(":000000")) //$NON-NLS-1$ + { + status = ChangedFile.Status.NEW; + } + else + { + status = ChangedFile.Status.MODIFIED; + } - file.hasStagedChanges = staged; - file.hasUnstagedChanges = !staged; - synchronized (index.files) - { - index.files.add(file); - } + String mode = null; + String sha = null; + if (tracked) + { + mode = fileStatus.get(0).substring(1); + sha = fileStatus.get(2); } + result.put(path, new ChangedFile(repository, path, status, mode, sha, staged, !staged)); } + return result; } - } - private static final class StagedFilesRefreshJob extends FilesRefreshJob + private final class StagedFilesRefreshJob extends FilesRefreshJob { private StagedFilesRefreshJob(GitIndex index, Set filePaths) { super(index, filePaths); } - public IStatus call() throws Exception + public Map call() throws Exception { // HEAD vs filesystem List args = CollectionsUtil.newList("diff-index", "--cached", //$NON-NLS-1$ //$NON-NLS-2$ @@ -995,27 +954,35 @@ public IStatus call() throws Exception IStatus result = repo.execute(GitRepository.ReadWrite.READ, args.toArray(new String[args.size()])); if (result != null && result.isOK()) { - readStagedFiles(result.getMessage()); + return readStagedFiles(result.getMessage()); } - return Status.OK_STATUS; + // We can get an error if this is a brand new repo with no commits (or staged changes) + if (result != null + && result.getCode() == 128 + && result.getMessage().startsWith( + "fatal: ambiguous argument 'HEAD': unknown revision or path not in the working tree")) + { + return Collections.emptyMap(); + } + throw new CoreException(result); } - private void readStagedFiles(String string) + private Map readStagedFiles(String string) { List lines = linesFromNotification(string); - Map> dic = dictionaryForLines(lines); - addFilesFromDictionary(dic, true, true); + Map> dic = dictionaryForLines(lines); + return addFilesFromDictionary(dic, true, true); } } - private static final class UnstagedFilesRefreshJob extends FilesRefreshJob + private final class UnstagedFilesRefreshJob extends FilesRefreshJob { private UnstagedFilesRefreshJob(GitIndex index, Set filePaths) { super(index, filePaths); } - public IStatus call() throws Exception + public Map call() throws Exception { // index vs filesystem List args = CollectionsUtil.newList("diff-files", "-z"); //$NON-NLS-1$ //$NON-NLS-2$ @@ -1028,20 +995,20 @@ public IStatus call() throws Exception IStatus result = repo.execute(GitRepository.ReadWrite.READ, args.toArray(new String[args.size()])); if (result != null && result.isOK()) { - readUnstagedFiles(result.getMessage()); + return readUnstagedFiles(result.getMessage()); } - return Status.OK_STATUS; + throw new CoreException(result); } - private void readUnstagedFiles(String string) + private Map readUnstagedFiles(String string) { List lines = linesFromNotification(string); - Map> dic = dictionaryForLines(lines); - addFilesFromDictionary(dic, false, true); + Map> dic = dictionaryForLines(lines); + return addFilesFromDictionary(dic, false, true); } } - private static final class UntrackedFilesRefreshJob extends FilesRefreshJob + private final class UntrackedFilesRefreshJob extends FilesRefreshJob { private UntrackedFilesRefreshJob(GitIndex index, Set filePaths) @@ -1049,7 +1016,7 @@ private UntrackedFilesRefreshJob(GitIndex index, Set filePaths) super(index, filePaths); } - public IStatus call() throws Exception + public Map call() throws Exception { // index vs working tree (HEAD?) List args = CollectionsUtil.newList("ls-files", "--others", //$NON-NLS-1$ //$NON-NLS-2$ @@ -1063,15 +1030,15 @@ public IStatus call() throws Exception IStatus result = repo.execute(GitRepository.ReadWrite.READ, args.toArray(new String[args.size()])); if (result != null && result.isOK()) { - readOtherFiles(result.getMessage()); + return readOtherFiles(result.getMessage()); } - return Status.OK_STATUS; + throw new CoreException(result); } - private void readOtherFiles(String string) + private Map readOtherFiles(String string) { List lines = linesFromNotification(string); - Map> dictionary = new HashMap>(lines.size()); + Map> dictionary = new HashMap>(lines.size()); // Other files are untracked, so we don't have any real index information. Instead, we can just fake it. // The line below is not used at all, as for these files the commitBlob isn't set List fileStatus = CollectionsUtil.newList(":000000", // for new file //$NON-NLS-1$ @@ -1086,10 +1053,10 @@ private void readOtherFiles(String string) { continue; } - dictionary.put(path, fileStatus); + dictionary.put(Path.fromPortableString(path), fileStatus); } - addFilesFromDictionary(dictionary, false, false); + return addFilesFromDictionary(dictionary, false, false); } } diff --git a/plugins/com.aptana.git.core/src/com/aptana/git/core/model/GitRepository.java b/plugins/com.aptana.git.core/src/com/aptana/git/core/model/GitRepository.java index f038c9ef80..586cedbaa8 100644 --- a/plugins/com.aptana.git.core/src/com/aptana/git/core/model/GitRepository.java +++ b/plugins/com.aptana.git.core/src/com/aptana/git/core/model/GitRepository.java @@ -1495,9 +1495,9 @@ public boolean validRefName(String refName) return result != null && result.isOK(); } - public IStatus deleteFile(String filePath) + public IStatus deleteFile(IPath filePath) { - IStatus result = execute(GitRepository.ReadWrite.WRITE, "rm", "-f", filePath); //$NON-NLS-1$ //$NON-NLS-2$ + IStatus result = execute(GitRepository.ReadWrite.WRITE, "rm", "-f", filePath.toPortableString()); //$NON-NLS-1$ //$NON-NLS-2$ if (result == null) { return new Status(IStatus.ERROR, GitPlugin.getPluginId(), "Failed to execute git rm -f"); //$NON-NLS-1$ diff --git a/plugins/com.aptana.git.core/src/com/aptana/git/core/model/IndexChangedEvent.java b/plugins/com.aptana.git.core/src/com/aptana/git/core/model/IndexChangedEvent.java index 25e98f3e58..98cb806e8d 100644 --- a/plugins/com.aptana.git.core/src/com/aptana/git/core/model/IndexChangedEvent.java +++ b/plugins/com.aptana.git.core/src/com/aptana/git/core/model/IndexChangedEvent.java @@ -44,7 +44,7 @@ public Set getFilesWithChanges() Set files = new HashSet(changedFiles.size()); for (ChangedFile changedFile : changedFiles) { - IPath path = workingDirectory.append(changedFile.getPath()).makeAbsolute(); + IPath path = workingDirectory.append(changedFile.getRelativePath()).makeAbsolute(); IFile file = ResourcesPlugin.getWorkspace().getRoot().getFileForLocation(path); if (file == null) { diff --git a/plugins/com.aptana.git.ui/src/com/aptana/git/ui/DiffFormatter.java b/plugins/com.aptana.git.ui/src/com/aptana/git/ui/DiffFormatter.java index 8a26fad21d..2355029152 100644 --- a/plugins/com.aptana.git.ui/src/com/aptana/git/ui/DiffFormatter.java +++ b/plugins/com.aptana.git.ui/src/com/aptana/git/ui/DiffFormatter.java @@ -14,6 +14,7 @@ import java.util.regex.Pattern; import org.eclipse.core.runtime.FileLocator; +import org.eclipse.core.runtime.IPath; import org.eclipse.core.runtime.Path; import org.eclipse.core.runtime.Platform; @@ -165,16 +166,16 @@ private static String injectIntoTemplate(String html) * @param diffs * @return */ - public static String toHTML(Map diffs) + public static String toHTML(Map diffs) { if (diffs == null) { return StringUtil.EMPTY; } StringBuilder combined = new StringBuilder(); - for (Map.Entry diffMap : diffs.entrySet()) + for (Map.Entry diffMap : diffs.entrySet()) { - combined.append(convertDiff(diffMap.getKey(), diffMap.getValue())).append('\n'); + combined.append(convertDiff(diffMap.getKey().toOSString(), diffMap.getValue())).append('\n'); } return injectIntoTemplate(combined.toString()); } diff --git a/plugins/com.aptana.git.ui/src/com/aptana/git/ui/internal/actions/CommitDialog.java b/plugins/com.aptana.git.ui/src/com/aptana/git/ui/internal/actions/CommitDialog.java index 11098727c2..1a0a2a45b2 100644 --- a/plugins/com.aptana.git.ui/src/com/aptana/git/ui/internal/actions/CommitDialog.java +++ b/plugins/com.aptana.git.ui/src/com/aptana/git/ui/internal/actions/CommitDialog.java @@ -634,7 +634,7 @@ public void widgetSelected(SelectionEvent e) final List copy = new ArrayList(changedFiles); for (ChangedFile cf : changedFiles) { - copy.add(new ChangedFile(cf)); + copy.add(cf.clone()); } gitRepository.index().discardChangesForFiles(changedFiles); @@ -774,14 +774,18 @@ private void updateDiff(final boolean staged, ChangedFile file) && !file.getStatus().equals(ChangedFile.Status.DELETED)) { // Special code to draw the image if the binary file is an image - String[] imageExtensions = new String[] { ".png", ".gif", ".jpeg", ".jpg", ".ico" }; //$NON-NLS-1$ //$NON-NLS-2$ //$NON-NLS-3$ //$NON-NLS-4$ //$NON-NLS-5$ - for (String extension : imageExtensions) + String fileExtension = file.getRelativePath().getFileExtension(); + if (fileExtension != null) { - if (file.getPath().endsWith(extension)) + String[] imageExtensions = new String[] { "png", "gif", "jpeg", "jpg", "ico" }; //$NON-NLS-1$ //$NON-NLS-2$ //$NON-NLS-3$ //$NON-NLS-4$ //$NON-NLS-5$ + for (String extension : imageExtensions) { - IPath fullPath = gitRepository.workingDirectory().append(file.getPath()); - updateDiff(file, ""); //$NON-NLS-1$ //$NON-NLS-2$ - return; + if (fileExtension.equalsIgnoreCase(extension)) + { + IPath fullPath = gitRepository.workingDirectory().append(file.getRelativePath()); + updateDiff(file, ""); //$NON-NLS-1$ //$NON-NLS-2$ + return; + } } } } @@ -796,7 +800,7 @@ private void updateDiff(final boolean staged, ChangedFile file) { try { - diff = DiffFormatter.toHTML(file.getPath(), diff); + diff = DiffFormatter.toHTML(file.getRelativePath().toPortableString(), diff); } catch (Throwable t) { @@ -839,7 +843,7 @@ private void createTableItem(Table table, ChangedFile file, boolean sort) for (TableItem existing : items) { String path = existing.getText(1); - if (file.getPath().compareTo(path) < 0) + if (file.getRelativePath().toOSString().compareTo(path) < 0) { break; } @@ -866,7 +870,7 @@ else if (file.getStatus() == ChangedFile.Status.NEW) } // item.setText(0, text); item.setImage(0, image); - item.setText(1, file.getPath()); + item.setText(1, file.getRelativePath().toOSString()); item.setData(CHANGED_FILE_DATA_KEY, file); } diff --git a/plugins/com.aptana.git.ui/src/com/aptana/git/ui/internal/actions/CompareWithHEADHandler.java b/plugins/com.aptana.git.ui/src/com/aptana/git/ui/internal/actions/CompareWithHEADHandler.java index 269a2e011e..65dca5a7ec 100644 --- a/plugins/com.aptana.git.ui/src/com/aptana/git/ui/internal/actions/CompareWithHEADHandler.java +++ b/plugins/com.aptana.git.ui/src/com/aptana/git/ui/internal/actions/CompareWithHEADHandler.java @@ -15,7 +15,7 @@ import org.eclipse.core.commands.ExecutionException; import org.eclipse.core.resources.IFile; import org.eclipse.core.resources.IResource; -import org.eclipse.core.runtime.Path; +import org.eclipse.core.runtime.IPath; import org.eclipse.team.core.history.IFileRevision; import org.eclipse.team.internal.ui.history.FileRevisionTypedElement; import org.eclipse.team.ui.synchronize.SaveableCompareEditorInput; @@ -49,11 +49,10 @@ protected Object doExecute(ExecutionEvent event) throws ExecutionException { continue; } - String name = repo.getChangedFileForResource(blah).getPath(); + IPath name = repo.getChangedFileForResource(blah).getRelativePath(); IFile file = (IFile) blah; ITypedElement base = SaveableCompareEditorInput.createFileElement(file); - final IFileRevision nextFile = GitPlugin.revisionForCommit( - new GitCommit(repo, "HEAD"), Path.fromOSString(name)); //$NON-NLS-1$ + final IFileRevision nextFile = GitPlugin.revisionForCommit(new GitCommit(repo, "HEAD"), name); //$NON-NLS-1$ final ITypedElement next = new FileRevisionTypedElement(nextFile); final GitCompareFileRevisionEditorInput in = new GitCompareFileRevisionEditorInput(base, next, null); CompareUI.openCompareEditor(in); diff --git a/plugins/com.aptana.git.ui/src/com/aptana/git/ui/internal/actions/DiffHandler.java b/plugins/com.aptana.git.ui/src/com/aptana/git/ui/internal/actions/DiffHandler.java index 41c6ed8c59..9d0e1b1775 100644 --- a/plugins/com.aptana.git.ui/src/com/aptana/git/ui/internal/actions/DiffHandler.java +++ b/plugins/com.aptana.git.ui/src/com/aptana/git/ui/internal/actions/DiffHandler.java @@ -16,6 +16,7 @@ import org.eclipse.core.commands.ExecutionException; import org.eclipse.core.resources.IContainer; import org.eclipse.core.resources.IResource; +import org.eclipse.core.runtime.IPath; import org.eclipse.jface.dialogs.IDialogConstants; import org.eclipse.jface.dialogs.MessageDialog; import org.eclipse.swt.SWT; @@ -95,7 +96,7 @@ private List getChangedFilesForResource(GitRepository repo, IResour @Override protected Object doExecute(ExecutionEvent event) throws ExecutionException { - Map diffs = new HashMap(); + Map diffs = new HashMap(); List changedFiles = getSelectedChangedFiles(); if (changedFiles == null || changedFiles.isEmpty()) { @@ -114,12 +115,12 @@ protected Object doExecute(ExecutionEvent event) throws ExecutionException continue; } - if (diffs.containsKey(file.getPath())) + if (diffs.containsKey(file.getRelativePath())) { continue; // already calculated diff... } String diff = repo.index().diffForFile(file, file.hasStagedChanges(), 3); - diffs.put(file.getPath(), diff); + diffs.put(file.getRelativePath(), diff); } if (diffs.isEmpty()) { diff --git a/plugins/com.aptana.git.ui/src/com/aptana/git/ui/internal/actions/MergeConflictsHandler.java b/plugins/com.aptana.git.ui/src/com/aptana/git/ui/internal/actions/MergeConflictsHandler.java index 828def4a57..30b4092084 100644 --- a/plugins/com.aptana.git.ui/src/com/aptana/git/ui/internal/actions/MergeConflictsHandler.java +++ b/plugins/com.aptana.git.ui/src/com/aptana/git/ui/internal/actions/MergeConflictsHandler.java @@ -20,7 +20,6 @@ import org.eclipse.core.runtime.CoreException; import org.eclipse.core.runtime.IPath; import org.eclipse.core.runtime.NullProgressMonitor; -import org.eclipse.core.runtime.Path; import org.eclipse.team.core.history.IFileRevision; import org.eclipse.team.internal.ui.history.FileRevisionTypedElement; import org.eclipse.team.ui.synchronize.SaveableCompareEditorInput; @@ -58,7 +57,7 @@ protected Object doExecute(ExecutionEvent event) throws ExecutionException return null; } - String name = repo.getChangedFileForResource(blah).getPath(); + IPath name = repo.getChangedFileForResource(blah).getRelativePath(); IFile file = (IFile) blah; try { @@ -69,8 +68,7 @@ protected Object doExecute(ExecutionEvent event) throws ExecutionException // We create a copy of the failed merged file with the markers as generated by Git file.copy(copyPath, true, new NullProgressMonitor()); // Then we replace the working file's contents by the contents pre-merge - final IFileRevision baseFile = GitPlugin.revisionForCommit( - new GitCommit(repo, ":2"), Path.fromOSString(name)); //$NON-NLS-1$ + final IFileRevision baseFile = GitPlugin.revisionForCommit(new GitCommit(repo, ":2"), name); //$NON-NLS-1$ IStorage storage = baseFile.getStorage(new NullProgressMonitor()); file.setContents(storage.getContents(), true, true, new NullProgressMonitor()); file.setCharset(IOUtil.UTF_8, new NullProgressMonitor()); @@ -83,7 +81,7 @@ protected Object doExecute(ExecutionEvent event) throws ExecutionException } // Now we use the pre-merge file and compare against the merging version. ITypedElement base = SaveableCompareEditorInput.createFileElement(file); - final IFileRevision nextFile = GitPlugin.revisionForCommit(new GitCommit(repo, ":3"), Path.fromOSString(name)); //$NON-NLS-1$ + final IFileRevision nextFile = GitPlugin.revisionForCommit(new GitCommit(repo, ":3"), name); //$NON-NLS-1$ final ITypedElement next = new FileRevisionTypedElement(nextFile); final GitCompareFileRevisionEditorInput in = new GitCompareFileRevisionEditorInput(base, next, null); CompareUI.openCompareEditor(in); diff --git a/plugins/com.aptana.git.ui/src/com/aptana/git/ui/internal/actions/RevertHandler.java b/plugins/com.aptana.git.ui/src/com/aptana/git/ui/internal/actions/RevertHandler.java index 7894a717ee..065f414422 100644 --- a/plugins/com.aptana.git.ui/src/com/aptana/git/ui/internal/actions/RevertHandler.java +++ b/plugins/com.aptana.git.ui/src/com/aptana/git/ui/internal/actions/RevertHandler.java @@ -36,7 +36,8 @@ protected void doOperation(GitRepository repo, List changedFiles) File file = new File(resource.getLocationURI()); for (ChangedFile changedFile : changedFiles) { - if (file.getAbsolutePath().endsWith(changedFile.getPath())) + // FIXME We should be able to ask for the full path for the changed file by appending to the git repo's working directory! + if (file.getAbsolutePath().endsWith(changedFile.getRelativePath().toOSString())) { changedResources.add(resource); break; diff --git a/tests/com.aptana.git.core.tests/src/com/aptana/git/core/GitMoveDeleteHookTest.java b/tests/com.aptana.git.core.tests/src/com/aptana/git/core/GitMoveDeleteHookTest.java index 4a4817b3ce..ea63d575f3 100644 --- a/tests/com.aptana.git.core.tests/src/com/aptana/git/core/GitMoveDeleteHookTest.java +++ b/tests/com.aptana.git.core.tests/src/com/aptana/git/core/GitMoveDeleteHookTest.java @@ -104,13 +104,14 @@ public void testDeleteFileUnforcedWithHistorySucceeds() oneOf(file).getProject(); oneOf(repo).getChangedFileForResource(file); - ChangedFile changedFile = new ChangedFile("fake_path.txt", ChangedFile.Status.MODIFIED); + ChangedFile changedFile = new ChangedFile(null, Path.fromPortableString("fake_path.txt"), + ChangedFile.Status.MODIFIED, null, null, false, false); will(returnValue(changedFile)); // keep history oneOf(tree).addToLocalHistory(file); - oneOf(repo).deleteFile(changedFile.getPath()); + oneOf(repo).deleteFile(changedFile.getRelativePath()); will(returnValue(org.eclipse.core.runtime.Status.OK_STATUS)); // repo says we deleted ok, so we should mark that on the tree oneOf(tree).deletedFile(file); @@ -133,7 +134,8 @@ public void testDeleteFileThatIsNewDoesntGoThroughRepo() oneOf(file).getProject(); oneOf(repo).getChangedFileForResource(file); - ChangedFile changedFile = new ChangedFile("fake_path.txt", ChangedFile.Status.NEW); + ChangedFile changedFile = new ChangedFile(null, Path.fromPortableString("fake_path.txt"), + ChangedFile.Status.NEW, null, null, false, false); will(returnValue(changedFile)); } }); @@ -178,10 +180,11 @@ public void testDeleteFileFailsInGitRepo() oneOf(file).getProject(); oneOf(repo).getChangedFileForResource(file); - ChangedFile changedFile = new ChangedFile("fake_path.txt", ChangedFile.Status.MODIFIED); + ChangedFile changedFile = new ChangedFile(null, Path.fromPortableString("fake_path.txt"), + ChangedFile.Status.MODIFIED, null, null, false, false); will(returnValue(changedFile)); - oneOf(repo).deleteFile(changedFile.getPath()); + oneOf(repo).deleteFile(changedFile.getRelativePath()); IStatus status = org.eclipse.core.runtime.Status.CANCEL_STATUS; will(returnValue(status)); // repo says we deleted ok, so we should mark that on the tree diff --git a/tests/com.aptana.git.core.tests/src/com/aptana/git/core/GitMoveDeleteIntegrationTest.java b/tests/com.aptana.git.core.tests/src/com/aptana/git/core/GitMoveDeleteIntegrationTest.java index 45889b8553..b6f81e9954 100644 --- a/tests/com.aptana.git.core.tests/src/com/aptana/git/core/GitMoveDeleteIntegrationTest.java +++ b/tests/com.aptana.git.core.tests/src/com/aptana/git/core/GitMoveDeleteIntegrationTest.java @@ -21,6 +21,7 @@ import org.eclipse.core.runtime.CoreException; import org.eclipse.core.runtime.IPath; import org.eclipse.core.runtime.NullProgressMonitor; +import org.eclipse.core.runtime.Path; import org.eclipse.core.runtime.preferences.IEclipsePreferences; import org.junit.Test; import org.osgi.service.prefs.BackingStoreException; @@ -89,7 +90,8 @@ private synchronized IProject getProject() throws CoreException } return fProject; } -@Test + + @Test public void testDeleteNewUnstagedFile() throws Exception { IFile file = getProject().getFile("newfile.txt"); @@ -100,7 +102,7 @@ public void testDeleteNewUnstagedFile() throws Exception // TODO Assert that we didn't delete through repo } -@Test + @Test public void testDeleteStagedFile() throws Exception { IFile file = getProject().getFile("newfile2.txt"); @@ -122,14 +124,15 @@ protected void assertContains(String fileName, List changedFiles) assertTrue("changed files was empty", changedFiles.size() > 0); for (ChangedFile file : changedFiles) { - if (file.getPath().equals(fileName)) + if (file.getRelativePath().equals(Path.fromPortableString(fileName))) { return; } } fail("Didn't find " + fileName); } -@Test + + @Test public void testDeleteAlreadyCommittedFileWithNoChanges() throws Exception { IFile file = getProject().getFile("newfile3.txt"); @@ -147,7 +150,8 @@ public void testDeleteAlreadyCommittedFileWithNoChanges() throws Exception assertFalse(file.exists()); // TODO Assert that we did delete through repo } -@Test + + @Test public void testDeleteUnstagedAlreadyCommittedFile() throws Exception { IFile file = getProject().getFile("newfile4.txt"); diff --git a/tests/com.aptana.git.core.tests/src/com/aptana/git/core/model/GitIndexTest.java b/tests/com.aptana.git.core.tests/src/com/aptana/git/core/model/GitIndexTest.java index eafe5c6267..3c4da6d449 100644 --- a/tests/com.aptana.git.core.tests/src/com/aptana/git/core/model/GitIndexTest.java +++ b/tests/com.aptana.git.core.tests/src/com/aptana/git/core/model/GitIndexTest.java @@ -40,8 +40,8 @@ public void testStageFilesUpdatesStagedFlagsOnAffectedFiles() throws Exception // Generate faked unmerged file in index List blah = new ArrayList(); - ChangedFile changedFile = new ChangedFile(fileName, Status.UNMERGED); - changedFile.hasUnstagedChanges = true; + ChangedFile changedFile = new ChangedFile(null, Path.fromPortableString(fileName), Status.UNMERGED, null, null, + false, true); blah.add(changedFile); GitIndex index = new GitIndex(repo); index.changedFiles = blah; @@ -105,7 +105,10 @@ public void testUnstageFilesUpdatesStagedFlagsOnAffectedFiles() throws Exception // Now fake the new status as being unmerged List blah = index.changedFiles(); - blah.iterator().next().status = Status.UNMERGED; + ChangedFile orig = blah.remove(0); + ChangedFile unmerged = new ChangedFile(orig.getRepository(), orig.getRelativePath(), Status.UNMERGED, + orig.getCommitBlobMode(), orig.getCommitBlobSHA(), orig.hasStagedChanges(), orig.hasUnstagedChanges()); + blah.add(0, unmerged); index.changedFiles = blah; assertFalse(index.hasUnresolvedMergeConflicts()); @@ -200,8 +203,8 @@ public void testBatchRefreshRepoWithEveryStatus() throws Exception // Commit a couple files, then test staged delete, unstaged delete, untracked, staged mod, unstaged mod. // Delete 1 and 2 - repo.deleteFile("file1.txt"); - repo.deleteFile("file2.txt"); + repo.deleteFile(Path.fromPortableString("file1.txt")); + repo.deleteFile(Path.fromPortableString("file2.txt")); // Modify 3 and 4 writer = new FileWriter(repo.workingDirectory().append("file3.txt").toOSString(), true); @@ -229,7 +232,8 @@ public void testBatchRefreshRepoWithEveryStatus() throws Exception { public boolean include(ChangedFile item) { - return CollectionsUtil.newSet("file1.txt", "file3.txt", "file5.txt").contains(item.portablePath); + return CollectionsUtil.newSet("file1.txt", "file3.txt", "file5.txt").contains( + item.getRelativePath().toPortableString()); } }); assertStageFiles(repo.index(), toStage); @@ -239,7 +243,7 @@ public boolean include(ChangedFile item) { public boolean include(ChangedFile item) { - return CollectionsUtil.newSet("file2.txt").contains(item.portablePath); + return CollectionsUtil.newSet("file2.txt").contains(item.getRelativePath().toPortableString()); } }); assertUnstageFiles(repo.index(), toUnstage); @@ -284,8 +288,8 @@ public void testDiffRefreshRepoWithEveryStatus() throws Exception // Commit a couple files, then test staged delete, unstaged delete, untracked, staged mod, unstaged mod. // Delete 1 and 2 - repo.deleteFile("file1.txt"); - repo.deleteFile("file2.txt"); + repo.deleteFile(Path.fromPortableString("file1.txt")); + repo.deleteFile(Path.fromPortableString("file2.txt")); // Modify 3 and 4 writer = new FileWriter(repo.workingDirectory().append("file3.txt").toOSString(), true); @@ -313,7 +317,8 @@ public void testDiffRefreshRepoWithEveryStatus() throws Exception { public boolean include(ChangedFile item) { - return CollectionsUtil.newSet("file1.txt", "file3.txt", "file5.txt").contains(item.portablePath); + return CollectionsUtil.newSet("file1.txt", "file3.txt", "file5.txt").contains( + item.getRelativePath().toPortableString()); } }); assertStageFiles(repo.index(), toStage); @@ -323,7 +328,7 @@ public boolean include(ChangedFile item) { public boolean include(ChangedFile item) { - return CollectionsUtil.newSet("file2.txt").contains(item.portablePath); + return CollectionsUtil.newSet("file2.txt").contains(item.getRelativePath().toPortableString()); } }); assertUnstageFiles(repo.index(), toUnstage); @@ -352,8 +357,9 @@ private void assertContains(List files, final String path, final St public boolean include(ChangedFile item) { - return ObjectUtil.areEqual(item.portablePath, path) && item.hasStagedChanges == hasStaged - && item.hasUnstagedChanges == hasUnstaged && item.status == status; + return ObjectUtil.areEqual(item.getRelativePath().toPortableString(), path) + && item.hasStagedChanges() == hasStaged && item.hasUnstagedChanges() == hasUnstaged + && item.status == status; } }); List fileStrings = CollectionsUtil.map(files, new IMap() diff --git a/tests/com.aptana.git.core.tests/src/com/aptana/git/core/model/GitRepositoryTest.java b/tests/com.aptana.git.core.tests/src/com/aptana/git/core/model/GitRepositoryTest.java index e1485ef2da..4f85c8b4a6 100644 --- a/tests/com.aptana.git.core.tests/src/com/aptana/git/core/model/GitRepositoryTest.java +++ b/tests/com.aptana.git.core.tests/src/com/aptana/git/core/model/GitRepositoryTest.java @@ -33,6 +33,7 @@ import org.eclipse.core.runtime.IPath; import org.eclipse.core.runtime.IStatus; import org.eclipse.core.runtime.NullProgressMonitor; +import org.eclipse.core.runtime.Path; import org.junit.Test; import org.osgi.framework.Version; @@ -193,7 +194,7 @@ public void testMultipleLineCommitMessage() throws Throwable // Hold onto filename/path for getting it's history later. ChangedFile file = changed.get(0); - String filePath = file.getPath(); + IPath filePath = file.getRelativePath(); // stage assertStageFiles(index, changed); @@ -204,7 +205,8 @@ public void testMultipleLineCommitMessage() throws Throwable assertCommit(index, commitMessage); GitRevList list = new GitRevList(repo); - IStatus result = list.walkRevisionListWithSpecifier(new GitRevSpecifier(filePath), new NullProgressMonitor()); + IStatus result = list.walkRevisionListWithSpecifier(new GitRevSpecifier(filePath.toPortableString()), + new NullProgressMonitor()); assertTrue(result.isOK()); List commits = list.getCommits(); @@ -233,7 +235,7 @@ public void testDeleteFile() throws Throwable // make sure it's there first assertTrue("File we want to delete through git repo doesn't exist", addedFile.exists()); // delete it - IStatus status = getRepo().deleteFile(addedFile.getName()); + IStatus status = getRepo().deleteFile(Path.fromPortableString(addedFile.getName())); assertTrue(MessageFormat.format("Deleting file in git repo returned an error status: {0}", status), status.isOK()); // make sure its deleted from filesystem