[flang-commits] [PATCH] D124759: [flang] Install Fortran_main library

[Diana Picus via Phabricator via flang-commits]

rovka added a comment.

In D124759#3508653 <https://reviews.llvm.org/D124759#3508653>, @awarzynski wrote:

> Hi @rovka & @dpalermo, thanks for working on this!
>
> I am in favor of this change, but would much prefer the original approach from this patch, which is in line with how e.g. LLVM <https://github.com/llvm/llvm-project/blob/be656df18721dc55a1de2eea64a3f73b6afa29a2/llvm/cmake/modules/AddLLVM.cmake#L801-L858> or Clang <https://github.com/llvm/llvm-project/blob/be656df18721dc55a1de2eea64a3f73b6afa29a2/clang/cmake/modules/AddClang.cmake#L49-L143> implement `add_llvm_library` and `add_clang_library`, respectively.
>
> I think that it would be good for the overall health of project to keep CMake modules semi-consistent across sub-projects. If we do diverge, that I'd like to make sure that that's unavoidable :) In particular, the error message from one of the earlier comments suggests //build issue// rather than an installation issue. I would hope that that can be solved //without// creating a dedicated macro for //installing// a library.

Well, the main problem there is that it was trying to build it as a shared library instead of a static one, although I had passed STATIC to add_flang_library. To be honest I didn't investigate why it still thought it should build it as shared, because @dpalermo 's patch works fine and imo the Fortran_main lib is sufficiently different from all the others that we can afford to take a different path with it. Note that the other libs would still use add_flang_library with either of the 2 patches, so we wouldn't be diverging that much.

> I'm traveling today, and for testing Flang I need stable SSH. Once I have access to a better connection, I can poke around - that's in the next few days. Would you mind waiting or, alternatively, having another go yourself ?

Unfortunately I don't have time to look into it more this week either. I'm not going to commit it yet, and we can revisit next week.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D124759/new/

https://reviews.llvm.org/D124759