swiftlang / swift

The Swift Programming Language
https://swift.org
Apache License 2.0
67.41k stars 10.34k forks source link

[SR-543] error: filename "Foo.swift" used twice #43160

Open drewcrawford opened 8 years ago

drewcrawford commented 8 years ago
Previous ID SR-543
Radar rdar://problem/34643301
Original Reporter @drewcrawford
Type Bug

Attachment: Download

Environment swift-2.2-SNAPSHOT-2016-01-11-a OSX 10.11.2
Additional Detail from JIRA | | | |------------------|-----------------| |Votes | 6 | |Component/s | Compiler | |Labels | Bug | |Assignee | None | |Priority | Medium | md5: 51e168cfb93f6b87ab9180d9e66125ad

is duplicated by:

Issue Description:

swift-llbuild has some kind of issue with compiling two sources with the same filename (even with different paths).

Reproduction steps:

1. Open attached project

  1. swift build

Or directly with llbuild via

1. Open attached project

  1. swift-build-tool -f .build/debug/MultiFilesTest.o/llbuild.yaml

Expected results: compiles
Actual results:

<unknown>:0: error: filename "Foo.swift" used twice: '/Users/drew/Code/MultiFilesTest/MyTarget/Foo.swift' and '/Users/drew/Code/MultiFilesTest/MyTarget/EvenMore/Foo.swift'
<unknown>:0: note: filenames are used to distinguish private declarations with the same name
<unknown>:0: error: build had 1 command failures
drewcrawford commented 8 years ago

cc: @ddunbar

mxcl commented 8 years ago

This is in fact a Swift compiler issue. Two files in the same module cannot have the same filename.

mxcl commented 8 years ago

Ideally we would be able to report which part of the stack is erroring here,

belkadan commented 8 years ago

And Swift uses file base names to distinguish private members, so we weren't planning to fix this.

(Why not use the whole path? Because then builds aren't reproducible. But packages have a meaningful path prefix, so maybe we could add that as an option.)

mxcl commented 8 years ago

Would be happy to add the code to PM for this. Alternatively I can improve the error message since I can detect this situation.

swift-ci commented 8 years ago

Comment by Srđan Rašić (JIRA)

It's very unfortunate not to be able to use same filenames with different paths. Imagine a very common scenario in iOS development where codebase is grouped by screens. At the moment we are forced to do something like

MasterViewController.swift
MasterViewModel.swift
MasterWireframe.swift

DetailViewController.swift
DetailViewModel.swift
DetailWireframe.swift

instead of much cleaner

Master/
  ViewController.swift
  ViewModel.swift
  Wireframe.swift

Detail/
  ViewController.swift
  ViewModel.swift
  Wireframe.swift

🙁

That, along with the lack of explicit namespaces often makes code full of unnecessary redundancies. When you add generics, readability of Swift code easily becomes much worse than that of ObjC. Support for more compact type names would help and first step in that direction would be fixing this bug because that would allow us to define classes, like the ones from above example, in extensions which implicitly provide namespaces.

I'm sure that very much people would like to see some improvement here.

mxcl commented 8 years ago

Master and Detail could be separate modules, personally I'd recommend this, and it works aounrd this issue.

Not that I'm saying we should not improve this situation… but there's an immediate solution that has other benefits.

swift-ci commented 8 years ago

Comment by sunny (JIRA)

I think it's quite silly to suggest we should create a framework for each of our MVC combinations. Given that Swift has no support for namespaces, there needs to be a cheap way to repeat filenames or class names. The current situation in Xcode is a lot of filenames like "MasterVie....oller.swift", especially on my 13" macbook!

swift-ci commented 7 years ago

Comment by Tyler Cloutier (JIRA)

Is there any resolution to the issue? A plan of action or no?

swift-ci commented 7 years ago

Comment by Sergey Suslov (JIRA)

+1, Any plans?

belkadan commented 7 years ago

These complaints are valid but that doesn't change the fact that we're actually using the uniqueness of filenames for something. Worse, those names may be encoded (in hashed form) into data that used Foundation's NSKeyedArchiver. We now have a warning for these cases, but we still don't want to risk a loss of user data.

Unless someone has a proposal that can address this concern, it's unlikely the restriction will be lifted.

drewcrawford commented 7 years ago

The obvious proposal is to hash the full path instead of the filename. But it is not 100% clear to me what the requirements are for NSKeyedArchiver and so what zones of the solution space are sensible

belkadan commented 7 years ago

As stated above, it's important to have reproducible builds no matter what your base directory is, so the full path is not a good solution. (Yes, there are other things keeping us from this in the default configuration of both Xcode and SwiftPM, but it's still something we're not going to give up by picking a design where it's not possible.)

The NSKeyedArchiver requirement is that private or fileprivate classes that conform to NSCoding may have written their name into an archive. The implementation of keyed archiving relies on looking up classes by name; in Swift that means the mangled name unless the class is top-level, non-generic, and non-private-or-fileprivate. There are workarounds, but unless someone thinks to test their app with an older archive they might not catch the bug at all.

(Strictly speaking, the implementation of NSKeyedArchiver on Linux never allowed private or fileprivate classes, so we could come up with an alternate solution on non-Apple platforms. But I'm not sure this is a good place to introduce platform-dependent restrictions.)

drewcrawford commented 7 years ago

Did we stabilize mangling? I was under the impression that slipped along with ABI stability, but I guess we are using it in userland file formats now.

It is pretty strange behavior for renaming a sourcefile with no code changes to break archive compatibility. Like you say, it is unlikely anyone would catch that in QA. That seems like a sufficient motivation to change the behavior independently of the issue in this bug.

Reproducible builds are a noble goal. Could we calculate the "smallest unique path" for this? In the typical case this removes the machine-specific part of the path, e.g.

\~/Documents/Code/MyProject/A/Foo.swift -> A/Foo.swift

\~/Documents/Code/MyProject/B/Foo.swift -> B/Foo.swift

There are some rare cases where swift source files are generated into tmp directories as part of a build, but I think those could be worked around as part of a reproducible builds effort

swift-ci commented 7 years ago

Comment by Sergey Suslov (JIRA)

Maybe you can add packages names like in Java? It's real necessary. For example we have structure like this:

In this case I need to create filename like ControllerCellButton.swift/ AnotherControllerCellButton.swift (and classnames same). It's real hell.

swift-ci commented 6 years ago

Comment by Grigory Entin (JIRA)

The NSKeyedArchiver requirement is that private or fileprivate classes that conform to NSCoding may have written their name into an archive. The implementation of keyed archiving relies on looking up classes by name; in Swift that means the mangled name unless the class is top-level, non-generic, and non-private-or-fileprivate. There are workarounds, but unless someone thinks to test their app with an older archive they might not catch the bug at all.

Does that mean that if I rename a file that happens to have such private classes, the (archived) data for the corresponding classes will become unreadable? I kind of understand rationale, but inability to rename a source file in the face of losing compatibility with already generated data sounds a bit frightening. I would prefer to make it compile time error/force @objc(name) requirement for the corresponding classes (to enforce explicit class names for the archiver), rather than silently ignore the issue.

Sorry, missed the comment from @drewcrawford who already mentioned the above. Overall, it sounds strange to me that filenames affecting build reproducibility is ok, but (base-relative) path names affecting build reproducibility is not ok. I really don't see much difference.

Btw, another solution for uniqueness would be support for some kind of (source code) @directive that would just allow explicitly stating the value that is currently derived from "filename" (something like #line in C preprocessor).

swift-ci commented 6 years ago

Comment by Grigory Entin (JIRA)

What if we allowed files with the same name as long as there's no conflicts vs private variables? I mean, make it "link time error"? Or, at least, don't make it error if there's just one (or no) files with private variables?

I'm using similarly-named files just for (module-scoped) type definitions (and no variables at all) (and not going to use them for anything else). I do understand that splitting out real modules could solve the problem, but I'd prefer not to be forced to do that.

Btw, as for modules, I wonder would that be still a problem if they're built as static frameworks?