This is an archive of the discontinued LLVM Phabricator instance.

Differential D61371

[benchmark] Fix win32 link on case-sensitive fs
ClosedPublic

Authored by jschueller on May 1 2019, 3:49 AM.

Details

Reviewers
chandlerc
RKSimon
lebedev.ri
Commits
rG1e4a094b37a1: [benchmark] Fix win32 link on case-sensitive fs
rCXX367356: [benchmark] Fix win32 link on case-sensitive fs
rL367356: [benchmark] Fix win32 link on case-sensitive fs
Summary

This fixes cross-builds with MinGW from case-sensitive file-systems (on Linux)

Diff Detail

Event Timeline

jschueller created this revision.May 1 2019, 3:49 AM
Herald added a reviewer: lebedev.ri. · View Herald TranscriptMay 1 2019, 3:49 AM
Herald added subscribers: llvm-commits, mstorsjo, lebedev.ri and 2 others. · View Herald Transcript
lebedev.ri added a comment.May 1 2019, 3:50 AM

Please can you submit this upstream first?

RKSimon added a comment.May 1 2019, 4:19 AM
In D61371#1485911, @lebedev.ri wrote:

Please can you submit this upstream first?

https://github.com/google/benchmark/blob/master/src/CMakeLists.txt

Yes, please create a pull request on google-benchmark's github first

lebedev.ri retitled this revision from Fix win32 link on case-sensitive fs to [benchmark] Fix win32 link on case-sensitive fs.May 1 2019, 12:46 PM
jschueller added a comment.May 1 2019, 11:58 PM

ok, see https://github.com/google/benchmark/pull/812

lebedev.ri added a comment.May 2 2019, 12:35 AM
  1. Please note there is one more copy of benchmark, in testsuite repo. Not sure if you care about that one, but it may be better to fix this in all of them at once.
  2. There is a llvm/utils/benchmark/README.LLVM and libcxx/utils/google-benchmark/README.LLVM (and in testsuite, if you change that one too) that needs to be updated to document this cherry-pick from upstream.
RKSimon added a comment.Jun 26 2019, 11:38 AM
In D61371#1487338, @jschueller wrote:

ok, see https://github.com/google/benchmark/pull/812

Any progress?

lebedev.ri added a comment.Jun 26 2019, 11:56 AM
In D61371#1559587, @RKSimon wrote:
In D61371#1487338, @jschueller wrote:

ok, see https://github.com/google/benchmark/pull/812

Any progress?

@jschueller never replied there, so it got stuck.
I could take that over, but i'd need an *explicit* acknowledgement that it is ok to steal that from @jschueller from @jschueller.

jschueller added a comment.Jul 15 2019, 12:17 PM

ok, you can "steal" my patch : )

lebedev.ri requested changes to this revision.Jul 22 2019, 6:15 AM

Submitted
https://github.com/google/benchmark/pull/840
yesterday, it just landed.
Please update README.llvm in these bundled copies to reflect this cherry-pick.
(BTW, there is one more copy, in test-suite, not sure if that matters)

This revision now requires changes to proceed.Jul 22 2019, 6:15 AM
RKSimon added a comment.Jul 22 2019, 6:29 AM

@lebedev.ri I think you can commandeer this

This revision was not accepted when it landed; it landed in state Needs Revision.Jul 30 2019, 1:47 PM
Closed by commit rL367356: [benchmark] Fix win32 link on case-sensitive fs (authored by lebedevri). · Explain Why
This revision was automatically updated to reflect the committed changes.

Revision Contents

PathSize
libcxx/
utils/
google-benchmark/
src/
2 lines
llvm/
utils/
benchmark/
src/
2 lines

Diff 197526

libcxx/utils/google-benchmark/src/CMakeLists.txt

Context not available.
# We need extra libraries on Windows # We need extra libraries on Windows
if(${CMAKE_SYSTEM_NAME} MATCHES "Windows") if(${CMAKE_SYSTEM_NAME} MATCHES "Windows")
target_link_libraries(benchmark Shlwapi) target_link_libraries(benchmark shlwapi)
endif() endif()
# We need extra libraries on Solaris # We need extra libraries on Solaris
Context not available.

llvm/utils/benchmark/src/CMakeLists.txt

Context not available.
# We need extra libraries on Windows # We need extra libraries on Windows
if(${CMAKE_SYSTEM_NAME} MATCHES "Windows") if(${CMAKE_SYSTEM_NAME} MATCHES "Windows")
target_link_libraries(benchmark Shlwapi) target_link_libraries(benchmark shlwapi)
endif() endif()
# We need extra libraries on Solaris # We need extra libraries on Solaris
Context not available.