[Un-revert][Glibc] Configure modulemap for target, not host #1704

modocache · 2016-03-16T19:04:21Z

What's in this pull request?

This reverts commit f2154ee, which reverted 04e1cd5. The original commit needed to be reverted because of an issue in which install targets were added to OS X builds that did not target Linux. This addresses that issue by guarding all the Linux-only CMake logic with a check for the SDK being built.

/cc @gribozavr @jrose-apple

Resolved bug number: None

Before merging this pull request to apple/swift repository:

Test pull request on Swift continuous integration.

Triggering Swift CI

The swift-ci is triggered by writing a comment on this PR addressed to the GitHub user @swift-ci. Different tests will run depending on the specific comment that you use. The currently available comments are:

Smoke Testing

Platform	Comment
All supported platforms	@swift-ci Please smoke test
OS X platform	@swift-ci Please smoke test OS X platform
Linux platform	@swift-ci Please smoke test Linux platform

Validation Testing

Platform	Comment
All supported platforms	@swift-ci Please test
OS X platform	@swift-ci Please test OS X platform
Linux platform	@swift-ci Please test Linux platform

Note: Only members of the Apple organization can trigger swift-ci.

This reverts commit f2154ee, which reverted 04e1cd5. The original commit needed to be reverted because of an issue in which install targets were added to OS X builds that did not target Linux. This addresses that issue by guarding all the Linux-only CMake logic with a check for the SDK being built.

modocache · 2016-03-16T19:05:04Z

stdlib/public/Glibc/CMakeLists.txt

+foreach(SDK ${SWIFT_SDKS})
+  foreach(arch ${SWIFT_SDK_${SDK}_ARCHITECTURES})
+    # Don't generate Glibc module maps for Darwin targets.
+    if("${SDK}" STREQUAL "LINUX" OR "${SDK}" STREQUAL "FREEBSD")


This if() statement is the only difference between this and #1679. ☝️

modocache · 2016-03-16T19:54:58Z

@swift-ci wasn't able to catch the failure in #1679 because it doesn't trigger a build that uses --install-swift=1. I just ran utils/build-toolchain locally (which uses that parameter) with this patch, and everything worked! 💯

gribozavr · 2016-03-16T20:07:10Z

@swift-ci Please test

modocache · 2016-03-16T23:19:08Z

@gribozavr Looks like the tests passed! Is this good to merge?

gribozavr · 2016-03-16T23:23:17Z

Yes, LGTM, thanks @modocache!

[Un-revert][Glibc] Configure modulemap for target, not host

modocache · 2016-03-16T23:25:53Z

Thanks! 👋

najacque · 2016-03-22T20:25:52Z

This change looks like it might be causing the package job to fail:

/tmp/swift-package-tests/test-multi-compile-glibc/Output/swift-multi-compile-glibc.py.tmp.dir/hello.swift:1:8: error: missing required module 'SwiftGlibc'
import Glibc

Could you please take a look:

https://ci.swift.org/view/Packages/job/oss-swift-package-linux-ubuntu-14_04/800/console
https://ci.swift.org/view/Packages/job/oss-swift-package-linux-ubuntu-15_10/792/console

modocache · 2016-03-22T20:55:06Z

Thanks for the heads up, @najacque. The failing jobs are using the buildbot_linux_1404 and buildbot_linux_1510 build script presets, which include the --test-installable-package flag. That flag isn't included in utils/build-toolchain, which I used locally to verify this pull request was OK. That explains why I missed this problem.

I'm looking into why the tests are failing now. The changes in #1757 seem relevant to this discussion as well...

modocache · 2016-03-22T21:44:43Z

@najacque I just finished a clean build of the buildbot_linux_1510 preset on an Ubuntu 15.10 machine, using the tip of master from about an hour ago (b094e77). Everything passed, including the package tests:

+ cd /home/modocache/GitHub/apple/swift-integration-tests
+ python /home/modocache/GitHub/apple/llvm/utils/lit/lit.py . -sv --param package-path=/home/modocache/GitHub/apple/build/buildbot_linux/none-swift_package_sandbox --param filecheck=/home/modocache/GitHub/apple/build/buildbot_linux/llvm-linux-x86_64/bin/FileCheck
lit.py: lit.cfg:101: note: 'pexpect' module unavailable, skipping related tests
lit.py: lit.cfg:117: note: testing package: '/home/modocache/GitHub/apple/build/buildbot_linux/none-swift_package_sandbox'
lit.py: lit.cfg:144: note: testing using 'FileCheck': '/home/modocache/GitHub/apple/build/buildbot_linux/llvm-linux-x86_64/bin/FileCheck'
lit.py: lit.cfg:147: note: testing using 'swift': '/home/modocache/GitHub/apple/build/buildbot_linux/none-swift_package_sandbox/usr/bin/swift'
lit.py: lit.cfg:150: note: testing using 'swiftc': '/home/modocache/GitHub/apple/build/buildbot_linux/none-swift_package_sandbox/usr/bin/swiftc'
lit.py: lit.cfg:153: note: testing using 'lldb': /home/modocache/GitHub/apple/build/buildbot_linux/none-swift_package_sandbox/usr/bin/lldb
Testing Time: 9.80s
  Expected Passes    : 11
  Unsupported Tests  : 5

I wonder if maybe this problem is now fixed, perhaps thanks to #1757. I'll do a bit of bisecting. In the meantime, could you confirm:

Whether this problem is now resolved?
If it is now resolved, is any work necessary to backport a fix, such as picking a fix into a snapshot release branch or somerhing?

rintaro · 2016-03-22T21:52:25Z

stdlib/public/Glibc/CMakeLists.txt

-    DESTINATION "lib/swift/glibc")
+      swift_install_in_component(stdlib
+          FILES "${output_dir}/glibc.modulemap"
+          DESTINATION "${output_dir}")


I think, this line was the problem.
${SWIFTLIB_DIR}/${SWIFT_SDK_${SDK}_LIB_SUBDIR}/${arch}
is not relevant for install destination.

Awesome, thanks @rintaro! I guess your removal of this directive in #1757 resolved the issue.

modocache · 2016-03-22T21:58:02Z

I just tried checking out the merge commit for this pull request, 38ceced, and running the package tests. Unfortunately, that worked as well--no test failures like the ones that appear in the CI build logs... there must be something else I've overlooking (perhaps some build artifacts from the passing build?). I'll continue looking into this, sorry for the trouble.

Ideas on what might be failing would be appreciated! 🙇

najacque · 2016-03-22T22:01:13Z

@shahmishal can you comment on the current state of the tests?

shahmishal · 2016-03-22T22:35:40Z

@najacque @modocache Build passed :) Thanks!
https://ci.swift.org/view/Packages/job/oss-swift-package-linux-ubuntu-14_04/

modocache · 2016-03-22T22:36:03Z

Oops, never mind--it was a build artifact that caused 38ceced to pass for me locally. With a completely clean build directory, that commit does fail with the same failures you've been seeing @najacque. So I do think that this was previously an issue, but it has since been resolved.

najacque · 2016-03-22T23:53:09Z

Ok, thanks for looking into it!

modocache reviewed Mar 16, 2016
View reviewed changes

modocache added a commit that referenced this pull request Mar 16, 2016

Merge pull request #1704 from modocache/unrevert-target-specific-glibc

38ceced

[Un-revert][Glibc] Configure modulemap for target, not host

modocache merged commit 38ceced into swiftlang:master Mar 16, 2016

modocache deleted the unrevert-target-specific-glibc branch March 16, 2016 23:25

rintaro reviewed Mar 22, 2016
View reviewed changes

modocache mentioned this pull request Apr 4, 2016

[build-script] XCTest CI builds on OS X #2036

Merged

1 task

[Un-revert][Glibc] Configure modulemap for target, not host #1704

[Un-revert][Glibc] Configure modulemap for target, not host #1704

Uh oh!

Conversation

modocache commented Mar 16, 2016

What's in this pull request?

Resolved bug number: None

Uh oh!

modocache Mar 16, 2016

Choose a reason for hiding this comment

Uh oh!

modocache commented Mar 16, 2016

Uh oh!

gribozavr commented Mar 16, 2016

Uh oh!

modocache commented Mar 16, 2016

Uh oh!

gribozavr commented Mar 16, 2016

Uh oh!

modocache commented Mar 16, 2016

Uh oh!

najacque commented Mar 22, 2016

Uh oh!

modocache commented Mar 22, 2016

Uh oh!

modocache commented Mar 22, 2016

Uh oh!

rintaro Mar 22, 2016

Choose a reason for hiding this comment

Uh oh!

modocache Mar 22, 2016

Choose a reason for hiding this comment

Uh oh!

modocache commented Mar 22, 2016

Uh oh!

najacque commented Mar 22, 2016

Uh oh!

shahmishal commented Mar 22, 2016

Uh oh!

modocache commented Mar 22, 2016

Uh oh!

najacque commented Mar 22, 2016

Uh oh!

Uh oh!