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

misc: Use external freestanding headers implementation #859

Closed
wants to merge 1 commit into from

Conversation

mintsuki
Copy link
Contributor

No description provided.

@Geertiebear
Copy link
Member

What does this PR fix?

@mintsuki
Copy link
Contributor Author

It is not a fix for anything. It makes mlibc rely on an external, more complete implementation of the freestanding C headers instead of its internal ones which were rather incomplete.

@Geertiebear
Copy link
Member

How incomplete are they?

@mintsuki
Copy link
Contributor Author

Refer to limits.h for example. In general they are incomplete enough that relying on an external implementation instead of NIH'ing them makes some sense, but it is subject to personal preference.

@mintsuki mintsuki closed this Apr 22, 2023
@mintsuki mintsuki deleted the external-fstd-hdrs branch April 22, 2023 09:46
@mintsuki mintsuki restored the external-fstd-hdrs branch April 24, 2023 21:03
@mintsuki mintsuki reopened this Apr 24, 2023
@ikbenlike
Copy link
Contributor

as I mentioned before on discord, I approve of this idea but I do not have the means to verify the correctness of this particular implementation. I would say that the rationale is similar as with the linux-headers situation, in that there is no need to maintain a set of headers like this ourselves

@Dennisbonke
Copy link
Member

Not against this. However, this would break Managarm if I'm not mistaken, can you prepare a PR for bootstrap-managarm too?

@mintsuki
Copy link
Contributor Author

Not against this. However, this would break Managarm if I'm not mistaken, can you prepare a PR for bootstrap-managarm too?

@Dennisbonke see bootstrap-managarm#232

@Geertiebear
Copy link
Member

Refer to limits.h for example. In general they are incomplete enough that relying on an external implementation instead of NIH'ing them makes some sense, but it is subject to personal preference.

I'd prefer then that we just embed the external headers into mlibc then. Are they really so complex and change so often that we should rely on an external repository? Seems better to me to just add the missing parts to mlibc.

@mintsuki
Copy link
Contributor Author

You make a compelling point, honestly. You can freely copy my freestanding headers licensed under the 0BSD license into mlibc if you want.

The idea of this PR is more about making it work similarily to linux-headers and ensuring that changes and new addition to the freestanding headers are "automatically" reflected.

The freestanding headers are usually modified or changed to support new language standards and features, but you're right in that this is not something that happens all the time and it can be easily backported to the mlibc tree itself.

@Geertiebear
Copy link
Member

Sorry it's taken me so long to get back to this. I agree that it would be useful to have the freestanding headers automatically updated. Currently mlibc doesn't have a unified way of dealing with external packages (tracked in issue #601). I propose that until we have that issue in order this PR instead copies the new freestanding headers into the current tree, and we make a new issue to make sure we don't forget that we need to keep it in sync. Then once we figured out a good way to keep third party code in sync we can change it over to the external repo.

Reason I'm saying this is because as it stands this PR would add some annoyance to the developer workflow, which I'd rather avoid for now.

@mintsuki mintsuki closed this Nov 29, 2023
@mintsuki mintsuki deleted the external-fstd-hdrs branch November 29, 2023 17:00
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.

None yet

4 participants