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

Overload re2 cmakelists to fix cmake/threads detection issue on conda-forge mac builds, and pin C++ dependency versions #1634

Merged
merged 8 commits into from
Dec 2, 2021

Conversation

timkpaine
Copy link
Member

No description provided.

@timkpaine timkpaine added dependencies internal Internal refactoring and code quality improvement and removed cla-present labels Nov 28, 2021
@texodus
Copy link
Member

texodus commented Nov 29, 2021

Is this a fix for the CI failure in #1632?

@timkpaine
Copy link
Member Author

Tangentially related, i think re2's Cmakelists is incompatible with certain versions of mac os SDK (leading to the failures we see over on conda) so this applies the threading fix into it directly. #1632 would let you rely on a preinstalled version of re2 (and/or exprtk, and i'll have it do rapidjson and all our others) which will let us abide more by the rules of ecosystems like conda (and by extension, athena) by optionally not vendoring our dependencies at build time (but by still allowing that to keep our lives sane for our nice static builds for JS).

@timkpaine timkpaine changed the title Overload re2 cmakelists Overload re2 cmakelists to fix cmake/threads detection issue on conda-forge mac builds Nov 29, 2021
@timkpaine timkpaine changed the title Overload re2 cmakelists to fix cmake/threads detection issue on conda-forge mac builds Overload re2 cmakelists to fix cmake/threads detection issue on conda-forge mac builds, and pin C++ dependency versions Nov 29, 2021
@timkpaine
Copy link
Member Author

had to blow away yarn cache, full build in: https://dev.azure.com/finosfoundation/perspective/_build/results?buildId=3647&view=results

@timkpaine
Copy link
Member Author

@texodus i think windows yarn cache is broken, did you run into this before? on another branch i turned it off, if thats acceptable i can do it here

@texodus
Copy link
Member

texodus commented Nov 30, 2021

Yes I have encountered this before and Azure has less-than-helpful advice for resolving it (e.g. rename the cache ona new branch). I am skeptical that perspective w/esbuild even benefits from this in terms of build time, so if its a blocker I think it is safe to remove.

Copy link
Member

@texodus texodus left a comment

Choose a reason for hiding this comment

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

Thanks for the PR! Looks good!

I am going to remove yarn caching entirely from our Azure script.

@texodus texodus merged commit 1fe8a58 into finos:master Dec 2, 2021
@timkpaine timkpaine deleted the tkp/re2threadsfix branch April 4, 2022 18:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
internal Internal refactoring and code quality improvement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants