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

Add additional compiler/linker options in Package.swift #107

Closed
wants to merge 4 commits into from
Closed

Add additional compiler/linker options in Package.swift #107

wants to merge 4 commits into from

Conversation

drewcrawford
Copy link
Contributor

For various reasons, users may want to add custom compiler and linker flags, at least for the present.

Why should we do this?

Because it provides either a solution or a workaround for all these bugs:

That's 20% of the open bugs. There are probably more than that, that we haven't heard about yet.

While this approach is far from perfect... it works, and a patch in the hand is worth two in the bush. Designing "perfect" solutions can happen in parallel with offering a workaround for these problems right now.

What should we add to this?

Maybe we should print a notice to people who use this feature that they should file a bug in the tracker to explain why.

Maybe we should warn the user that this feature is "deprecated" and that we intend to remove it when we have "final" solutions to the problems it solves.

Haven't we tried this before?

Yes, this is the resurrection of #75 by @OutOfBedlam as I suggested on the mailing list. Differences include:

@drewcrawford drewcrawford changed the title Add addtional compiler/linker options in Package.swift Add additional compiler/linker options in Package.swift Jan 4, 2016
@dunkelstern
Copy link

Cool, this would fix quite some workarounds I've been doing currently.

Perhaps it would be even better to include something like this in module maps too? (Example: Glibc needs -D_GNU_SOURCE when importing headers to unlock the full api.) We already have the link statement there to define against which libs we want to link but there's no possibility to define C preprocessor macros.

@onevcat
Copy link
Contributor

onevcat commented Jan 4, 2016

It's great!

@colemancda
Copy link

Awesome!

@mxcl
Copy link
Contributor

mxcl commented Jan 4, 2016

I haven't read through all of the material here yet, but just to throw in my immediate rational for why I would rather solve this properly than to accept this patch:

Packaging ecosystems are fragile.

We all have installed some package that tries to depend on something that cannot be built. It is very frustrating. Every time it happens we say “Why didn't they build a better packaging system that isn't so fragile?!" Well, here we are trying to do that.

Allowing completely arbitrary compiler flags is fragile. If swiftpm cannot infer anything about what the flag does it cannot intelligently apply the flag, and this leads to build breakage.

I'll review everything though. I'm all for helping the current situation with a patch that can be removed, but as said, the problem is more that packages stick around and don't get fixed and small decisions today can cause problems for years ahead.

However, whatever happens, I consider it a priority (now holidays are done) to provide something to satisfy this problem.

@dglo
Copy link

dglo commented Jan 4, 2016

This PR adds the user-specified paths after /usr/local/{include,lib}, making it difficult to override anything in that directory. I'd prefer to see the hard-coded /usr/local paths added last.

@drewcrawford
Copy link
Contributor Author

I would rather solve this properly than to accept this patch

I would rather solve it properly too, but I'm not sure it is realistic, or at least in realistic in a medium-term timeframe.

SR-397 is a really good bellwether. Of course that is a compiler bug. Of course that is where it should be solved. But in the mind of a compiler engineer, "a workaround exists". You "simply" have to compile with -fblocks on Linux.

So now we have 3 options:

  1. We can tell users to go jump in a lake. They are not allowed to use the compiler flag that works around the bug, because the threat of someone creating a fragile package someday is judged worse than not being able to build software right now.
  2. We can apply force apply -fblocks inside SwiftPM, (and more generally) carefully curate and maintain a list of compiler flags to work around Compiler Bug of the Week.
  3. We can let users apply their own compiler flags and warn them not to be stupid.

1 is just going to stop people from using SwiftPM altogether, or force early adopters into a fork that has this PR applied. That's a fragile ecosystem; one where the ecosystem has unofficial Package.swift extensions.

2 devolves to 1, because we probably do not know what is the Compiler Bug of the Week until the snapshot goes out.

That leaves door 3. I do not see any way to avoid it.

The disconnect here might be that where some people see a list of 5 bugs and we can just solve them all, others see an unknown number of bugs that we are discovering at a rate of one per week. Under that framework this problem is not as simple as "just" writing 5 proposals, although we should definitely start on that.

@mxcl
Copy link
Contributor

mxcl commented Jan 4, 2016

I'm cool with allowing extra flags on the command line. Since this won't dirty the packaging ecosystem.

swift build -Lfoo -lfoo

@ddunbar
Copy link
Contributor

ddunbar commented Jan 4, 2016

@drewcrawford One thing that I think is worth calling out here is that this project is still very much a work in progress. You should think of it as an alpha code base that hasn't had a release yet. Yes, it is useful for doing some things, and we are excited people are using it to do cool stuff, but the approach we want to take for developing and adding features is to be in a good place at the time of release...

With that mindset, it doesn't usually make sense to add features that we consider actively/potentially harmful, even if that solves some current short term pain.

@drewcrawford
Copy link
Contributor Author

@ddunbar I think this may be a cathedral/bazaar thing. Pushing users away until we anticipate and solve all their problems strikes me as a recipe for shipping something badly broken because we won't think of everything and we've sent away the people who would have helped us find the problems. Whereas if we let pedestrians define the walkways then we have higher confidence that we are actually building a good solution.

Swift is unusable on Linux without a build system. People are going to do something. We have the opportunity to define that something. Should they use autotools? Should they fork the project? Should they use additionalCompilerOptions? IMO one of these is better than the others. I understand that none of them are great, but people are going to build their software on Linux some way or another regardless of what we think.

@mxcl command line stuff is better than nothing, but SR-235 needs a per-target solution; we don't link all dependencies with the same library. Would you support a swift build --foolibrary-compileargs="..." kind of solution?

@mxcl
Copy link
Contributor

mxcl commented Jan 5, 2016

Linking every target is unlikely to be a problem. Applying -fblocks to all targets is unlikely to be a problem in most situations. This is after all just a workaround solution.

Can we change this patch so:

swift build -Xcc -fblocks

Works?

I don't think these things should be in Package.swift. These sort of problems need to be solved properly. Most users when they encounter this sort of issue look for a workaround and move on. If they can just add stuff to the manifest they will commit it and forget about it, if they have to type it every time they will pressure us to fix it properly.


The -Xcc syntax follows that of swiftc, etc. but maybe there's something nicer that can be proposed.

@TomasLinhart
Copy link

Would this -Xcc allow to specify custom header paths so SR-145 is possible to workaround?

@mxcl
Copy link
Contributor

mxcl commented Jan 5, 2016

Yes.

@drewcrawford
Copy link
Contributor Author

Is the specification that any arguments after "swift build" are passed to Swiftc? Is there any way to pass arguments to the linker?

I agree that if we can solve this in a way that doesn't pollute Package.swift, we should.

Sent from my iPhone

On Jan 5, 2016, at 1:27 PM, Tomáš Linhart [email protected] wrote:

Would this -Xcc allow to specify custom header paths so SR-145 is possible to workaround?


Reply to this email directly or view it on GitHub.

@mxcl
Copy link
Contributor

mxcl commented Jan 5, 2016

Arguments after -Xcc pass to the compiler, arguments after -Xlinker go to the linker. Same as to swiftc

swiftc also directly maps -I, -l etc. so we could do that too, though I'm less keen as it seems to be encouraging these work arounds.

@dunkelstern
Copy link

If it is done that way the users of swiftpm will likely create build scripts again, i don't know if that is the best thing as all packages will have their own method of building.

If I build a project that depends on a package that needs -fblocks I would have to supply that parameter to my build command even if my project does not directly need the parameter?

Example:
"MyProject" depends on "WebFramework"
"WebFramework" depends on "WebServer"
"WebServer" used GCD and so depends on -fblocks

Will I have to supply that -Xcc -fblocks when building my project? That would mean I have to check each and every dependency if it needs some parameters?

Example: https://github.com/dunkelstern/unchained has about 17 dependencies, some would probably need compiler flags on one OS but not the other... How can we make sure it will not get a build system hell?

@mxcl
Copy link
Contributor

mxcl commented Jan 6, 2016

@dunkelstern we will properly solve any situation where a -Xcc is needed, this is strictly a thread for temporary workarounds.

@dunkelstern
Copy link

Even in that case I personally would vote for the original pull request. Perhaps throw a warning if a package is using additional parameters.

I feel the problem with these workarounds is, they tend to stick longer than anyone would expect and yes, they are possibly fragile, but it is even more fragile if I have to know all dependencies by heart.

I think the problem is that we want the users to rely on the package manager and not creating makefiles for building stuff again. And if the early adopters (like myself) can't communicate how to build their projects because of obscure command line parameters we will have the situation that there will be build scripts again. (like "Use the following blob of bash code to build the project or die")

@TomasLinhart
Copy link

@mxcl Actually this PR could be accepted and whenever you use this parameter, you get a warning about using it and saying it will be replaced in future with different approach.

And once you have better way how to solve this problem, you just kill this parameter and package maintainers just update their package without changing anything in the application that is using the package.

@mxcl
Copy link
Contributor

mxcl commented Jan 6, 2016

If we build a system where these work arounds are required for a long time then we have failed.

So far every need for these work arounds can be solved in short-order, and we have plans to do so.

@drewcrawford
Copy link
Contributor Author

every need for these work arounds can be solved in short-order, and we have plans to do so.

I think the real question is whether these bugs are really a closed set, or whether it is a Compiler Bug of the Week type situation. Because in the latter case, no finite number of proposals can ever resolve the problems addressed by this PR.

My intuition is that is we are there. This is based on a few lines of evidence:

  1. That the related bugs has grown at a predictable rate of very roughly one per week
  2. Early adopters, like myself and apparently @dunkelstern, are considering crazy workarounds to the potential closing of this PR.
  3. That xcodebuild, autotools, ninja, setuptools, are "mature" toolchains that all support this, and I use this feature in all of them. (Is there anyone who reports they don't use the "other compiler flags" in some Xcode project right now?)
  4. The only excpetion I can think of at all is cargo.

What happened with cargo

cargo is, perhaps, a close model for us, as they are the package manager for Rust, which is an extremely similar language. They adopted, in 2014, a philosophy apparently identical to the one @mxcl / @ddunbar are advocating here.

It should be noted that the design of cargo is currently to explicitly not allow arbitrary flags. It is very easy to have compilations go awry very quickly, and packages specifying flags for themselves is likely a decision that should not be up to the author but rather the builder.

We certainly plan on allowing more flavorful configuration that is possible today through profiles, but it will likely not just be "pass these flags through to the compiler".

Right now profiles are pretty bare, only supporting opt-level and debug, but the plan is to make them much richer by adding a variety of other options. At this time we don't plan on allowing, through the normal cargo build interface, arbitrary flags to be passed to the compiler.

Which they tried for a year and did not work very well. I will fast-forward the numerous issues on this topic that were opened in their tracker and focus specifically on the part where the language authors started subverting this philosophy:

Rust team members have told me that they replace the rustc binary with a shell script that calls out to the binary itself in order to add extra arguments, as a workaround for this lack of functionality when debugging.

Servo, which is "the" flagship Rust software, had a list of serious objections to this as well:

Note that those are off the top of my head, just things we've wanted to do in the last couple of weeks. I would much rather see you prohibit the specific rustc flags that you don't want people to pass through rather than one-by-one enable subsets from the vast array of flags that do not jeapordize your platform-independent compilation goals.

Two years later, this philosophy has softened considerably. Specifically:

  1. In practice, many, many people (ab)use the "external build system" in cargo to build pure Rust projects, to work around this limitation.
  2. They have introduced a special command that, while not including arguments in the manifest itself, allows a user to script full customization in a few lines of bash.
  3. The cargo maintainer has reversed his position, now saying that we should "avoid" putting flags in the manifest but patches on this subject are "always a future possibility!"

Let's learn from cargo's example.

tl;dr

If we build a system where these work arounds are required for a long time then we have failed.

Then xcodebuild et al are failures. And that is not a definition of "failure" I care about. I do care very much about the failure of the cargo approach to this problem, and I care that we don't import an approach that is known to start a lot of flamewars and insane workarounds into the Swift language.

I think I've made the best case I can for this. If it wasn't convincing then close the PR and I'll maintain it in a fork. One thing I learned during the cargo edition of this debate is that sometimes the efficient solution is for everybody to just do what they think is best.

@colemancda
Copy link

@drewcrawford really makes a convincing case. I understand exactly what @mxcl is trying to do, but these issues are not a limited subset, there are even more issues than the reported bugs related to this, and I personally know people who are not creating more tickets because they don't want to put a burden the developers of SPM, nonetheless this is a big issue and people are resorting to makefiles, which should not be happening.

(I reported SR-83)

@colemancda
Copy link

Take a look at PerfectlySoft/Perfect, the biggest Swift backend framework. They are using makefiles due to the limitations of SPM.

@mxcl
Copy link
Contributor

mxcl commented Jan 7, 2016

SwiftPM is still very young. It's not surprising Perfect are having to work around things we don't yet do. We don't do very much yet.

Let's take a deep breath. Nothing is final yet. Let us try to solve these problems without doing irreparable damage to our ecosystem. If we fail like Cargo apparently did, then we will accept the inevitable.

@colemancda
Copy link

Sounds reasonable. Thanks.

Coleman,

On Jan 6, 2016, at 8:50 PM, Max Howell [email protected] wrote:

SwiftPM is still very young. It's not surprising Perfect are having to work around things we don't yet do. We don't do very much yet.

Let's take a deep breathe. Nothing is final yet. Let us try to solve these problems without doing irreparable damage to our ecosystem. If we fail like Cargo apparently did, then we will accept the inevitable.


Reply to this email directly or view it on GitHub #107 (comment).

@mxcl
Copy link
Contributor

mxcl commented Jan 7, 2016

I have a proposal for solving the -I, -L, -l issues almost ready to send. The -fblocks issue can be solved, but I'm waiting on a little more feedback before we make a decision.

In the meantime I think an -Xcc etc. situation to allow people to workaround these sort of issues is important. It is inevitable that these situations arise, now and in the future, developer tools are always innovating and changing. So we should change this patch to allow that.

In the future if it appears we are failing to provide solutions to any flag issues that arise, we can revisit this suggestion.

I stand by my statement that we owe it to the future of this community to not make it easy for Packages to introduce breakage. Build-systems are fragile, and must be assembled carefully, this requires the toolchain to control the process without individual packages having the power to stick a dagger in.

@colemancda
Copy link

What if the packages aren’t allowed to add linker flags, but the final build command is. This would provide a workaround for a lot of people, and would be “use at your own risk”. Again, this would not ruin the ecosystem, since the Package declaration would not change. It would very useful for the edge cases, and would require careful use by the end user, since he or she mush know all the linker flags before hand. That would provide a temporary solution, and it would feel more like a temporary crutch. I suggest command like arguments for “swift build” . Again, since the package.swift file would not change, maybe the project owners would feel more comfortable.

Coleman,

On Jan 6, 2016, at 9:07 PM, Max Howell [email protected] wrote:

I have a proposal for solving the -I, -L, -l issues almost ready to send. The -fblocks issue can be solved, but I'm waiting on a little more feedback before we make a decision.

In the meantime I think an -Xcc etc. situation to allow people to workaround these sort of issues is important. It is inevitable that these situations arise, now and in the future, developer tools are always innovating and changing. So we should change this patch to allow that.

In the future if it appears we are failing to provide solutions to any flag issues that arise, we can revisit this suggestion.

I stand by my statement that we owe it to the future of this community to not make it easy for Packages to introduce breakage. Build-systems are fragile, and must be assembled carefully, this requires the toolchain to control the process without individual packages having the power to stick a dagger in.


Reply to this email directly or view it on GitHub #107 (comment).

@mxcl
Copy link
Contributor

mxcl commented Jan 7, 2016

What if the packages aren’t allowed to add linker flags, but the final build command is. This would provide a workaround for a lot of people, and would be “use at your own risk”. Again, this would not ruin the ecosystem, since the Package declaration would not change. It would very useful for the edge cases, and would require careful use by the end user, since he or she mush know all the linker flags before hand. That would provide a temporary solution, and it would feel more like a temporary crutch. I suggest command like arguments for “swift build” . Again, since the package.swift file would not change, maybe the project owners would feel more comfortable.

Unless I'm mistaken, this is what I already proposed.

@drewcrawford
Copy link
Contributor Author

Certainly I think there is time to see if there a "third way", that is neither cargo's way, nor xcodebuild/autotools/setuptools way.

The thing I really object to is doing what cargo did and expecting different results. It would be good to innovate here if we can, but that does require actually innovating, not repeating history.

Part of the problem is I don't understand exactly what problem @mxcl / @ddunbar are trying to solve. It's been stated that compiler flags are "fragile" and cannot be applied "intelligently" but I have not seen motivated examples, e.g. consider package Foo, it has these flags, that introduces these problems, it cannot build on System Z etc. We could consider blacklisting only the flags that cause trouble in this situation but that has follow-on consequences X and Y and that has led us to the present design.

An argument of that form helps everybody understand what problem the current practice is designed to solve, and help us identify the search space for what a third way might be that avoids the failures in other languages.

Alternatively an argument of that form might make clear that there is no solution to that problem that does not terminate in a cargo-like design, which is also a useful conclusion from my point of view.

On Jan 6, 2016, at 7:50 PM, Max Howell [email protected] wrote:

SwiftPM is still very young. It's not surprising Perfect are having to work around things we don't yet do. We don't do very much yet.

Let's take a deep breathe. Nothing is final yet. Let us try to solve these problems without doing irreparable damage to our ecosystem. If we fail like Cargo apparently did, then we will accept the inevitable.


Reply to this email directly or view it on GitHub #107 (comment).

@colemancda
Copy link

Oh ok, awesome. I misunderstood what the proposed solution with all the discussion covering this. I look forward to that PR. Thanks.

Coleman,

On Jan 6, 2016, at 9:19 PM, Max Howell [email protected] wrote:

What if the packages aren’t allowed to add linker flags, but the final build command is. This would provide a workaround for a lot of people, and would be “use at your own risk”. Again, this would not ruin the ecosystem, since the Package declaration would not change. It would very useful for the edge cases, and would require careful use by the end user, since he or she mush know all the linker flags before hand. That would provide a temporary solution, and it would feel more like a temporary crutch. I suggest command like arguments for “swift build” . Again, since the package.swift file would not change, maybe the project owners would feel more comfortable.

Unless I'm mistaken, this is what I already proposed.


Reply to this email directly or view it on GitHub #107 (comment).

@dunkelstern
Copy link

I think the real problem we're trying to solve here is interaction with library code, which means in most cases C. There are a ton of C build systems and all support compiler flags. I feel a lot of work has gone into every one of them and nobody before was able to solve it correctly.

Perhaps it is not even a problem of the swift package manager but really something that has to be solved on a lower level. As I understand everyone on the Swift team is trying to avoid building a compiler that has many flags you may set on your own. It should be simple to use (and I fully support that!). Most flags we're talking about (like include search paths, -fblocks etc.) are really flags for the underlying C/library interface layer so i think they should be included in the module maps of the library we try to use. If you get conflicting flags for libraries on this level there is probably no solution to include both of the conflicting libs in a project at all (because they won't work when trying to use them in a C project either).

I know this seems a bit off topic to exactly this pull request but I feel the discussion has landed here and is not currently on the mailing list (If requested we could of course discuss the issue there, if someone starts a thread).

In module maps we already have link, requires and conflicts, so why not include some more there as it seems it is specific to that layer. My proposal would be to have at least:

  • define to add a -D<something> flag (like -D_GNU_SOURCE we could really need for the Glibc)
  • some kind of searchPath to add a library and include search path (this would probably fix the issue that module maps are really platform specific right now, try to create one that works for iOS, OSX and Linux and you'll see)
  • compilerFlags for -Xcc type issues
  • linkerFlags for -Xlinker type issues

Furthermore the requires flag blocks could just turn on -fblocks if the platform supports it instead of failing with an obscure objc runtime not supported error. But I think that is probably just a bug.

The real positive side of this would be that the swiftpm does indeed not need any additionalParameters type settings as the really important settings would be defined by the library interface maintainer (who has the needed knowledge) and not the user of the library (who does not know what obscure parameters are needed for their 1000 dependencies).

@drewcrawford
Copy link
Contributor Author

Wow. IMO @dunkelstern's proposal is clearly superior.

  1. It's less general than this PR, which may assuage concerns of fully general solutions
  2. It is still general enough to solve the entire set of bugs addressed by this PR
  3. It encourages module map usage
  4. It tracks fblocks with libdispatch, not with its callers, which is a property this PR doesn't have.

I would prefer to see the define problem solved via this proposed language feature, but until and unless we get that, I agree we need that here.

This is very much a "third way".

@mxcl
Copy link
Contributor

mxcl commented Jan 13, 2016

I'm working on the -Xcc patch

@mxcl mxcl closed this in 2e841e1 Jan 14, 2016
@rolivieri
Copy link

This is great news that this issue has been closed! Do you guys happen to know when this enhancement will make it to the official swift snapshot (https://swift.org/download/#latest-development-snapshots)?

@mxcl
Copy link
Contributor

mxcl commented Jan 18, 2016

It'll be in the next snapshot. Should be tomorrow…?

@mxcl
Copy link
Contributor

mxcl commented Jan 18, 2016

Please note I closed it with the -Xcc patch and not this PR.

@eyelash
Copy link

eyelash commented Jan 26, 2016

Is there a way to pass multiple flags without having to put -Xcc in front of every flag? swift build -Xcc "-I/some/path -I/some/other/path" does not seem to be working.

@mxcl
Copy link
Contributor

mxcl commented Jan 26, 2016

@eyelash no. I'd be ok with adding something, but basically, this is how it works with build tools as tradition.

@TomasLinhart
Copy link

@mxcl I would also like to be able to pass multiple flags because it would make easier usage of pkg-config --cflags --libs xxx on Linux and other platforms.

@mxcl
Copy link
Contributor

mxcl commented Jan 27, 2016

I am reluctant to change how this works as we have adopted the same pattern as swiftc, to change it so you can input multiple flags without specifying Xcc repeatedly would be inconsistent.

I feel your pain, but it is inconsiderate to users to actively pursue a bad user experience: I acknowledge this particular UX is bad, but it would be even worse to be inconsistent.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.