Skip to content

Reduce size of llvm cache by not build webassembly target for llvm<19 #357

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

Merged
merged 1 commit into from
Nov 29, 2024

Conversation

mcbarton
Copy link
Collaborator

@mcbarton mcbarton commented Nov 28, 2024

@vgvassilev Due to the work to get the wasm version of xeus-cpp working we will have a wasm version of CppInterOp we will be able to test, and keep on top of from version 19. Although it will build, I suspect our wasm version of CppInterOp for llvm<19 won't work. Therefore I suggest we don't build the web assembly target for llvm<19, and remove the wasm jobs for llvm<19 so that ci runs are quicker. I've cleared the cache for 2 jobs before putting the PR in so that we can test if my changes work. If you're happy with the changes in this PR the cache will need to be cleared before merging. With the space saved in the cache by this change should give us the room in the cache for when llvm 20 is released.

@mcbarton mcbarton requested a review from vgvassilev November 28, 2024 20:31
Copy link
Contributor

@vgvassilev vgvassilev left a comment

Choose a reason for hiding this comment

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

LGTM!

Copy link

codecov bot commented Nov 28, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 74.55%. Comparing base (5ac8e62) to head (75f8591).
Report is 1 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##             main     #357   +/-   ##
=======================================
  Coverage   74.55%   74.55%           
=======================================
  Files           8        8           
  Lines        3211     3211           
=======================================
  Hits         2394     2394           
  Misses        817      817           

@mcbarton mcbarton force-pushed the remove-targets-to-build-llvm branch from 28ac60e to 75f8591 Compare November 28, 2024 23:04
@mcbarton mcbarton merged commit f794dee into main Nov 29, 2024
42 checks passed
@mcbarton mcbarton deleted the remove-targets-to-build-llvm branch November 29, 2024 07:22
@anutosh491
Copy link
Collaborator

anutosh491 commented Nov 29, 2024

Due to the work to get the wasm version of xeus-cpp working we will have a wasm version of CppInterOp we will be able to test, and keep on top of from version 19. Although it will build, I suspect our wasm version of CppInterOp for llvm<19 won't work. Therefore I suggest we don't build the web assembly target for llvm<19, and remove the wasm jobs for llvm<19 so that ci runs are quicker.

Hey @mcbarton
I think I know on how to get a proper wasm build for CppInterOp that can be put to use for downstream projects like xeus-cpp (have been playing around locally) . It should also rid of the numerous undefined symbols out of clang as raised here (#334) .

For that I would possibly like

  1. llvm 19.1.5 to be out (and the PR getting clang-repl to work in the browser to be a part of it [clang-repl] Fix generation of wasm binaries while running clang-repl in browser llvm/llvm-project#117978)
  2. CppInterOp to be based on this llvm version.
  3. Then I could raise a PR introducing a useable wasm build.

Maybe we could raise an issue to "fix the wasm build" (and possibly assign it to me) till then ?

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.

3 participants