Skip to content

Support both static and dynamic linking mode in testing for vxWorks #63789

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 14 commits into from
Sep 7, 2019
Merged

Support both static and dynamic linking mode in testing for vxWorks #63789

merged 14 commits into from
Sep 7, 2019

Conversation

BaoshanPang
Copy link
Contributor

@BaoshanPang BaoshanPang commented Aug 21, 2019 •
edited
Loading

  1. Support both static and dynamic linking mode in testing for vxWorks
  2. Ignore unsupported test cases: net:tcp:tests:timeouts and net:ucp:tests:timeouts

r? @alexcrichton

BaoshanPang and others added 3 commits August 20, 2019 15:43
@BaoshanPang
Merge pull request #6 from rust-lang/master
0c8a14c 
rebase code from rust-lang/rust master branch
@bpangWR
run test for vxWorks in 'pure' static linking mode by default;
45d5f22 
if environment variables 'RUST_TEST_DYLINK' is set to 1, then run test in 'pure' dynamic linking mode
@BaoshanPang
Merge pull request #2 from Wind-River/bpang-runtest
b37d107 
Support static and dynamic linking mode for vxWorks in running test suite
@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Aug 21, 2019
n-salim and others added 2 commits August 22, 2019 15:26
@n-salim
VxWorks ignores the SO_SNDTIMEO socket option (this is long-standing
f5b1b1c 
behavior), so skip the following tests:

net::tcp::tests::timeouts
net::udp::tests::timeouts
@n-salim
Merge pull request #7 from Wind-River/vxworks-V7LIBC-942
76f1721 
Skip socket timeout tests on VxWorks
@alexcrichton
Copy link
Member

Thanks! With a name like use_dynamic_linking this doesn't necessarily sound vxworks-specific, so perhaps this could just be blanket applied to all targets? It looks like it's opt-in behavior so I don't think it'll break anything anyway.

Additionally could the other logic about selecting -Cprefer-dynamic all be centralized in this method as well?

bpangWR and others added 3 commits August 27, 2019 16:54
@bpangWR
run test for vxWorks in 'pure' static linking mode by default;
cae6d66 
if environment variables 'RUST_TEST_DYLINK' is set to 1, then run test in 'pure' dynamic linking mode
@BaoshanPang
Merge branch 'master' into bpang-runtest
043c19c
@BaoshanPang
Merge pull request #10 from Wind-River/bpang-runtest
964c37c 
run test for vxWorks in 'pure' static linking mode by default;
@BaoshanPang
Copy link
Contributor Author

BaoshanPang commented Aug 29, 2019 •
edited
Loading

Thanks! With a name like use_dynamic_linking this doesn't necessarily sound vxworks-specific, so perhaps this could just be blanket applied to all targets? It looks like it's opt-in behavior so I don't think it'll break anything anyway.

Additionally could the other logic about selecting -Cprefer-dynamic all be centralized in this method as well?

I have revised the code in more VxWorks specific way which should be safer.

@alexcrichton
Is this acceptable? I feel making it more genereic is little risk for me as I don't know other targets very well.

alexcrichton
alexcrichton reviewed Sep 3, 2019
View reviewed changes
src/tools/compiletest/src/runtest.rs Outdated

fn is_vxworks_pure_dynamic(&self) -> bool {
if self.config.target.contains("vxworks") {
match env::var("RUST_TEST_DYLINK") {
Copy link
Member

Choose a reason for hiding this comment

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

Could this branch perhaps just be !self.is_vxworks_pure_static()? (basically reading the env var only once)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Could this branch perhaps just be !self.is_vxworks_pure_static()? (basically reading the env var only once)

For !self.is_vxworks_pure_static(), it would be true for all non-vxworks targets which is not what we want.

Copy link
Member

Choose a reason for hiding this comment

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

Er sorry I meant just in this branch, so only if the target is vxworks do we delegate to is_vxworks_pure_static

Copy link
Contributor Author

@BaoshanPang BaoshanPang Sep 5, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

I see. I have uploaded the code that simply the function is_vxworks_pure_dynamic().

@alexcrichton
Copy link
Member

@bors: r+

@bors
Copy link
Collaborator

bors commented Sep 6, 2019

📌 Commit 414d104 has been approved by alexcrichton

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Sep 6, 2019
@bors
Copy link
Collaborator

bors commented Sep 6, 2019

⌛ Testing commit 414d104 with merge da13f06...

bors added a commit that referenced this pull request Sep 6, 2019
@bors
Auto merge of #63789 - Wind-River:master, r=alexcrichton
da13f06 
Support both static and dynamic linking mode in testing for vxWorks

1. Support both static and dynamic linking mode in testing for vxWorks
2. Ignore unsupported test cases: net:tcp:tests:timeouts and net:ucp:tests:timeouts

r? @alexcrichton
@bors
Copy link
Collaborator

bors commented Sep 7, 2019

☀️ Test successful - checks-azure
Approved by: alexcrichton
Pushing da13f06 to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Sep 7, 2019
@bors bors merged commit 414d104 into rust-lang:master Sep 7, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@alexcrichton alexcrichton alexcrichton left review comments

Assignees

@alexcrichton alexcrichton

Labels
merged-by-bors This PR was explicitly merged by bors. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@BaoshanPang @alexcrichton @bors @rust-highfive @bpangWR @n-salim