Skip to content

natalie: fix build on Xcode 8.1 (Swift 3.0.1) #6406

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

Closed
wants to merge 1 commit into from

Conversation

zmwangx
Copy link
Contributor

@zmwangx zmwangx commented Oct 30, 2016

  • Have you followed the guidelines for contributing?
  • Have you checked that there aren't other open pull requests for the same formula update/change?
  • Have you built your formula locally with brew install --build-from-source <formula>, where <formula> is the name of the formula you're submitting?
  • Does your build pass brew audit --strict <formula> (after doing brew install <formula>)?

In Swift 3.0.1 Swift Pacakge Manager now interprets CC as either an
absolute path or a path relative to pwd (see swiftlang/swift-package-manager@be54a9f line 47-50).

Our CC is usually just clang, which is interpreted as buildpath/clang and leads to an invalid toolchain error. Sample logs: https://gist.github.com/c8541f621bc2cc1028638391c7b50269.

Kind of funny that I discovered the bug while setting out to fix natalie on Sierra (for #5488), not knowing that it was already updated, just not ticked off.

@zmwangx
Copy link
Contributor Author

zmwangx commented Oct 30, 2016

By the way, I grepped the code base and only found one occurrence of system "swift", "build", but other swift-based formula may be broken too.

@ilovezfs
Copy link
Contributor

ilovezfs commented Nov 6, 2016

This is probably fine, but I'm wondering whether we should delete CC, or just turn it into the full path since this will now ignore a user's --cc=, right?

@zmwangx
Copy link
Contributor Author

zmwangx commented Nov 6, 2016

I doubt you can compile a Swift project with any flavor of GCC. What else is there other than clang which is the default?

@zmwangx
Copy link
Contributor Author

zmwangx commented Nov 6, 2016

I don't really oppose the idea of turning it into a full path.

@ilovezfs
Copy link
Contributor

ilovezfs commented Nov 6, 2016

I guess maybe if someone wants to use the llvm clang for instance?

@zmwangx
Copy link
Contributor Author

zmwangx commented Nov 6, 2016

I never used --cc myself and I didn't check the code, so I'm not sure, but the wording in the manpage makes it seem like you can only pass a name, not a path.

              If  --cc=compiler  is passed, attempt to compile using compiler. compiler should be the name
              of the compiler's executable, for instance gcc-4.2 for Apple's GCC 4.2,  or  gcc-4.9  for  a
              Homebrew-provided GCC 4.9.

Maybe it's just confusingly worded.

@ilovezfs
Copy link
Contributor

ilovezfs commented Nov 6, 2016

Yes, remember we were talking about coming up with some special name for the llvm clang? Presumably we'd want to honor that selection ...

@ilovezfs
Copy link
Contributor

ilovezfs commented Nov 6, 2016

I have to say though if this affects all swift projects now it seems like it belongs in superenv or something

@zmwangx
Copy link
Contributor Author

zmwangx commented Nov 6, 2016

If affects swift build, but not necessarily other parts of swift. Note the problematic code is in swift-package-manager. I don't know enough about the structure of the Swift project to make further assessments (except through field testing, which I'll try a little bit later).

@zmwangx
Copy link
Contributor Author

zmwangx commented Nov 6, 2016

except through field testing, which I'll try a little bit later

Oh, actually I was building swiftgen (with the Swift 2.3 patch) just now, and it wasn't affected. So definitely not all Swift projects are broken. But a case can still be made about solving this in superenv.

@zmwangx
Copy link
Contributor Author

zmwangx commented Nov 6, 2016

By the way, I tried

ENV["CC"] = `which #{ENV["CC"]}`

just now, which sets CC to /usr/local/Homebrew/Library/Homebrew/shims/super/clang. Doesn't really work, unfortunately.

@ilovezfs
Copy link
Contributor

ilovezfs commented Nov 6, 2016

Isn't it ENV.cc?

@zmwangx
Copy link
Contributor Author

zmwangx commented Nov 6, 2016

Isn't it ENV.cc?

Okay sure, I can't tell the difference between ENV.cc and ENV["CC"] except when assigning.

Anyway, ENV.cc is also a name that needs to be resolved. And when it resolves to a symlink it doesn't work, due to the fixme:

        // Check that it's valid in the file system.
        // FIXME: We should also check that it resolves to an executable file
        //        (it could be a symlink to such as file).
        guard localFileSystem.exists(swiftCompiler) else {
            throw Error.invalidToolchain(problem: "could not find `swiftc` at expected path \(swiftCompiler.asString)")
        }

Of course we could further resolve symlinks...

@ilovezfs
Copy link
Contributor

ilovezfs commented Nov 6, 2016

And why is using the shim as the full path wrong?

@zmwangx
Copy link
Contributor Author

zmwangx commented Nov 6, 2016

Because I forgot to chomp, and was misled by a sloppy reading of the FIXME.

ENV["CC"] = `which #{ENV.cc}`.chomp

is fine.

@zmwangx zmwangx force-pushed the natalie-fix-build-xcode-8.1 branch from 3a9b327 to b0e6860 Compare November 6, 2016 17:52
@ilovezfs
Copy link
Contributor

ilovezfs commented Nov 6, 2016

cool. I think you can use the which() ruby command.

In Swift 3.0.1 Swift Pacakge Manager now interprets CC as either an
absolute path or a path relative to pwd (see
swiftlang/swift-package-manager@be54a9f line
47-50).

Our CC is usually just 'clang', which is interpreted as buildpath/clang
and leads to an invalid toolchain error. Sample logs:
https://gist.github.com/c8541f621bc2cc1028638391c7b50269.
@zmwangx zmwangx force-pushed the natalie-fix-build-xcode-8.1 branch from b0e6860 to 9b1a01b Compare November 6, 2016 17:57
@zmwangx
Copy link
Contributor Author

zmwangx commented Nov 6, 2016

Whoo handy.

@ilovezfs
Copy link
Contributor

ilovezfs commented Nov 7, 2016

@BrewTestBot test this please

@ilovezfs ilovezfs closed this in 99ff7ac Nov 7, 2016
@zmwangx zmwangx deleted the natalie-fix-build-xcode-8.1 branch November 8, 2016 12:59
@Homebrew Homebrew locked and limited conversation to collaborators May 4, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants