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

update to 1.0.19, removed headers #27

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

CPunch
Copy link

@CPunch CPunch commented Dec 31, 2023

closes #5 and #26

been using your project for a while now, thank you!

let me know if you need any changes

Copy link
Owner

@robinlinden robinlinden left a comment

Choose a reason for hiding this comment

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

Neat, thank you! And I'm happy you found the project useful! :D

The only real comment is that I'd like for each commit to be in a working state, so maybe commit 1: remove headers, commit 2: update to 1.0.19, but if you'd like I can rebase and restructure the commits for you. :P

Seems like the MSVC CI is also failing due to unresolved symbols. I guess you ran this on Linux? Need any help looking into those failures or is it something you can dig into for a bit?

@CPunch
Copy link
Author

CPunch commented Jan 1, 2024

sure! thats something i can get started on tomorrow, and yeah you called it I wrote it while on linux haha

feel free to restructure it yourself if I don't get to it, thanks!

@CPunch
Copy link
Author

CPunch commented Jan 1, 2024

looks like MSVC will need a bit more work, still looking into it haha

@robinlinden
Copy link
Owner

No rush! Let me know if you hit a wall with it and I can take a look. :P

@CPunch
Copy link
Author

CPunch commented Jan 7, 2024

currently hit a wall haha, some things I've tried doing (although might just be spinning my wheels):

  • verified that the missing symbols are actually being compiled in the translation units
  • double checked sodium isn't supplying any special definitions to MSVC
  • verified it wasn't just some weird side effect of removing the headers for MSVC (commit 65de118 works for MSVC)

let me know if you have any other ideas or if you can check as well, thank you!

@@ -147,12 +137,13 @@ target_compile_definitions(${PROJECT_NAME}
$<$<BOOL:${SODIUM_ENABLE_BLOCKING_RANDOM}>:USE_BLOCKING_RANDOM>
$<$<BOOL:${SODIUM_MINIMAL}>:MINIMAL>
$<$<C_COMPILER_ID:MSVC>:_CRT_SECURE_NO_WARNINGS>
$<$<C_COMPILER_ID:MSVC>:>
Copy link
Author

Choose a reason for hiding this comment

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

this was committed by mistake, i'll be sure to remove this in the final working commit

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.

All of the .h files in the build are can be removed
2 participants