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

Use PIC for (default) static lib #639

Merged
merged 2 commits into from
Mar 18, 2019
Merged

Use PIC for (default) static lib #639

merged 2 commits into from
Mar 18, 2019

Conversation

zbeekman
Copy link
Collaborator

caf wrapper script defaults to static linkage against libcaf_mpi.a but if you're
building a shared lib with the caf script this fails because objects in libcaf_mpi.a
are not compiled with -fPIC. So creating shared libs fails.

The wrapper script should probably default to building and linking against the shared lib

Avg response time coverage on master
Issue Stats Codecov branch

Summary of changes

Always compile position independent code so that the caf wrapper script can build shared libs. Probably, the better solution is to link against the shared lib by default.

Rationale for changes

Allow caf to compile JSON-Fortran

Additional info and certifications

This pull request (PR) is a:

  • Bug fix
  • Feature addition
  • Other, Please describe:

I certify that

  • I reviewed and followed the contributing guidelines, including
    - Increasing test coverage for all feature-addition PRs
    - Increasing test coverage for all bug-fix PRs for which there
    does not already exist a related test that failed before the PR
    - At least maintaining test coverage for all other PRs
    - Ensuring that all tests pass when run locally
    - Naming PR to indicate work in progress (WIP) and to attach the PR
    to the appropriate bug report or feature request issue
    - White space (no trailing white space or white space errors may
    be introduced)
    - Commenting code where it is non-obvious and non-trivial
    - Logically atomic, self consistent and coherent commits
    - Commit message content
    - Waiting 24 hours before self-approving the PR to give another
    OpenCoarrays developer a chance to review my proposed code

 `caf` wrapper script defaults to static linkage against libcaf_mpi.a but if you're
 building a shared lib with the `caf` script this fails because objects in `libcaf_mpi.a`
 are not compiled with `-fPIC`. So creating shared libs fails.

 The wrapper script should probably default to building and linking against the shared lib
@ghost ghost assigned zbeekman Mar 18, 2019
@ghost ghost added the needs-review label Mar 18, 2019
@zbeekman zbeekman requested a review from rouson March 18, 2019 19:23
@zbeekman
Copy link
Collaborator Author

Fixes #635

 The LD_LIB_P_VAR *should* be *once* de-referenced in the echo statements within
 `prerequisits/install-functions/report_results.sh` so that it takes the value of
 `LD_LIBRARY_PATH` or `DYLD_LIBRARY_PATH` in the setup script, which then checks
 if the user has anything set here and decides whether to add additional search
 paths (if they have it set) or set it directly if it is empty.
@rouson
Copy link
Member

rouson commented Mar 18, 2019

LGTM pending CI tests passing.

@codecov
Copy link

codecov bot commented Mar 18, 2019

Codecov Report

Merging #639 into master will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master     #639   +/-   ##
=======================================
  Coverage   55.73%   55.73%           
=======================================
  Files           4        4           
  Lines        3158     3158           
=======================================
  Hits         1760     1760           
  Misses       1398     1398

Copy link
Member

@rouson rouson left a comment

Choose a reason for hiding this comment

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

LGTM pending CI passing

@zbeekman zbeekman merged commit e7a9bdf into master Mar 18, 2019
@ghost ghost removed the needs-review label Mar 18, 2019
@zbeekman zbeekman deleted the PIC-static-lib branch March 20, 2019 19:09
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.

None yet

2 participants