Skip to content

Conversation

@drexin
Copy link
Contributor

@drexin drexin commented Mar 28, 2020

3.15.1 contains a bug that causes libraries that use the CMake Swift module to be built with -Onone, even in release mode.

@drexin drexin requested review from compnerd and shahmishal March 28, 2020 02:49
@drexin
Copy link
Contributor Author

drexin commented Mar 28, 2020

@swift-ci smoke test

Copy link
Member

@compnerd compnerd left a comment

Choose a reason for hiding this comment

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

LGTM, assuming @shahmishal doesn't have any concerns.

@shahmishal
Copy link
Member

This will require additional changes to CI before this can be merged, please hold off from merging it.

@shahmishal
Copy link
Member

20:00:39 -- Could NOT find OpenSSL, try to set the path to OpenSSL root folder in the system variable OPENSSL_ROOT_DIR (missing: OPENSSL_CRYPTO_LIBRARY OPENSSL_INCLUDE_DIR) 
20:00:39 CMake Error at Utilities/cmcurl/CMakeLists.txt:454 (message):
20:00:39   Could not find OpenSSL.  Install an OpenSSL development package or
20:00:39   configure CMake with -DCMAKE_USE_OPENSSL=OFF to build without OpenSSL.

@swift-ci clean smoke test Linux

@drexin
Copy link
Contributor Author

drexin commented Mar 28, 2020

@swift-ci smoke test

1 similar comment
@drexin
Copy link
Contributor Author

drexin commented Mar 28, 2020

@swift-ci smoke test

@drexin
Copy link
Contributor Author

drexin commented Mar 28, 2020

@swift-ci smoke test

1 similar comment
@drexin
Copy link
Contributor Author

drexin commented Mar 28, 2020

@swift-ci smoke test

@drexin
Copy link
Contributor Author

drexin commented Mar 28, 2020

@swift-ci smoke test windows

1 similar comment
@drexin
Copy link
Contributor Author

drexin commented Mar 28, 2020

@swift-ci smoke test windows

@drexin
Copy link
Contributor Author

drexin commented Mar 28, 2020

@swift-ci smoke test

2 similar comments
@drexin
Copy link
Contributor Author

drexin commented Mar 28, 2020

@swift-ci smoke test

@drexin
Copy link
Contributor Author

drexin commented Mar 28, 2020

@swift-ci smoke test

@drexin
Copy link
Contributor Author

drexin commented Mar 28, 2020

@swift-ci smoke test linux

@drexin
Copy link
Contributor Author

drexin commented Mar 28, 2020

@swift-ci smoke test windows

@RLovelett
Copy link
Contributor

Two questions:

  1. Should the README also be updated?
  2. Should the CMakeLists.txt be updated?

@compnerd
Copy link
Member

@RLovelett - updating the README is probably a good idea. The CMakeLists.txt update - in its current state any version is less than useful as it is an implementation of CMake in CMake. The work to clean up the build system is still incomplete, but help is welcome!

@drexin
Copy link
Contributor Author

drexin commented Apr 1, 2020

@swift-ci smoke test

@drexin
Copy link
Contributor Author

drexin commented Apr 1, 2020

@compnerd @shahmishal Bumped version in README and CMakeLists.txt. I think it's better to at least fail when it doesn't have the correct version. WDYT? I also changed the version for swift-5.2-branch back to 3.15.1 in update-checkout-config.json, since we decided not to upgrade there.

@drexin
Copy link
Contributor Author

drexin commented Apr 1, 2020

@swift-ci clean smoke test

@shahmishal
Copy link
Member

shahmishal commented Apr 1, 2020

I don't think we should bump the CMakeLists.txt, README is fine. We don't want to force everyone to update to CMake 3.16.5 to build the Swift compiler.

@drexin
Copy link
Contributor Author

drexin commented Apr 1, 2020

Ok, makes sense. I'll revert that.

@drexin
Copy link
Contributor Author

drexin commented Apr 1, 2020

@swift-ci clean smoke test

3.15.1 contains a bug that causes libraries that use the CMake Swift module to be built with -Onone, even in release mode.
@drexin
Copy link
Contributor Author

drexin commented Apr 1, 2020

@swift-ci clean smoke test

@drexin
Copy link
Contributor Author

drexin commented Apr 1, 2020

@swift-ci smoke test windows

@compnerd
Copy link
Member

compnerd commented Apr 1, 2020

Sure, bumping both is fine, I just meant that it doesn’t really convey much at the moment.

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.

4 participants