Skip to content

SR-7039: Add test for -static-stdlib when using Dispatch #58

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

Merged
merged 1 commit into from
Jul 19, 2019

Conversation

spevans
Copy link
Contributor

@spevans spevans commented May 27, 2019

No description provided.

@spevans
Copy link
Contributor Author

spevans commented May 27, 2019

@spevans
Copy link
Contributor Author

spevans commented May 30, 2019

@swift-ci test

@spevans
Copy link
Contributor Author

spevans commented May 30, 2019

@weissi Could you test and merge this please I don't have commit access to this repo, thanks

@weissi
Copy link

weissi commented May 30, 2019

@kevints is doing the release management for this release

@weissi
Copy link

weissi commented May 31, 2019

@swift-ci test

@AnnaZaks AnnaZaks requested a review from jrose-apple July 18, 2019 04:30
REQUIRES: platform=Linux
RUN: rm -rf %t
RUN: mkdir -p %t
RUN: %{swiftc} -static-stdlib %S/dispatch_test.swift -lDispatchStubs -lbsd -o %t/dispatch_test

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

-lDispatchStubs being necessary seems very suspect to me. Shouldn't that be handled by Dispatch itself?

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kevints is doing the release management for this release

This is targeted at master, so this should be reviewed by code owners instead.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For that matter -lbsd seems suspect. Is the problem a missing modulemap for libbsd?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

libdispatch.a is an archive library so wont auto link libswiftDispatch.a and libDispatchStubs.a hence being explicitly listed.

-lbsd was needed the last time I tested this to provide strlcpy and getprogname however it looks like they are now included as shims in the dispatch source tree so I have removed -lbsd from the test.

@kevints @jrose-apple Could someone please re-CI test this PR, thanks

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, @kevints reminded me about what we can and can't do with static libraries. I just wasn't thinking.

@spevans spevans force-pushed the pr_sr_7039_static_stdlib_test branch from e1bbc3e to 638cfa6 Compare July 18, 2019 21:41
@jrose-apple
Copy link

@swift-ci Please test

@jrose-apple
Copy link

@swift-ci test

@spevans spevans force-pushed the pr_sr_7039_static_stdlib_test branch from 638cfa6 to 67172d8 Compare July 19, 2019 14:07
@spevans
Copy link
Contributor Author

spevans commented Jul 19, 2019

@shahmishal Is it possible to run tests in this repo?

@shahmishal
Copy link
Member

We don't support PR testing on this repository yet.

@spevans
Copy link
Contributor Author

spevans commented Jul 19, 2019

I tested this PR using another one swiftlang/swift-corelibs-foundation#2335 and the log is here: https://ci.swift.org/job/swift-corelibs-foundation-PR-Linux/3441/consoleFull

Looks like it passed ok so I think its ok to merge now.

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.

5 participants