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

Fix phasar research tool to enable container build #664

Open
wants to merge 8 commits into
base: vara-dev
Choose a base branch
from

Conversation

boehmseb
Copy link
Member

This PR enables Phasar to be built in a container.

@MMory @pdschubert: There are two things that should be changed in phasar's install-llvm.sh script for this to work properly:

  • the script currently uses sudo for installation, this is not necessary when installing to a user-writeable directory (as we use here)
  • Martin suggested using source tarballs instead of cloning llvm to speed up the process, especially since the build is done in a temporary directory

@codecov
Copy link

codecov bot commented Jul 25, 2022

Codecov Report

Patch coverage: 50.00% and project coverage change: -0.01% ⚠️

Comparison is base (8c877d2) 68.20% compared to head (4c2a268) 68.20%.

Additional details and impacted files
@@             Coverage Diff              @@
##           vara-dev     #664      +/-   ##
============================================
- Coverage     68.20%   68.20%   -0.01%     
============================================
  Files           333      333              
  Lines         25680    25682       +2     
============================================
+ Hits          17516    17517       +1     
- Misses         8164     8165       +1     
Files Changed Coverage Δ
varats/varats/tools/driver_build_setup.py 52.72% <ø> (ø)
varats/varats/tools/research_tools/phasar.py 40.19% <50.00%> (+0.19%) ⬆️

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@vulder
Copy link
Contributor

vulder commented Jul 27, 2022

Should we merge this after the phasar changes were worked in?

@vulder
Copy link
Contributor

vulder commented Mar 3, 2023

@fabianbs96 @boehmseb What's the current state of the phasar changes? Can we start to integrate this branch?

@boehmseb
Copy link
Member Author

boehmseb commented Mar 3, 2023

The only remaining issue is to decide how to handle the llvm install.
Phasar currently needs to be built against llvm 14.

The options are:

  • make the user responsible for having the correct llvm version installed and only install llvm 14 by ourself when using containers
  • always install llvm 14 by ourself
  • make the llvm install configurable via CLI (probably not a good idea because this would require reserach-tool-specific command line options)

@vulder
Copy link
Contributor

vulder commented Mar 9, 2023

If we can easily switch to 14 it's probably the simplest thing to do, especially as our LLVM state is also 14.

We use llvm-14 in the container images anyway and for non-container builds the user should be responsible for supplying the correct llvm version.
@vulder
Copy link
Contributor

vulder commented Sep 2, 2023

@fabianbs96 @boehmseb What's the current state of this PR? We should resolve the llvm-14 issue to merge this one otherwise the branch will drift more and and more.

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.

2 participants