motiejus
/
jgit
1
Fork 0
Code Releases Activity

Clarify WorkingTreeOptions and filemode usage 

To improve runtime performance, caching the WorkingTreeOptions inside
of the Config object using the Config.SectionParser API allows
the WorkingTreeOptions to be accessed more efficiently whenever a
FileTreeIterator is constructed for the Repository.

Instead of passing the filemode handling option into isModified(),
the WorkingTreeIterator should always honor whatever setting has
been configured in this repository, as defined by its own copy of
the WorkingTreeOptions.  This simplifies all of the callers as they
no longer need to lookup core.filemode on their own.

A few locations were changed from always using a hardcoded "true"
on the file mode to passing what is actually configured in the
repository.  This is a behavior change, but corrects what should be
considered to be bugs as the core.filemode variable wasn't always
being used.

Change-Id: Idb176736fa0dc97af372f1d652a94ecc72fb457c
Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
This commit is contained in:
Shawn O. Pearce 2010-12-07 18:13:36 -08:00
parent c181e1ab8a
commit 11fd0fe03a
10 changed files with 51 additions and 123 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

5
org.eclipse.jgit.pgm/src/org/eclipse/jgit/pgm/opt/AbstractTreeIteratorHandler.java
View File

@ -97,7 +97,10 @@ public int parseArguments(final Parameters params) throws CmdLineException {
final String name = params.getParameter(0);
if (new File(name).isDirectory()) {
setter.addValue(new FileTreeIterator(new File(name), FS.DETECTED, WorkingTreeOptions.createDefaultInstance()));
setter.addValue(new FileTreeIterator(
new File(name),
FS.DETECTED,
clp.getRepository().getConfig().get(WorkingTreeOptions.KEY)));
return 1;
}

4
org.eclipse.jgit.test/tst/org/eclipse/jgit/treewalk/AbstractTreeIteratorTest.java
View File

@ -49,10 +49,10 @@
import junit.framework.TestCase;
import org.eclipse.jgit.errors.IncorrectObjectTypeException;
import org.eclipse.jgit.lib.Config;
import org.eclipse.jgit.lib.Constants;
import org.eclipse.jgit.lib.FileMode;
import org.eclipse.jgit.lib.ObjectReader;
import org.eclipse.jgit.lib.CoreConfig.AutoCRLF;
public class AbstractTreeIteratorTest extends TestCase {
@ -63,7 +63,7 @@ private static String prefix(String path) {
public class FakeTreeIterator extends WorkingTreeIterator {
public FakeTreeIterator(String pathName, FileMode fileMode) {
super(prefix(pathName), new WorkingTreeOptions(AutoCRLF.FALSE));
super(prefix(pathName), new Config().get(WorkingTreeOptions.KEY));
mode = fileMode.getBits();
final int s = pathName.lastIndexOf('/');

10
org.eclipse.jgit.test/tst/org/eclipse/jgit/treewalk/FileTreeIteratorTest.java
View File

@ -80,7 +80,7 @@ public void testEmptyIfRootIsFile() throws Exception {
final File r = new File(trash, paths[0]);
assertTrue(r.isFile());
final FileTreeIterator fti = new FileTreeIterator(r, db.getFS(),
WorkingTreeOptions.createConfigurationInstance(db.getConfig()));
db.getConfig().get(WorkingTreeOptions.KEY));
assertTrue(fti.first());
assertTrue(fti.eof());
}
@ -89,7 +89,7 @@ public void testEmptyIfRootDoesNotExist() throws Exception {
final File r = new File(trash, "not-existing-file");
assertFalse(r.exists());
final FileTreeIterator fti = new FileTreeIterator(r, db.getFS(),
WorkingTreeOptions.createConfigurationInstance(db.getConfig()));
db.getConfig().get(WorkingTreeOptions.KEY));
assertTrue(fti.first());
assertTrue(fti.eof());
}
@ -101,14 +101,14 @@ public void testEmptyIfRootIsEmpty() throws Exception {
assertTrue(r.isDirectory());
final FileTreeIterator fti = new FileTreeIterator(r, db.getFS(),
WorkingTreeOptions.createConfigurationInstance(db.getConfig()));
db.getConfig().get(WorkingTreeOptions.KEY));
assertTrue(fti.first());
assertTrue(fti.eof());
}
public void testSimpleIterate() throws Exception {
final FileTreeIterator top = new FileTreeIterator(trash, db.getFS(),
WorkingTreeOptions.createConfigurationInstance(db.getConfig()));
db.getConfig().get(WorkingTreeOptions.KEY));
assertTrue(top.first());
assertFalse(top.eof());
@ -157,7 +157,7 @@ public void testSimpleIterate() throws Exception {
public void testComputeFileObjectId() throws Exception {
final FileTreeIterator top = new FileTreeIterator(trash, db.getFS(),
WorkingTreeOptions.createConfigurationInstance(db.getConfig()));
db.getConfig().get(WorkingTreeOptions.KEY));
final MessageDigest md = Constants.newMessageDigest();
md.update(Constants.encodeASCII(Constants.TYPE_BLOB));

4
org.eclipse.jgit.test/tst/org/eclipse/jgit/treewalk/FileTreeIteratorWithTimeControl.java
View File

@ -46,9 +46,9 @@
import java.util.SortedSet;
import java.util.TreeSet;
import org.eclipse.jgit.lib.Config;
import org.eclipse.jgit.lib.ObjectReader;
import org.eclipse.jgit.lib.Repository;
import org.eclipse.jgit.lib.CoreConfig.AutoCRLF;
import org.eclipse.jgit.util.FS;
/**
@ -88,7 +88,7 @@ public FileTreeIteratorWithTimeControl(Repository repo,
public FileTreeIteratorWithTimeControl(File f, FS fs,
TreeSet<Long> modTimes) {
super(f, fs, new WorkingTreeOptions(AutoCRLF.FALSE));
super(f, fs, new Config().get(WorkingTreeOptions.KEY));
this.modTimes = modTimes;
}

52
org.eclipse.jgit/src/org/eclipse/jgit/dircache/DirCacheCheckout.java
View File

@ -61,7 +61,6 @@
import org.eclipse.jgit.lib.ObjectId;
import org.eclipse.jgit.lib.ObjectLoader;
import org.eclipse.jgit.lib.Repository;
import org.eclipse.jgit.lib.StoredConfig;
import org.eclipse.jgit.treewalk.AbstractTreeIterator;
import org.eclipse.jgit.treewalk.CanonicalTreeParser;
import org.eclipse.jgit.treewalk.EmptyTreeIterator;
@ -184,9 +183,7 @@ public DirCacheCheckout(Repository repo, ObjectId headCommitTree, DirCache dc,
*/
public DirCacheCheckout(Repository repo, ObjectId headCommitTree,
DirCache dc, ObjectId mergeCommitTree) throws IOException {
this(repo, headCommitTree, dc, mergeCommitTree, new FileTreeIterator(
repo.getWorkTree(), repo.getFS(),
WorkingTreeOptions.createDefaultInstance()));
this(repo, headCommitTree, dc, mergeCommitTree, new FileTreeIterator(repo));
}
/**
@ -224,9 +221,7 @@ public DirCacheCheckout(Repository repo, DirCache dc,
*/
public DirCacheCheckout(Repository repo, DirCache dc,
ObjectId mergeCommitTree) throws IOException {
this(repo, null, dc, mergeCommitTree, new FileTreeIterator(
repo.getWorkTree(), repo.getFS(),
WorkingTreeOptions.createDefaultInstance()));
this(repo, null, dc, mergeCommitTree, new FileTreeIterator(repo));
}
/**
@ -311,7 +306,7 @@ void processEntry(CanonicalTreeParser m, DirCacheBuildIterator i,
if (m != null) {
if (i == null || f == null || !m.idEqual(i)
|| (i.getDirCacheEntry() != null && (f.isModified(
i.getDirCacheEntry(), true, config_filemode()) ||
i.getDirCacheEntry(), true) ||
i.getDirCacheEntry().getStage() != 0))) {
update(m.getEntryPathString(), m.getEntryObjectId(),
m.getEntryFileMode());
@ -391,7 +386,7 @@ public boolean checkout() throws IOException {
file.getParentFile().mkdirs();
file.createNewFile();
DirCacheEntry entry = dc.getEntry(path);
checkoutEntry(repo, file, entry, config_filemode());
checkoutEntry(repo, file, entry);
}
@ -575,7 +570,7 @@ void processEntry(AbstractTreeIterator h, AbstractTreeIterator m,
case 0xFFD: // 12 13 14
if (hId.equals(iId)) {
dce = i.getDirCacheEntry();
if (f == null || f.isModified(dce, true, config_filemode()))
if (f == null || f.isModified(dce, true))
conflict(name, i.getDirCacheEntry(), h, m);
else
remove(name);
@ -641,8 +636,7 @@ else if (m == null)
if (m == null || mId.equals(iId)) {
if (m==null && walk.isDirectoryFileConflict()) {
if (dce != null
&& (f == null || f.isModified(dce, true,
config_filemode())))
&& (f == null || f.isModified(dce, true)))
conflict(name, i.getDirCacheEntry(), h, m);
else
remove(name);
@ -664,7 +658,7 @@ else if (m == null)
*/
if (hId.equals(iId)) {
if (f == null || f.isModified(dce, true, config_filemode()))
if (f == null || f.isModified(dce, true))
conflict(name, i.getDirCacheEntry(), h, m);
else
remove(name);
@ -674,9 +668,7 @@ else if (m == null)
if (!hId.equals(mId) && !hId.equals(iId) && !mId.equals(iId))
conflict(name, i.getDirCacheEntry(), h, m);
else if (hId.equals(iId) && !mId.equals(iId)) {
if (dce != null
&& (f == null || f.isModified(dce, true,
config_filemode())))
if (dce != null && (f == null || f.isModified(dce, true)))
conflict(name, i.getDirCacheEntry(), h, m);
else
update(name, mId, m.getEntryFileMode());
@ -738,19 +730,6 @@ private void update(String path, ObjectId mId, FileMode mode) {
}
}
private Boolean filemode;
private boolean config_filemode() {
// TODO: temporary till we can actually set parameters. We need to be
// able to change this for testing.
if (filemode == null) {
StoredConfig config = repo.getConfig();
filemode = Boolean.valueOf(config.getBoolean("core", null,
"filemode", true));
}
return filemode.booleanValue();
}
/**
* If <code>true</code>, will scan first to see if it's possible to check
* out, otherwise throw {@link CheckoutConflictException}. If
@ -790,8 +769,7 @@ private void cleanUpConflicts() throws CheckoutConflictException {
private boolean isModified(String path) throws CorruptObjectException, IOException {
NameConflictTreeWalk tw = new NameConflictTreeWalk(repo);
tw.addTree(new DirCacheIterator(dc));
tw.addTree(new FileTreeIterator(repo.getWorkTree(), repo.getFS(),
WorkingTreeOptions.createDefaultInstance()));
tw.addTree(new FileTreeIterator(repo));
tw.setRecursive(true);
tw.setFilter(PathFilter.create(path));
DirCacheIterator dcIt;
@ -801,8 +779,7 @@ private boolean isModified(String path) throws CorruptObjectException, IOExcepti
wtIt = tw.getTree(1, WorkingTreeIterator.class);
if (dcIt == null || wtIt == null)
return true;
if (wtIt.isModified(dcIt.getDirCacheEntry(), true,
config_filemode())) {
if (wtIt.isModified(dcIt.getDirCacheEntry(), true)) {
return true;
}
}
@ -824,12 +801,10 @@ private boolean isModified(String path) throws CorruptObjectException, IOExcepti
* has to exist already
* @param entry
* the entry containing new mode and content
* @param config_filemode
* whether the mode bits should be handled at all.
* @throws IOException
*/
public static void checkoutEntry(final Repository repo, File f, DirCacheEntry entry,
boolean config_filemode) throws IOException {
public static void checkoutEntry(final Repository repo, File f,
DirCacheEntry entry) throws IOException {
ObjectLoader ol = repo.open(entry.getObjectId());
File parentDir = f.getParentFile();
File tmpFile = File.createTempFile("._" + f.getName(), null, parentDir);
@ -840,7 +815,8 @@ public static void checkoutEntry(final Repository repo, File f, DirCacheEntry en
channel.close();
}
FS fs = repo.getFS();
if (config_filemode && fs.supportsExecute()) {
WorkingTreeOptions opt = repo.getConfig().get(WorkingTreeOptions.KEY);
if (opt.isFileMode() && fs.supportsExecute()) {
if (FileMode.EXECUTABLE_FILE.equals(entry.getRawMode())) {
if (!fs.canExecute(tmpFile))
fs.setExecute(tmpFile, true);

20
org.eclipse.jgit/src/org/eclipse/jgit/lib/CoreConfig.java
View File

@ -80,16 +80,10 @@ public static enum AutoCRLF {
private final boolean logAllRefUpdates;
private final boolean fileMode;
private final AutoCRLF autoCRLF;
private CoreConfig(final Config rc) {
compression = rc.getInt("core", "compression", DEFAULT_COMPRESSION);
packIndexVersion = rc.getInt("pack", "indexversion", 2);
logAllRefUpdates = rc.getBoolean("core", "logallrefupdates", true);
fileMode = rc.getBoolean("core", "filemode", true);
autoCRLF = rc.getEnum("core", null, "autocrlf", AutoCRLF.FALSE);
}
/**
@ -113,18 +107,4 @@ public int getPackIndexVersion() {
public boolean isLogAllRefUpdates() {
return logAllRefUpdates;
}
/**
* @return whether to trust file modes
*/
public boolean isFileMode() {
return fileMode;
}
/**
* @return whether automatic CRLF conversion has been configured
*/
public AutoCRLF getAutoCRLF() {
return autoCRLF;
}
}

5
org.eclipse.jgit/src/org/eclipse/jgit/merge/ResolveMerger.java
View File

@ -244,7 +244,7 @@ private void checkout() throws NoWorkTreeException, IOException {
createDir(f.getParentFile());
DirCacheCheckout.checkoutEntry(db,
f,
entry.getValue(), true);
entry.getValue());
} else {
if (!f.delete())
failingPathes.put(entry.getKey(),
@ -446,8 +446,7 @@ private boolean processEntry(CanonicalTreeParser base,
// is not modified
if (work != null
&& (!nonTree(work.getEntryRawMode()) || work
.isModified(index.getDirCacheEntry(), true,
true))) {
.isModified(index.getDirCacheEntry(), true))) {
failingPathes.put(tw.getPathString(),
MergeFailureReason.DIRTY_WORKTREE);
return false;

4
org.eclipse.jgit/src/org/eclipse/jgit/treewalk/FileTreeIterator.java
View File

@ -84,8 +84,8 @@ public class FileTreeIterator extends WorkingTreeIterator {
* the repository whose working tree will be scanned.
*/
public FileTreeIterator(Repository repo) {
this(repo.getWorkTree(), repo.getFS(), WorkingTreeOptions
.createConfigurationInstance(repo.getConfig()));
this(repo.getWorkTree(), repo.getFS(),
repo.getConfig().get(WorkingTreeOptions.KEY));
initRootIterator(repo);
}

8
org.eclipse.jgit/src/org/eclipse/jgit/treewalk/WorkingTreeIterator.java
View File

@ -540,13 +540,9 @@ protected Entry current() {
* @param forceContentCheck
* True if the actual file content should be checked if
* modification time differs.
* @param checkFilemode
* whether the executable-bit in the filemode should be checked
* to detect modifications
* @return true if content is most likely different.
*/
public boolean isModified(DirCacheEntry entry, boolean forceContentCheck,
boolean checkFilemode) {
public boolean isModified(DirCacheEntry entry, boolean forceContentCheck) {
if (entry.isAssumeValid())
return false;
@ -563,7 +559,7 @@ public boolean isModified(DirCacheEntry entry, boolean forceContentCheck,
// Ignore the executable file bits if checkFilemode tells me to do so.
// Ignoring is done by setting the bits representing a EXECUTABLE_FILE
// to '0' in modeDiff
if (!checkFilemode)
if (!state.options.isFileMode())
modeDiff &= ~FileMode.EXECUTABLE_FILE.getBits();
if (modeDiff != 0)
// Report a modification if the modes still (after potentially

62
org.eclipse.jgit/src/org/eclipse/jgit/treewalk/WorkingTreeOptions.java
View File

@ -43,59 +43,33 @@
package org.eclipse.jgit.treewalk;
import org.eclipse.jgit.lib.Config;
import org.eclipse.jgit.lib.CoreConfig;
import org.eclipse.jgit.lib.Config.SectionParser;
import org.eclipse.jgit.lib.CoreConfig.AutoCRLF;
/**
* Contains options used by the WorkingTreeIterator.
*/
/** Options used by the {@link WorkingTreeIterator}. */
public class WorkingTreeOptions {
/** Key for {@link Config#get(SectionParser)}. */
public static final Config.SectionParser<WorkingTreeOptions> KEY = new SectionParser<WorkingTreeOptions>() {
public WorkingTreeOptions parse(final Config cfg) {
return new WorkingTreeOptions(cfg);
}
};
/**
* Creates default options which reflect the original configuration of Git
* on Unix systems.
*
* @return created working tree options
*/
public static WorkingTreeOptions createDefaultInstance() {
return new WorkingTreeOptions(AutoCRLF.FALSE);
}
private final boolean fileMode;
/**
* Creates options based on the specified repository configuration.
*
* @param config
* repository configuration to create options for
*
* @return created working tree options
*/
public static WorkingTreeOptions createConfigurationInstance(Config config) {
return new WorkingTreeOptions(config.get(CoreConfig.KEY).getAutoCRLF());
}
/**
* Indicates whether EOLs of text files should be converted to '\n' before
* calculating the blob ID.
**/
private final AutoCRLF autoCRLF;
/**
* Creates new options.
*
* @param autoCRLF
* indicates whether EOLs of text files should be converted to
* '\n' before calculating the blob ID.
*/
public WorkingTreeOptions(AutoCRLF autoCRLF) {
this.autoCRLF = autoCRLF;
private WorkingTreeOptions(final Config rc) {
fileMode = rc.getBoolean("core", "filemode", true);
autoCRLF = rc.getEnum("core", null, "autocrlf", AutoCRLF.FALSE);
}
/**
* Indicates whether EOLs of text files should be converted to '\n' before
* calculating the blob ID.
*
* @return true if EOLs should be canonicalized.
*/
/** @return true if the execute bit on working files should be trusted. */
public boolean isFileMode() {
return fileMode;
}
/** @return how automatic CRLF conversion has been configured. */
public AutoCRLF getAutoCRLF() {
return autoCRLF;
}