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

Fix feature check macros for POSIX realpath #4302

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

Conversation

beutlich
Copy link
Member

@beutlich beutlich commented Feb 8, 2024

Resolves #4301.

This is PR for master that hopefully can get back-ported to main/4.1.0 after merge.

@beutlich beutlich added the L: C-Sources Issue addresses Modelica/Resources/C-Sources label Feb 8, 2024
Copy link
Contributor

@HansOlsson HansOlsson left a comment

Choose a reason for hiding this comment

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

Ok. Seems needed as it seems common to recommend -D_BSD_SOURCE

@beutlich beutlich enabled auto-merge (squash) February 9, 2024 12:51
@TeoGoddet
Copy link

@beutlich on my system and with the lib creating the problem (open62541), switching to defined(_BSD_SOURCE) did not fix the problem.
The problem was then that realpath was not availlable, which may be not normal and due to another error in another lib

@beutlich
Copy link
Member Author

@TeoGoddet Can you please find out why realpath is not available when you expected it to be available?

@TeoGoddet
Copy link

@beutlich I don't think that realpath should be available (dymola windows with visual studio 22 compiler)

I will try to understand if Open62541 is doing wrong or not with the BSD_SOURCE

https://github.com/open62541/open62541/blob/3f381ea37a8e2c0e352a0e05f49859627f753ac1/tools/ua-tool/ua.c#L18

@beutlich
Copy link
Member Author

@TeoGoddet OK, waiting for your feedback then.

@beutlich
Copy link
Member Author

beutlich commented Feb 13, 2024

@TeoGoddet I also do not understand why would you want to build ua.c (executable) together with ModelicaInternal.c. 😕

@TeoGoddet
Copy link

@beutlich sorry I gave the wrong file, the problematic one is : https://github.com/open62541/open62541/blob/88dbcb6d644aaf3c38d2a1af9fc9cc2755c24c42/arch/win32/ua_architecture.h#L133
They may be wrong defining _BSD_SOURCE without defining all the expected function, what do you think ?

@beutlich
Copy link
Member Author

what do you think ?

That header file is 5 years old and not part of current open62541 code. Still being confused.

@TeoGoddet
Copy link

TeoGoddet commented Feb 13, 2024

@beutlich It end up in the single file version of the lib for some reason (found here https://github.com/open62541/open62541/releases/tag/v1.3.6)
I tested removing it and it seems that the lib still work
I create an issue on their side about this

On this side I think that this PR is needed to ensure proper handling of -D_BSD_SOURCE that is quite common

@beutlich
Copy link
Member Author

@TeoGoddet Could you please share a minimal Modelica model and workflow (in a separate repo) that demonstrates your observations? Thanks!

@beutlich beutlich requested review from sjoelund and removed request for sjoelund March 9, 2024 16:29
@beutlich beutlich requested a review from adrpo March 13, 2024 06:37
@beutlich beutlich force-pushed the fix-realpath-feature-check branch 2 times, most recently from 28848d7 to 19dc0db Compare March 29, 2024 13:09
@beutlich beutlich enabled auto-merge (squash) May 21, 2024 16:27
@beutlich beutlich force-pushed the fix-realpath-feature-check branch from 19dc0db to 7620d2f Compare May 21, 2024 16:27
@casella
Copy link
Contributor

casella commented Jul 31, 2024

@sjoelund, @adrpo, @AnHeuermann can you please review this and/or comment? I think this is OK but I'm not much into this kind of low-level C programming.

@AnHeuermann
Copy link
Contributor

Sorry, I don't know how this POSIX stuff is supposed to work, so I can't give a review.

@casella
Copy link
Contributor

casella commented Aug 19, 2024

Maybe @fedetft could help with that? He for sure knows a thing or two about POSIX 😄

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
L: C-Sources Issue addresses Modelica/Resources/C-Sources
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Cannot compile library due to _BSD_SOURCE wrong usage
5 participants