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

[SR-10805] The root property in PackageModel.Sources struct should use the directory name as on disk #4699

Closed
ankitspd opened this issue May 30, 2019 · 9 comments
Labels
bug good first issue Good for newcomers Edit

Comments

@ankitspd
Copy link
Member

Previous ID SR-10805
Radar rdar://problem/49648458
Original Reporter @aciidb0mb3r
Type Bug
Additional Detail from JIRA
Votes 0
Component/s Package Manager
Labels Bug, StarterBug
Assignee Nevwild (JIRA)
Priority Medium

md5: f50a6a909df06a42211f31452a8f37d6

Issue Description:

The root property in PackageModel.Sources struct currently gets the target directory's basename from the target name in the package manifest. This is troublesome on case-insensitive filesystems because we're not using the exact path on disk.

@swift-ci
Copy link
Contributor

swift-ci commented Jun 8, 2019

Comment by Andrew Benson (JIRA)

Just trying to clarify here...

@aciidb0mb3r – you are saying the issue is with varying case for case-sensitive filesystems may have a target folder name that's different from the casing in the Package.swift – is that correct?

In the case of /some/path/to/project/TargetName, you would want to use "TargetName" even if the manifest shows "targetname"?

What about TARgetNAme? Or if there are several matching entries, such as targetname, TargetName, TARGETNAME, and targetNAME?

What would be the proper behavior in this case?

Are you sure matching the name case insensitively for case-sensitive filesystems is really the right behavior at all?

@ankitspd
Copy link
Member Author

Ideally, we should have never allowed this but existing packages will break if we change the behavior now. We could, however, start emitting a warning in these cases but still accept the right path on disk. I don't think you can have multiple directories with the same name but different casing on a case-insensitive file system.

@swift-ci
Copy link
Contributor

Comment by Andrew Benson (JIRA)

On a case IN-sensitive filesystem, you are right, that you cannot have multiple directories with different cases. However, for case-SENSITIVE filesystems, you can definitely have that – directories named "Hello", "hello", and "heLLo" can all coexist, for example.

@ankitspd
Copy link
Member Author

Right, this is only a problem for case-insensitive file systems. For case-sensitive filesystem, we'll use whatever is in the manifest and fail if it doesn't exactly match (this should already be the case).

@swift-ci
Copy link
Contributor

Comment by Andrew Benson (JIRA)

Okay, so what problem is this trying to solve?

In a case-insensitive filesystem if we try to access “foo” but instead get “Foo”, doesn’t that already work as expected?

Or is the issue that we explicitly check if “foo” exists at some point, rather than just trying to access it, and this are told NO, since the filename on disk is “Foo”?

@ankitspd
Copy link
Member Author

> Or is the issue that we explicitly check if “foo” exists at some point, rather than just trying to access it, and this are told NO, since the filename on disk is “Foo”?

Right. Some tools like the indexer rely on getting the right casing. I am now wondering if it's even possible to do something about this without knowing if the filesystem is case-insensitive or not. I'll need to think more about possible solutions here.

@ankitspd
Copy link
Member Author

ankitspd commented Jul 9, 2019

master: #2216

5.1: #2217

@belkadan
Copy link

This is completed, correct?

@swift-ci swift-ci transferred this issue from apple/swift-issues Apr 25, 2022
@shahmishal shahmishal transferred this issue from apple/swift May 4, 2022
@tomerd tomerd added good first issue Good for newcomers Edit and removed Package Manager labels May 14, 2022
@sjavora
Copy link
Contributor

sjavora commented Jan 30, 2024

@tomerd looks like this was already implemented per this comment: #4699 (comment)

@tomerd tomerd closed this as completed Jan 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug good first issue Good for newcomers Edit
Projects
None yet
Development

No branches or pull requests

5 participants