Skip to content

Make the platform expansions MSAN aware#271

Merged
giordano merged 3 commits intoJuliaPackaging:masterfrom
gbaraldi:expand-msan-abi
Sep 25, 2022
Merged

Make the platform expansions MSAN aware#271
giordano merged 3 commits intoJuliaPackaging:masterfrom
gbaraldi:expand-msan-abi

Conversation

@gbaraldi
Copy link
Contributor

How about this, so then we don't get useless builds, but still have the tags when needed

Comment on lines +796 to +800
if sanitize(platform) == "memory"
p = deepcopy(platform)
p["cxxstring_abi"] = "cxx11" #Clang only seems to generate cxx11 abi
return p
end
Copy link
Member

Choose a reason for hiding this comment

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

Add tests

Comment on lines +752 to +754
if sanitize(platform) == "memory"
return platform #MSAN can't use libgfortran
end
Copy link
Member

Choose a reason for hiding this comment

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

And for this as well.

src/Rootfs.jl Outdated
end

if sanitize(platform) == "memory"
return platform #MSAN can't use libgfortran
Copy link
Member

Choose a reason for hiding this comment

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

I'd say "doesn't use", rather than "can't use", it just uses a different fortran runtime library

Copy link
Member

@giordano giordano left a comment

Choose a reason for hiding this comment

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

Good that I asked to add tests 🙂

src/Rootfs.jl Outdated
end

if sanitize(platform) == "memory"
return platform #MSAN doesn't use libgfortran, it uses libflang
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
return platform #MSAN doesn't use libgfortran, it uses libflang
return [platform] # MSAN doesn't use libgfortran, it uses libflang

Also, I'm not sure "libflang" is a thing. They seem to just call it flang runtime: https://github.com/llvm/llvm-project/blob/2b187effbd65ec795689f93e78c42fc9ea34b706/flang/runtime/CMakeLists.txt#L12

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's the actual name funilly enough https://github.com/JuliaPackaging/Yggdrasil/blob/master/F/FlangClassic/FlangClassic_RTLib/build_tarballs.jl.
At least for FlangClassic which is what we use.

Copy link
Member

Choose a reason for hiding this comment

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

Right, I was looking at the wrong flang.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In fact the OpenBlas recipe is wrong right now, it's not just a build dep but a runtime one too

src/Rootfs.jl Outdated
if sanitize(platform) == "memory"
p = deepcopy(platform)
p["cxxstring_abi"] = "cxx11" #Clang only seems to generate cxx11 abi
return p
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
return p
return [p]

@giordano giordano merged commit 0fcd680 into JuliaPackaging:master Sep 25, 2022
@gbaraldi
Copy link
Contributor Author

This made it type unstable for some reason, which led to the issue above.

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