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.

Sign up for GitHub

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

Jump to bottom

Duplicate project names handling in SmartImport wizard is broken #1294

Open
kwin opened this issue Nov 8, 2023 · 9 comments · May be fixed by #1297
Open

Duplicate project names handling in SmartImport wizard is broken #1294

kwin opened this issue Nov 8, 2023 · 9 comments · May be fixed by #1297
Milestone
4.31 M1

Comments

@kwin
Copy link

kwin commented Nov 8, 2023 •
edited

According to https://github.com/eclipse-platform/eclipse.platform.ui/blob/f4f59cebf2d6e22201284f6722eeefa4d1929115/bundles/org.eclipse.ui.ide/src/org/eclipse/ui/internal/wizards/datatransfer/SmartImportJob.java#L608C1-L608C1 there should be a graceful project name handling in case of duplicates. Unfortunately the wizard using https://github.com/eclipse-platform/eclipse.platform.ui/blob/master/bundles/org.eclipse.ui.ide/src/org/eclipse/ui/internal/wizards/datatransfer/SmartImportRootWizardPage.java already blocks triggering the SmartImportJob due to the conflicting project names (although coming from another root folder):

Screenshot 2023-11-08 at 16 35 20

The validation probably needs to be relaxed as at least for Maven projects duplicate folder names are quite common (in separate repositories).
@kwin
Copy link
Author

kwin commented Nov 8, 2023

Instead of checking only the project name for clashes one should really compare the project location (i.e. full path). @mickaelistria Since you once fixed 4d20901 how was this ever supposed to work?

@kwin
Copy link
Author

kwin commented Nov 8, 2023

The issues seems to be the additional check in https://github.com/eclipse-platform/eclipse.platform.ui/blob/f4f59cebf2d6e22201284f6722eeefa4d1929115/bundles/org.eclipse.ui.ide/src/org/eclipse/ui/internal/wizards/datatransfer/SmartImportRootWizardPage.java#L225C15-L225C36 added with 45b2826.

@vogella
Copy link
Contributor

vogella commented Nov 8, 2023

@kwin sounds like you already analysed that in detail. Please provide a PR to improve this. We are currently in feature freeze for 2023-12 but once master opens for 2024-03 (in a few weeks) a valid patch could be applied.

@mickaelistria
Copy link
Contributor

If there are already some .project in the folders; then we cannot decide of another project name at import and wizard sticks to the project name defined in .project. Is some project with same name already exists, then the import cannot work automatically.
If it's your care here, then the issue is more about the message in Import As which should instead check location and say "A project with same name already exists in workspace" if the location is different.

@kwin
Copy link
Author

kwin commented Nov 8, 2023 •
edited

@mickaelistria The check is really only about the name, there is no .project in those folders anywhere:

eclipse.platform.ui/bundles/org.eclipse.ui.ide/src/org/eclipse/ui/internal/wizards/datatransfer/SmartImportRootWizardPage.java

Line 721 in f4f59ce

protected boolean isExistingProjectName(File element) {
. Can you remember why you committed 45b2826 in the first place.

I posted the wrong screenshot above initially, it is now fixed. It indeed says "Project with same name already imported" which is true but should not prevent importing that.

@mickaelistria
Copy link
Contributor

Can you remember why you committed 45b2826 in the first place.

No sorry.

The check is really only about the name, there is no .project in those folders anywhere:

The expectation is that if a project with same name as the folder already exists and there is no .project to force the name, then the importer attempts to set then name to parentFolder_currentFolder , recursively, until the name is free.
It looks like you've demonstrated the implementation here doesn't match the expectation and have a clue how to fix it. I would happily review a PR ;)

@laeubi
Copy link
Contributor

laeubi commented Nov 9, 2023

We are currently in feature freeze for 2023-12

Is this not more a bug than a feature?

kwin added a commit to kwin/eclipse.platform.ui that referenced this issue Nov 9, 2023
@kwin
Allow duplicate folder names in SmartImport wizard eclipse-platform#1294
9d61855 


Only block if new project's description's name is set and already taken.
Otherwise the smart import will come up with a new project name
prepending the parent folder name followed by "_" (recursively).

Fixes eclipse-platform#1294
@kwin kwin linked a pull request Nov 9, 2023 that will close this issue
Open
@kwin
Copy link
Author

kwin commented Nov 21, 2023

Any chance this can be included as bug fix for 2023-12?

@mickaelistria
Copy link
Contributor

Any chance this can be included as bug fix for 2023-12?

None, sorry.

@mickaelistria mickaelistria added this to the 4.31 M1 milestone Nov 21, 2023
mickaelistria pushed a commit to kwin/eclipse.platform.ui that referenced this issue Dec 4, 2023
@kwin @mickaelistria
Allow duplicate folder names in SmartImport wizard eclipse-platform#1294
efd01bf 


Only block if new project's description's name is set and already taken.
Otherwise the smart import will come up with a new project name
prepending the parent folder name followed by "_" (recursively).

Fixes eclipse-platform#1294
mickaelistria pushed a commit to kwin/eclipse.platform.ui that referenced this issue Dec 6, 2023
@kwin @mickaelistria
Allow duplicate folder names in SmartImport wizard eclipse-platform#1294
c14dd99 


Only block if new project's description's name is set and already taken.
Otherwise the smart import will come up with a new project name
prepending the parent folder name followed by "_" (recursively).

Fixes eclipse-platform#1294
mickaelistria pushed a commit to kwin/eclipse.platform.ui that referenced this issue Dec 10, 2023
@kwin @mickaelistria
Allow duplicate folder names in SmartImport wizard eclipse-platform#1294
4760c84 


Only block if new project's description's name is set and already taken.
Otherwise the smart import will come up with a new project name
prepending the parent folder name followed by "_" (recursively).

Fixes eclipse-platform#1294
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
4.31 M1
Development

Successfully merging a pull request may close this issue.

Allow duplicate folder names in SmartImport wizard #1294 kwin/eclipse.platform.ui
4 participants
@vogella @kwin @mickaelistria @laeubi