Skip to content

Commit

Permalink
[FS] Always copy file-attributes using Eclipse's Native(File)Handler
Browse files Browse the repository at this point in the history 
Files.copy seems to copy file-attributes differently than Eclipse
NativeHandler (for files) do on Mac. In order to stay consistent always
use the Eclipse way.

Fixes #1504
  • Loading branch information
@HannesWell
HannesWell committed Aug 16, 2024
1 parent c62d47a commit 0308ed0
Show file tree
Hide file tree
Showing 4 changed files with 53 additions and 26 deletions.
9 changes: 3 additions & 6 deletions ...ndles/org.eclipse.core.filesystem/src/org/eclipse/core/filesystem/provider/FileStore.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
import org.eclipse.core.internal.filesystem.FileCache;
import org.eclipse.core.internal.filesystem.Messages;
import org.eclipse.core.internal.filesystem.Policy;
import org.eclipse.core.internal.filesystem.local.LocalFile;
import org.eclipse.core.runtime.CoreException;
import org.eclipse.core.runtime.IPath;
import org.eclipse.core.runtime.IProgressMonitor;
Expand Down Expand Up @@ -129,7 +130,7 @@ protected void copyDirectory(IFileInfo sourceInfo, IFileStore destination, int o
// create directory
destination.mkdir(EFS.NONE, subMonitor.newChild(1));
// copy attributes
transferAttributes(sourceInfo, destination);
LocalFile.transferAttributes(sourceInfo, destination);

if (children == null)
return;
Expand Down Expand Up @@ -167,7 +168,7 @@ protected void copyFile(IFileInfo sourceInfo, IFileStore destination, int option
OutputStream out = destination.openOutputStream(EFS.NONE, subMonitor.newChild(1));) {
in.transferTo(out);
subMonitor.worked(93);
transferAttributes(sourceInfo, destination);
LocalFile.transferAttributes(sourceInfo, destination);
subMonitor.worked(5);
} catch (IOException e) {
Policy.error(EFS.ERROR_WRITE, NLS.bind(Messages.failedCopy, sourcePath), e);
Expand Down Expand Up @@ -429,8 +430,4 @@ public String toString() {
@Override
public abstract URI toURI();

private void transferAttributes(IFileInfo sourceInfo, IFileStore destination) throws CoreException {
int options = EFS.SET_ATTRIBUTES | EFS.SET_LAST_MODIFIED;
destination.putInfo(sourceInfo, options, null);
}
}
17 changes: 13 additions & 4 deletions ...org.eclipse.core.filesystem/src/org/eclipse/core/internal/filesystem/local/LocalFile.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -148,28 +148,37 @@ public void copy(IFileStore destFile, int options, IProgressMonitor monitor) thr
super.copy(destFile, options, monitor);
}

private static final CopyOption[] NO_OVERWRITE = {StandardCopyOption.COPY_ATTRIBUTES};
private static final CopyOption[] OVERWRITE_EXISTING = {StandardCopyOption.COPY_ATTRIBUTES, StandardCopyOption.REPLACE_EXISTING};
private static final int LARGE_FILE_SIZE_THRESHOLD = 1024 * 1024; // 1 MiB experimentally determined
private static final CopyOption[] NO_OVERWRITE = {};
private static final CopyOption[] OVERWRITE_EXISTING = {StandardCopyOption.REPLACE_EXISTING};
public static final int LARGE_FILE_SIZE_THRESHOLD = 1024 * 1024; // 1 MiB experimentally determined

@Override
protected void copyFile(IFileInfo sourceInfo, IFileStore destination, int options, IProgressMonitor monitor) throws CoreException {
if (sourceInfo.getLength() > LARGE_FILE_SIZE_THRESHOLD && destination instanceof LocalFile target) {
SubMonitor subMonitor = SubMonitor.convert(monitor, NLS.bind(Messages.copying, this), 100);
try {
boolean overwrite = (options & EFS.OVERWRITE) != 0;
Files.copy(this.file.toPath(), target.file.toPath(), overwrite ? OVERWRITE_EXISTING : NO_OVERWRITE);
subMonitor.worked(93);
transferAttributes(sourceInfo, destination);
subMonitor.worked(5);
} catch (FileAlreadyExistsException e) {
Policy.error(EFS.ERROR_EXISTS, NLS.bind(Messages.fileExists, target.filePath), e);
} catch (IOException e) {
Policy.error(EFS.ERROR_WRITE, NLS.bind(Messages.failedCopy, this.filePath, target.filePath), e);
} finally {
IProgressMonitor.done(monitor);
subMonitor.done();
}
} else {
super.copyFile(sourceInfo, destination, options, monitor);
}
}

public static final void transferAttributes(IFileInfo sourceInfo, IFileStore destination) throws CoreException {
int options = EFS.SET_ATTRIBUTES | EFS.SET_LAST_MODIFIED;
destination.putInfo(sourceInfo, options, null);
}

@Override
public void delete(int options, IProgressMonitor monitor) throws CoreException {
if (monitor == null)
Expand Down
2 changes: 2 additions & 0 deletions resources/tests/org.eclipse.core.tests.resources/META-INF/MANIFEST.MF
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,8 @@ Import-Package: org.assertj.core.api,
org.junit.jupiter.api.extension,
org.junit.jupiter.api.function,
org.junit.jupiter.api.io,
org.junit.jupiter.params,
org.junit.jupiter.params.provider,
org.junit.platform.suite.api,
org.mockito
Bundle-ActivationPolicy: lazy
Expand Down
51 changes: 35 additions & 16 deletions ...ipse.core.tests.resources/src/org/eclipse/core/tests/resources/session/TestBug323833.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,18 +16,24 @@

import static org.eclipse.core.resources.ResourcesPlugin.getWorkspace;
import static org.eclipse.core.tests.resources.ResourceTestPluginConstants.PI_RESOURCES_TESTS;
import static org.eclipse.core.tests.resources.ResourceTestUtil.createInFileSystem;
import static org.eclipse.core.tests.resources.ResourceTestUtil.createTestMonitor;
import static org.junit.jupiter.api.Assertions.assertTrue;
import static org.junit.jupiter.api.Assumptions.assumeTrue;

import java.io.BufferedOutputStream;
import java.io.File;
import java.io.IOException;
import java.io.OutputStream;
import java.nio.file.Path;
import java.util.Random;
import java.util.concurrent.atomic.AtomicInteger;
import org.eclipse.core.filesystem.EFS;
import org.eclipse.core.filesystem.IFileInfo;
import org.eclipse.core.filesystem.IFileStore;
import org.eclipse.core.internal.filesystem.FileCache;
import org.eclipse.core.internal.filesystem.local.LocalFile;
import org.eclipse.core.runtime.CoreException;
import org.eclipse.core.runtime.IPath;
import org.eclipse.core.runtime.Platform.OS;
import org.eclipse.core.tests.harness.session.CustomSessionWorkspace;
import org.eclipse.core.tests.harness.session.ExecuteInHost;
Expand All @@ -38,13 +44,16 @@
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.TestMethodOrder;
import org.junit.jupiter.api.extension.RegisterExtension;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.ValueSource;

/**
* Test for bug 323833
*/
@TestMethodOrder(MethodOrderer.OrderAnnotation.class)
public class TestBug323833 {
private static final String READONLY_FILE_NAME = "test";
private static final AtomicInteger FILE_SUFFIX = new AtomicInteger(0);

private static CustomSessionWorkspace sessionWorkspace = SessionTestExtension.createCustomWorkspace();

Expand All @@ -55,42 +64,52 @@ public class TestBug323833 {
@AfterAll
@ExecuteInHost
public static void restoreFileWriabilityForCleanup() throws CoreException, IOException {
sessionWorkspace.getWorkspaceDirectory().resolve(READONLY_FILE_NAME).toFile().setWritable(true, false);
Path workspaceDirectory = sessionWorkspace.getWorkspaceDirectory();
for (int i = 1; i <= FILE_SUFFIX.get(); i++) {
workspaceDirectory.resolve(READONLY_FILE_NAME + i).toFile().setWritable(true, false);
}
}

@Test
@ParameterizedTest(name = "File size {0} bytes")
@Order(1)
public void test1() throws Exception {
if (!OS.isMac()) {
return;
}
@ValueSource(ints = { 10, LocalFile.LARGE_FILE_SIZE_THRESHOLD + 10 })
void test1(int fileSize) throws Exception {
// assumeTrue(OS.isMac());

IFileStore fileStore = EFS.getLocalFileSystem().getStore(getWorkspace().getRoot().getLocation())
.getChild(READONLY_FILE_NAME);
createInFileSystem(fileStore);
IPath workspaceRootLocation = getWorkspace().getRoot().getLocation();
String filename = READONLY_FILE_NAME + FILE_SUFFIX.incrementAndGet();
IFileStore fileStore = EFS.getLocalFileSystem().getStore(workspaceRootLocation).getChild(filename);
createInFileSystem(fileStore, fileSize);

// set EFS.ATTRIBUTE_READ_ONLY which also sets EFS.IMMUTABLE on Mac
IFileInfo info = fileStore.fetchInfo();
info.setAttribute(EFS.ATTRIBUTE_READ_ONLY, true);
fileStore.putInfo(info, EFS.SET_ATTRIBUTES, createTestMonitor());

// create a cached file
File cachedFile = null;
cachedFile = fileStore.toLocalFile(EFS.CACHE, createTestMonitor());

File cachedFile = fileStore.toLocalFile(EFS.CACHE, createTestMonitor());
IFileInfo cachedFileInfo = new LocalFile(cachedFile).fetchInfo();

// check that the file in the cache has attributes set
assertTrue(cachedFileInfo.getAttribute(EFS.ATTRIBUTE_READ_ONLY));
assertTrue(cachedFileInfo.getAttribute(EFS.ATTRIBUTE_IMMUTABLE));
}

private static final Random RANDOM = new Random();

private static void createInFileSystem(IFileStore file, int fileSize) throws CoreException, IOException {
file.getParent().mkdir(EFS.NONE, null);
try (OutputStream output = new BufferedOutputStream(file.openOutputStream(EFS.NONE, null))) {
for (int size = 0; size < fileSize; size++) {
output.write(RANDOM.nextInt(Byte.SIZE));
}
}
}

@Test
@Order(2)
public void test2() throws CoreException {
if (!OS.isMac()) {
return;
}
assumeTrue(OS.isMac());

FileCache.getCache();
}
Expand Down

0 comments on commit 0308ed0

Please sign in to comment.