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

Inhibit attempted copying of missing crypto.if.lib for static win32 build #8754

Closed
wants to merge 4 commits into from
Closed

Inhibit attempted copying of missing crypto.if.lib for static win32 build #8754

wants to merge 4 commits into from

Conversation

wrowe
Copy link
Contributor

@wrowe wrowe commented Oct 24, 2019

See discussion at;
https://github.com/envoyproxy/envoy/pull/8280/files/b84fc6bc69677d00bd3f8ddfcbd23efee0e44893#r332278621

This deserves attention by the boringssl bazel build maintainers, solving for short-term.

Signed-off-by: William A Rowe Jr wrowe@pivotal.io
Signed-off-by: Yechiel Kalmenson ykalmenson@pivotal.io

Description:
Risk Level: Low
Testing: Local on Windows
Docs Changes: n/a
Release Notes: n/a

…uild

Signed-off-by: William A Rowe Jr <wrowe@pivotal.io>
Signed-off-by: Yechiel Kalmenson <ykalmenson@pivotal.io>
@wrowe
Copy link
Contributor Author

wrowe commented Oct 30, 2019

@lizan per discussion elsewhere, could you look at this?

@lizan
Copy link
Member

lizan commented Oct 30, 2019

Can you add a line bazel --bazelrc=windows\.bazelrc build @boringssl//:ssl to ci/ci/windows_ci_steps.ps1? So the build is testing in CI to reduce your moving target.

This tests compilation of several components in the Windows CI
which were impacted by the recent foreign_cc cmake build changes
and proposed fixes to boringssl's internal bazel build schema.

Signed-off-by: William A Rowe Jr <wrowe@pivotal.io>
Signed-off-by: Yechiel Kalmenson <ykalmenson@pivotal.io>

Signed-off-by: William A Rowe Jr <wrowe@pivotal.io>
@wrowe
Copy link
Contributor Author

wrowe commented Oct 31, 2019

That's very sensible. We've added other foreign_cc components which are building correctly, and can begin adding other bazel-based dependencies as well as we've reviewed them. This resolves all the issues of accepting this patch?

Signed-off-by:    William A Rowe Jr <wrowe@pivotal.io>
Signed-off-by: Yechiel Kalmenson <ykalmenson@pivotal.io>

Signed-off-by: William A Rowe Jr <wrowe@pivotal.io>
@wrowe
Copy link
Contributor Author

wrowe commented Nov 1, 2019

We've reconfirmed this subset builds correctly on Windows, patched against master. We are likely missing one or more of the msys2 (bash), python, jdk8 or even perhaps go language tooling?

Signed-off-by: Yechiel Kalmenson <ykalmenson@pivotal.io>
Signed-off-by: William A Rowe Jr <wrowe@pivotal.io>
@stale
Copy link

stale bot commented Nov 14, 2019

This pull request has been automatically marked as stale because it has not had activity in the last 7 days. It will be closed in 7 days if no further activity occurs. Please feel free to give a status update now, ping for review, or re-open when it's ready. Thank you for your contributions!

@stale stale bot added the stale stalebot believes this issue/PR has not been touched recently label Nov 14, 2019
@stale
Copy link

stale bot commented Nov 21, 2019

This pull request has been automatically closed because it has not had activity in the last 14 days. Please feel free to give a status update now, ping for review, or re-open when it's ready. Thank you for your contributions!

@wrowe
Copy link
Contributor Author

wrowe commented Jan 6, 2020

/wait

I don't believe we want this patch as-is because on OS/X I understand we are building all objects dynamic.

@wrowe
Copy link
Contributor Author

wrowe commented Jan 31, 2020

Since this was 3 mos stale, I've rebased and force-pushed.

@lizan could you have a look? I guess I misunderstood that everywhere we consuming boringssl we expect it to be simply a static lib dependency.

@lizan
Copy link
Member

lizan commented Jan 31, 2020

@wrowe can you open a new PR based on this? Since you force-pushed GH doesn't allow me to re-open.

@wrowe
Copy link
Contributor Author

wrowe commented Jan 31, 2020

Good to learn that, thanks. Will do so over the weekend.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
stale stalebot believes this issue/PR has not been touched recently waiting
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants