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.

Sign up for GitHub

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

Jump to bottom

fix: use current binary directory mariadb shared object and dont override env variable #1669

Merged
merged 2 commits into from
Dec 8, 2024
Merged

fix: use current binary directory mariadb shared object and dont override env variable #1669

merged 2 commits into from
Dec 8, 2024

Conversation

EmosewaMC
Copy link
Collaborator

we were overwriting the users environment variable with the build directory always, resulting in inis always being placed in the incorrect spot if a preset was specified, or if the user had an environment variable where they stored the inis.

Also, assuming it passes ci, made it so we use the shared library in the binary directory as opposed to the one

mac please

@EmosewaMC EmosewaMC changed the title fix: grab correct mariadb file and dont override env variable fix: use current binary directory mariadb shared object and dont override env variable Dec 7, 2024
@EmosewaMC
mac stuff
1d1c429 
grab correct mariadb file and dont override env variable

fix for unix

Update CMakeLists.txt

unix only
Xiphoseer
Xiphoseer approved these changes Dec 8, 2024
View reviewed changes
Copy link
Contributor

@Xiphoseer Xiphoseer left a comment

Choose a reason for hiding this comment

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

Thank you!

@aronwk-aaron aronwk-aaron merged commit 8b56b0b into main Dec 8, 2024
4 checks passed
@jadebenn jadebenn deleted the ci-fixes branch December 8, 2024 19:52
jadebenn added a commit that referenced this pull request Dec 8, 2024
@jadebenn
fix: Make multi-config presets work again
e9c4bbe 
Reverted breaking change for multiple-configuration presets that was previously included in #1669
@jadebenn jadebenn mentioned this pull request Dec 8, 2024
Closed
jadebenn
jadebenn reviewed Dec 8, 2024
View reviewed changes
Copy link
Collaborator

@jadebenn jadebenn left a comment •
edited
Loading

Choose a reason for hiding this comment

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

I can't exactly do a 'changes required' since this has already been merged, but I think we need to revisit this PR.

EDIT: I did not understand the issue as well as I thought I did.

dDatabase/CMakeLists.txt Show resolved Hide resolved
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jadebenn jadebenn jadebenn left review comments

@aronwk-aaron aronwk-aaron aronwk-aaron approved these changes

@Xiphoseer Xiphoseer Xiphoseer approved these changes

@Jettford Jettford Awaiting requested review from Jettford

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@EmosewaMC @jadebenn @Xiphoseer @aronwk-aaron