Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Avoid duplicated IFileInfo fetches when creating resources #1598

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*******************************************************************************
* Copyright (c) 2000, 2017 IBM Corporation and others.
* Copyright (c) 2000, 2024 IBM Corporation and others.
*
* This program and the accompanying materials
* are made available under the terms of the Eclipse Public License 2.0
Expand Down Expand Up @@ -557,15 +557,6 @@ public IHistoryStore getHistoryStore() {
return _historyStore;
}

/**
* Returns the real name of the resource on disk. Returns null if no local
* file exists by that name. This is useful when dealing with
* case insensitive file systems.
*/
public String getLocalName(IFileStore target) {
return target.fetchInfo().getName();
}

protected IPath getProjectDefaultLocation(IProject project) {
return workspace.getRoot().getLocation().append(project.getFullPath());
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*******************************************************************************
* Copyright (c) 2000, 2015 IBM Corporation and others.
* Copyright (c) 2000, 2024 IBM Corporation and others.
*
* This program and the accompanying materials
* are made available under the terms of the Eclipse Public License 2.0
Expand All @@ -21,6 +21,7 @@
import java.io.IOException;
import java.io.InputStream;
import java.io.Reader;
import java.nio.file.Path;
import java.util.ArrayList;
import java.util.List;
import java.util.Map.Entry;
Expand Down Expand Up @@ -221,25 +222,24 @@ void checkCreatable() throws CoreException {

IFileInfo create(int updateFlags, IProgressMonitor subMonitor, IFileStore store)
throws CoreException, ResourceException {
String message;
IFileInfo localInfo;
if (BitMask.isSet(updateFlags, IResource.FORCE)) {
// Assume the file does not exist - otherwise implementation fails later
// Assume the file does not exist - otherwise implementation fails later
// during actual write
localInfo = new FileInfo(getName()); // with exists==false
} else {
localInfo=store.fetchInfo();
localInfo = store.fetchInfo();
if (localInfo.exists()) {
// return an appropriate error message for case variant collisions
if (!Workspace.caseSensitive) {
String name = getLocalManager().getLocalName(store);
String name = localInfo.getName();
if (name != null && !localInfo.getName().equals(name)) {
message = NLS.bind(Messages.resources_existsLocalDifferentCase,
IPath.fromOSString(store.toString()).removeLastSegments(1).append(name).toOSString());
String message = NLS.bind(Messages.resources_existsLocalDifferentCase,
Path.of(store.toString()).resolveSibling(name));
throw new ResourceException(IResourceStatus.CASE_VARIANT_EXISTS, getFullPath(), message, null);
}
}
message = NLS.bind(Messages.resources_fileExists, store.toString());
String message = NLS.bind(Messages.resources_fileExists, store.toString());
throw new ResourceException(IResourceStatus.FAILED_WRITE_LOCAL, getFullPath(), message, null);
}
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*******************************************************************************
* Copyright (c) 2000, 2015 IBM Corporation and others.
* Copyright (c) 2000, 2024 IBM Corporation and others.
*
* This program and the accompanying materials
* are made available under the terms of the Eclipse Public License 2.0
Expand All @@ -15,6 +15,7 @@
package org.eclipse.core.internal.resources;

import java.net.URI;
import java.nio.file.Path;
import org.eclipse.core.filesystem.IFileInfo;
import org.eclipse.core.filesystem.IFileStore;
import org.eclipse.core.internal.utils.Messages;
Expand Down Expand Up @@ -47,9 +48,10 @@ protected void assertCreateRequirements(IFileStore store, IFileInfo localInfo, i
if (!force && localInfo.exists()) {
//return an appropriate error message for case variant collisions
if (!Workspace.caseSensitive) {
String name = getLocalManager().getLocalName(store);
String name = localInfo.getName();
if (name != null && !store.getName().equals(name)) {
String msg = NLS.bind(Messages.resources_existsLocalDifferentCase, IPath.fromOSString(store.toString()).removeLastSegments(1).append(name).toOSString());
String msg = NLS.bind(Messages.resources_existsLocalDifferentCase,
Path.of(store.toString()).resolveSibling(name));
throw new ResourceException(IResourceStatus.CASE_VARIANT_EXISTS, getFullPath(), msg, null);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CASE_VARIANT_EXISTS makes only sense when name is tested against the actual name in the filesystem. This codepath is only taken if localInfo.exists(), so never in the performance critical case where where the directory is clean.

}
}
Expand Down Expand Up @@ -101,14 +103,14 @@ public void create(int updateFlags, boolean local, IProgressMonitor monitor) thr
assertCreateRequirements(store, localInfo, updateFlags);
workspace.beginOperation(true);
if (force && !Workspace.caseSensitive && localInfo.exists()) {
String name = getLocalManager().getLocalName(store);
String name = localInfo.getName();
if (name == null || localInfo.getName().equals(name)) {
delete(true, null);
} else {
// The file system is not case sensitive and a case variant exists at this
// location
String msg = NLS.bind(Messages.resources_existsLocalDifferentCase,
IPath.fromOSString(store.toString()).removeLastSegments(1).append(name).toOSString());
Path.of(store.toString()).resolveSibling(name));
throw new ResourceException(IResourceStatus.CASE_VARIANT_EXISTS, getFullPath(), msg, null);
}
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*******************************************************************************
* Copyright (c) 2000, 2022 IBM Corporation and others.
* Copyright (c) 2000, 2024 IBM Corporation and others.
*
* This program and the accompanying materials
* are made available under the terms of the Eclipse Public License 2.0
Expand All @@ -21,6 +21,7 @@
package org.eclipse.core.internal.resources;

import java.net.URI;
import java.nio.file.Path;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collection;
Expand Down Expand Up @@ -101,9 +102,10 @@ protected void assertCreateRequirements(IProjectDescription description) throws
IFileStore store = getStore();
IFileInfo localInfo = store.fetchInfo();
if (localInfo.exists()) {
String name = getLocalManager().getLocalName(store);
String name = localInfo.getName();
if (name != null && !store.getName().equals(name)) {
String msg = NLS.bind(Messages.resources_existsLocalDifferentCase, IPath.fromOSString(store.toString()).removeLastSegments(1).append(name).toOSString());
String msg = NLS.bind(Messages.resources_existsLocalDifferentCase,
Path.of(store.toString()).resolveSibling(name));
throw new ResourceException(IResourceStatus.CASE_VARIANT_EXISTS, getFullPath(), msg, null);
}
}
Expand Down
Loading