Libdispatch import order / nondeterminism


(Drew Crawford) #1

I have built the Feb 8th snapshot into /usr/local on Linux 64. I have enabled libdispatch, and it's installed to /usr/local as well. Additionally, I cherry-picked the PR https://github.com/apple/swift/pull/1212. It compiles, installs, everything's fine.

My /usr/local/include/dispatch directory has

# ls /usr/local/include/dispatch/
Dispatch.swiftdoc base.h data.h group.h io.h object.h queue.h source.h
Dispatch.swiftmodule block.h dispatch.h introspection.h module.map once.h semaphore.h time.h

Now I attempt to use Dispatch via

swift -Xcc -fblocks -L/usr/local/lib -I /usr/local/include/dispatch -lBlocksRuntime -L/usr/lib/x86_64-linux-gnu

This is because:

-Xcc -fblocks to pick up blocks support
-I /usr/local/include/dispatch to pick up /usr/local/include/dispatch/Dispatch.swiftmodule (as well as module.modulemap etc.)
-lBlocksRuntime -L/usr/lib/x86_64-linux-gnu to pick up blocks runtime

Okay. So now I import Dispatch:

Welcome to Swift version 3.0-dev (LLVM a7663bb722, Clang 4ca3c7fa28, Swift 319a912638). Type :help for assistance.
  1> import Dispatch

What I find is about half the time, this works just fine. The other half the time, I hit an import error:

<module-includes>:1:10: note: in file included from <module-includes>:1:
#include "/usr/local/include/dispatch/dispatch.h"
         ^
/usr/local/include/dispatch/dispatch.h:34:10: note: in file included from /usr/local/include/dispatch/dispatch.h:34:
#include <fcntl.h>
         ^
/usr/include/fcntl.h:67:11: note: in file included from /usr/include/fcntl.h:67:
# include <time.h>
          ^
/usr/local/include/dispatch/time.h:25:2: error: "Please #include <dispatch/dispatch.h> instead of this file directly."
#error "Please #include <dispatch/dispatch.h> instead of this file directly."
^

What seems to be happening here is that we interpret /usr/include/fcntl.h's #include <time.h> to be /usr/local/include/dispatch/time.h instead of /usr/include/time.h.

This behavior is puzzling to me.

I don't understand why the behavior is nondeterministic. Shouldn't an import failure be deterministic behavior? Or are we just trying imports in random order? That seems like a bad implementation.
I don't understand how to make sure fcntl's `time.h` goes to the right time.h without also losing Dispatch.swiftmodule and module.modulemap in the exchange.

Thanks in advance.


(Philippe Hausler) #2

I have built the Feb 8th snapshot into /usr/local on Linux 64. I have enabled libdispatch, and it's installed to /usr/local as well. Additionally, I cherry-picked the PR https://github.com/apple/swift/pull/1212. It compiles, installs, everything's fine.

My /usr/local/include/dispatch directory has

# ls /usr/local/include/dispatch/
Dispatch.swiftdoc base.h data.h group.h io.h object.h queue.h source.h
Dispatch.swiftmodule block.h dispatch.h introspection.h module.map once.h semaphore.h time.h

Now I attempt to use Dispatch via

swift -Xcc -fblocks -L/usr/local/lib -I /usr/local/include/dispatch -lBlocksRuntime -L/usr/lib/x86_64-linux-gnu

This is because:

-Xcc -fblocks to pick up blocks support
-I /usr/local/include/dispatch to pick up /usr/local/include/dispatch/Dispatch.swiftmodule (as well as module.modulemap etc.)
-lBlocksRuntime -L/usr/lib/x86_64-linux-gnu to pick up blocks runtime

Okay. So now I import Dispatch:

Welcome to Swift version 3.0-dev (LLVM a7663bb722, Clang 4ca3c7fa28, Swift 319a912638). Type :help for assistance.
  1> import Dispatch

What I find is about half the time, this works just fine. The other half the time, I hit an import error:

<module-includes>:1:10: note: in file included from <module-includes>:1:
#include "/usr/local/include/dispatch/dispatch.h"
         ^
/usr/local/include/dispatch/dispatch.h:34:10: note: in file included from /usr/local/include/dispatch/dispatch.h:34:
#include <fcntl.h>
         ^
/usr/include/fcntl.h:67:11: note: in file included from /usr/include/fcntl.h:67:
# include <time.h>
          ^
/usr/local/include/dispatch/time.h:25:2: error: "Please #include <dispatch/dispatch.h> instead of this file directly."
#error "Please #include <dispatch/dispatch.h> instead of this file directly."
^

What seems to be happening here is that we interpret /usr/include/fcntl.h's #include <time.h> to be /usr/local/include/dispatch/time.h instead of /usr/include/time.h.

This behavior is puzzling to me.

I don't understand why the behavior is nondeterministic. Shouldn't an import failure be deterministic behavior? Or are we just trying imports in random order? That seems like a bad implementation.
I don't understand how to make sure fcntl's `time.h` goes to the right time.h without also losing Dispatch.swiftmodule and module.modulemap in the exchange.

The -I/usr/local/include/dispatch probably sees time.h in there and thinks that is part of a system root header? Just a speculation.

···

On Feb 10, 2016, at 6:05 PM, Drew Crawford via swift-corelibs-dev <swift-corelibs-dev@swift.org> wrote:

Thanks in advance.

_______________________________________________
swift-corelibs-dev mailing list
swift-corelibs-dev@swift.org
https://lists.swift.org/mailman/listinfo/swift-corelibs-dev


(Drew Crawford) #3

I agree 100%, but I don't know how to fix it.

···

On Feb 10, 2016, at 8:08 PM, Philippe Hausler <phausler@apple.com> wrote:

The -I/usr/local/include/dispatch probably sees time.h in there and thinks that is part of a system root header? Just a speculation.


(Daniel A. Steffen) #4

having -I /usr/local/include/dispatch doesn’t seem right to me, the C header include convention is all <dispatch/*.h> so the dispatch/ directory itself should not be part of the search path.

maybe that means that something different needs to happen to pick up the module files (AFAIK on Darwin this works differently by having a toplevel modulefile in /usr/include that references the submodules, I don’t know how modules are intended to work on other platforms)

Daniel

···

On Feb 10, 2016, at 18:09, Drew Crawford via swift-corelibs-dev <swift-corelibs-dev@swift.org> wrote:

On Feb 10, 2016, at 8:08 PM, Philippe Hausler <phausler@apple.com <mailto:phausler@apple.com>> wrote:

The -I/usr/local/include/dispatch probably sees time.h in there and thinks that is part of a system root header? Just a speculation.

I agree 100%, but I don't know how to fix it.
_______________________________________________
swift-corelibs-dev mailing list
swift-corelibs-dev@swift.org
https://lists.swift.org/mailman/listinfo/swift-corelibs-dev


(Daniel Dunbar) #5

I don't have the full context here, but...

For C and regular header include style, it works roughly the same on linux, except that we don't have all the modules declared in one level. However:

  #include <dispatch/dispatch.h>

will cause the compiler to look inside the dispatch subdirectory, at which point it should seem the module map file and then load dispatch as a module.

However, this doesn't work for Swift because Swift doesn't go through the header search path, it goes through the "module search" path (like @import foo in Obj-C). For that to work, Clang will need to know to preload the module map file in order to understand how to map dispatch. Clang has `-fmodule-map-file=<path>` to cause Clang to scan (not necessarily load) a module map in advance.

- Daniel

···

On Feb 10, 2016, at 8:31 PM, Daniel A. Steffen via swift-corelibs-dev <swift-corelibs-dev@swift.org> wrote:

having -I /usr/local/include/dispatch doesn’t seem right to me, the C header include convention is all <dispatch/*.h> so the dispatch/ directory itself should not be part of the search path.

maybe that means that something different needs to happen to pick up the module files (AFAIK on Darwin this works differently by having a toplevel modulefile in /usr/include that references the submodules, I don’t know how modules are intended to work on other platforms)

Daniel

On Feb 10, 2016, at 18:09, Drew Crawford via swift-corelibs-dev <swift-corelibs-dev@swift.org <mailto:swift-corelibs-dev@swift.org>> wrote:

On Feb 10, 2016, at 8:08 PM, Philippe Hausler <phausler@apple.com <mailto:phausler@apple.com>> wrote:

The -I/usr/local/include/dispatch probably sees time.h in there and thinks that is part of a system root header? Just a speculation.

I agree 100%, but I don't know how to fix it.
_______________________________________________
swift-corelibs-dev mailing list
swift-corelibs-dev@swift.org <mailto:swift-corelibs-dev@swift.org>
https://lists.swift.org/mailman/listinfo/swift-corelibs-dev

_______________________________________________
swift-corelibs-dev mailing list
swift-corelibs-dev@swift.org
https://lists.swift.org/mailman/listinfo/swift-corelibs-dev


(Drew Crawford) #6

The problem is that to pick up a modulemap / swiftmodule file right now in Swift, we need "-I /path/to" where /path/to contains "module.modulemap" / "foo.swiftmodule"

Meanwhile passing "-I /path/to" also will pick up all header files in that directory, which here includes "time.h".

I personally think that behavior (the Swift compiler behavior) is wrong to couple these two ideas. But arguing the compiler is wrong is probably above my pay grade.

Anyway, as long as this is the compiler behavior, we can't have "time.h" in the same directory as the modulemap. So either

we put swiftmodule / modulemap in /usr/local/include/dispatch and headers into /usr/local/include/dispatch/headers
we put headers in /usr/local/include/dispatch and swiftmodule/modulemap into /usr/local/include/dispatch/module

I don't know which one to do, and I'm bad at wrestling autotools, so I'm not sure I can PR.

I'm working around by arbitrarily picking one of these resolutions and shell scripting it as part of my install.

···

On Feb 10, 2016, at 10:31 PM, Daniel A. Steffen <dsteffen@apple.com> wrote:

having -I /usr/local/include/dispatch doesn’t seem right to me, the C header include convention is all <dispatch/*.h> so the dispatch/ directory itself should not be part of the search path.


(Dave Grove) #7

To: "Daniel A. Steffen" <dsteffen@apple.com>
Cc: swift-corelibs-dev@swift.org
Date: 02/11/2016 06:19 AM
Subject: Re: [swift-corelibs-dev] libdispatch import order /

nondeterminism

Sent by: swift-corelibs-dev-bounces@swift.org

having -I /usr/local/include/dispatch doesn’t seem right to me, the
C header include convention is all <dispatch/*.h> so the dispatch/
directory itself should not be part of the search path.

The problem is that to pick up a modulemap / swiftmodule file right
now in Swift, we need "-I /path/to" where /path/to contains
"module.modulemap" / "foo.swiftmodule"

Meanwhile passing "-I /path/to" also will pick up all header files
in that directory, which here includes "time.h".

I personally think that behavior (the Swift compiler behavior) is
wrong to couple these two ideas. But arguing the compiler is wrong
is probably above my pay grade.

Anyway, as long as this is the compiler behavior, we can't have
"time.h" in the same directory as the modulemap. So either

we put swiftmodule / modulemap in /usr/local/include/dispatch and
headers into /usr/local/include/dispatch/headers
we put headers in /usr/local/include/dispatch and swiftmodule/
modulemap into /usr/local/include/dispatch/module

I don't know which one to do, and I'm bad at wrestling autotools, so
I'm not sure I can PR.

I'm working around by arbitrarily picking one of these resolutions
and shell scripting it as part of my install.

I'm working on changes so that when built with swift/utils/build-script the
pieces of libdispatch get properly placed in the installable Swift package
that is being built. I think this is actually where we want to get to,
and it avoids needing to pass the -I arguments to swiftc. When the bits
are placed properly in the Swift install, swiftc is finding them without
help.

I think I'm on track to submit a PR later today. So far, changes are
confined to in the autotools setup in libdispatch.

--dave

···

swift-corelibs-dev-bounces@swift.org wrote on 02/11/2016 06:18:58 AM: > From: Drew Crawford via swift-corelibs-dev <swift-corelibs-dev@swift.org>

On Feb 10, 2016, at 10:31 PM, Daniel A. Steffen <dsteffen@apple.com> wrote:


(Daniel A. Steffen) #8

having -I /usr/local/include/dispatch doesn’t seem right to me, the C header include convention is all <dispatch/*.h> so the dispatch/ directory itself should not be part of the search path.

The problem is that to pick up a modulemap / swiftmodule file right now in Swift, we need "-I /path/to" where /path/to contains "module.modulemap" / "foo.swiftmodule"

Meanwhile passing "-I /path/to" also will pick up all header files in that directory, which here includes "time.h".

I personally think that behavior (the Swift compiler behavior) is wrong to couple these two ideas. But arguing the compiler is wrong is probably above my pay grade.

Anyway, as long as this is the compiler behavior, we can't have "time.h" in the same directory as the modulemap. So either

we put swiftmodule / modulemap in /usr/local/include/dispatch and headers into /usr/local/include/dispatch/headers

I doubt that is a very feasible option, we need to keep the installed library compatible with C clients (the CF pieces in corelibs-foundation for one) and we’d like to diverge from the Darwin install layout as little as possible

···

On Feb 11, 2016, at 3:18, Drew Crawford <drew@sealedabstract.com> wrote:

On Feb 10, 2016, at 10:31 PM, Daniel A. Steffen <dsteffen@apple.com <mailto:dsteffen@apple.com>> wrote:

we put headers in /usr/local/include/dispatch and swiftmodule/modulemap into /usr/local/include/dispatch/module

I don't know which one to do, and I'm bad at wrestling autotools, so I'm not sure I can PR.

I'm working around by arbitrarily picking one of these resolutions and shell scripting it as part of my install.


(Dave Grove) #9

I've done some exploring this morning (see branch [1]). It improves
things, but I'm hoping we can do better, which is why there isn't a PR yet.

The branch does two things:
  (a) Add a configure option to cause "make install" to install into a
directory structure that matches the one used for a Swift installable
package instead of its default structure.
  (b) Some cleanup of how module.map is generated and where it is
installed normally based on this email chain.

Part (a) seems to be working fine. I want to do one more test-build of
Swift from scratch to make sure, but yesterday I was getting a Swift build
where the resulting swiftc could successfully fully import Dispatch without
any extra command line arguments.

Part (b) isn't any worse and maybe is marginally better, but is not really
doing what we want. In particular, with no extra command line arguments,
swiftc seems to be finding the module.map file and importing the C header
files, but it isn't finding Dispatch.swiftmodule. As a result,
dispatch_group_async (for example) is available to the Swift program, but
DISPATCH_QUEUE_CONCURRENT is not.

If I give swiftc -I/usr/local/include/dispatch then it finds
Dispatch.swiftmodule and DISPATCH_QUEUE_CONCURRENT is available.

Giving -Xcc -fmodule-module-map=<file> does not help;
DISPATCH_QUEUE_CONCURRENT is not available.

In a "normal" install we get in PREFIX (/usr/local by default).
  include/dispatch/
    *.h from dispatch directory
    module.map
    Dispatch.swiftmodule
    Dispatch.swiftdoc
  include/os
    *.h from os directory
  lib
    libdispatch.so

I've tried putting Dispatch.swiftmodule in a variety of places in
the /usr/include tree (lib, lib/x86_64), but none of them seem to help.

Any thoughts on other things to try before submitting a PR?

thanks,

--dave

[1]
https://github.com/dgrove-oss/swift-corelibs-libdispatch/tree/swift-overlay-cp2


(Daniel Dunbar) #10

I missed that a swift module was also being installed here. +Jordan, who probably has an opinion on how this should be done.

My guess is that we want a story where this all works when installed as part of a swift toolchain, and where it would be bundle inside the compiler like an overlay.

I don't think we have a real story for installing .swiftmodule files, because that usually isn't safe (they are tied to the compiler version).

- Daniel

···

On Feb 11, 2016, at 11:00 AM, Daniel A. Steffen via swift-corelibs-dev <swift-corelibs-dev@swift.org> wrote:

On Feb 11, 2016, at 3:18, Drew Crawford <drew@sealedabstract.com <mailto:drew@sealedabstract.com>> wrote:

On Feb 10, 2016, at 10:31 PM, Daniel A. Steffen <dsteffen@apple.com <mailto:dsteffen@apple.com>> wrote:

having -I /usr/local/include/dispatch doesn’t seem right to me, the C header include convention is all <dispatch/*.h> so the dispatch/ directory itself should not be part of the search path.

The problem is that to pick up a modulemap / swiftmodule file right now in Swift, we need "-I /path/to" where /path/to contains "module.modulemap" / "foo.swiftmodule"

Meanwhile passing "-I /path/to" also will pick up all header files in that directory, which here includes "time.h".

I personally think that behavior (the Swift compiler behavior) is wrong to couple these two ideas. But arguing the compiler is wrong is probably above my pay grade.

Anyway, as long as this is the compiler behavior, we can't have "time.h" in the same directory as the modulemap. So either

we put swiftmodule / modulemap in /usr/local/include/dispatch and headers into /usr/local/include/dispatch/headers

I doubt that is a very feasible option, we need to keep the installed library compatible with C clients (the CF pieces in corelibs-foundation for one) and we’d like to diverge from the Darwin install layout as little as possible

we put headers in /usr/local/include/dispatch and swiftmodule/modulemap into /usr/local/include/dispatch/module

I don't know which one to do, and I'm bad at wrestling autotools, so I'm not sure I can PR.

I'm working around by arbitrarily picking one of these resolutions and shell scripting it as part of my install.

_______________________________________________
swift-corelibs-dev mailing list
swift-corelibs-dev@swift.org
https://lists.swift.org/mailman/listinfo/swift-corelibs-dev


(Dave Grove) #11

It certainly is easier to only support the scenario when libdispatch with
embedded Swift overlay is being built/installed as part of a complete Swift
toolchain. If that is all that can be robustly supported
(because .swiftmodule is tied to the compiler version), then it will be
easy to adapt what I've done to only support that scenario. Is that what
I should do?

--dave

            <jordan_rose@apple.com>, David P Grove/Watson/IBM@IBMUS
            swift-corelibs-dev@swift.org
            nondeterminism
Sent by: daniel_dunbar@apple.com

I missed that a swift module was also being installed here. +Jordan, who
probably has an opinion on how this should be done.

My guess is that we want a story where this all works when installed as
part of a swift toolchain, and where it would be bundle inside the compiler
like an overlay.

I don't think we have a real story for installing .swiftmodule files,
because that usually isn't safe (they are tied to the compiler version).

- Daniel

···

From: Daniel Dunbar <daniel_dunbar@apple.com>
To: "Daniel A. Steffen" <dsteffen@apple.com>, Jordan Rose
Cc: Drew Crawford <drew@sealedabstract.com>,
Date: 02/11/2016 02:04 PM
Subject: Re: [swift-corelibs-dev] libdispatch import order /

      On Feb 11, 2016, at 11:00 AM, Daniel A. Steffen via swift-corelibs-dev <swift-corelibs-dev@swift.org> wrote:

            On Feb 11, 2016, at 3:18, Drew Crawford < drew@sealedabstract.com> wrote:

                  On Feb 10, 2016, at 10:31 PM, Daniel A. Steffen < dsteffen@apple.com> wrote:

                  having -I /usr/local/include/dispatch doesn’t seem right
                  to me, the C header include convention is all
                  <dispatch/*.h> so the dispatch/ directory itself should
                  not be part of the search path.

            The problem is that to pick up a modulemap / swiftmodule file
            right now in Swift, we need "-I /path/to" where /path/to
            contains "module.modulemap" / "foo.swiftmodule"

            Meanwhile passing "-I /path/to" also will pick up all header
            files in that directory, which here includes "time.h".

            I personally think that behavior (the Swift compiler behavior)
            is wrong to couple these two ideas. But arguing the compiler
            is wrong is probably above my pay grade.

            Anyway, as long as this is the compiler behavior, we can't have
            "time.h" in the same directory as the modulemap. So either

                  we put swiftmodule / modulemap
                  in /usr/local/include/dispatch and headers
                  into /usr/local/include/dispatch/headers

      I doubt that is a very feasible option, we need to keep the installed
      library compatible with C clients (the CF pieces in
      corelibs-foundation for one) and we’d like to diverge from the Darwin
      install layout as little as possible

                  we put headers in /usr/local/include/dispatch and
                  swiftmodule/modulemap
                  into /usr/local/include/dispatch/module

            I don't know which one to do, and I'm bad at wrestling
            autotools, so I'm not sure I can PR.

            I'm working around by arbitrarily picking one of these
            resolutions and shell scripting it as part of my install.

      _______________________________________________
      swift-corelibs-dev mailing list
      swift-corelibs-dev@swift.org
      https://lists.swift.org/mailman/listinfo/swift-corelibs-dev


(Drew Crawford) #12

I think if Swift is available on the system at installation time, it would be safe to use it to generate/install the overlay.

···

On Feb 11, 2016, at 1:19 PM, David P Grove <groved@us.ibm.com> wrote:

It certainly is easier to only support the scenario when libdispatch with embedded Swift overlay is being built/installed as part of a complete Swift toolchain. If that is all that can be robustly supported (because .swiftmodule is tied to the compiler version), then it will be easy to adapt what I've done to only support that scenario. Is that what I should do?