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 toml11 #10330

Closed
wants to merge 3 commits into from
Closed

Update toml11 #10330

wants to merge 3 commits into from

Conversation

p01arst0rm
Copy link
Contributor

Motivation

Changes to the source structure are required in preparation for accepting the meson buildsystem. including thirdparty files inside the nix source is not recommended, as it impacts portability and complicates building, and introduces unknown/undeclared dependencies which may cause conflicts on user's systems. these issues are fixed by separating out thirdparty code and properly declaring it, as in #3160.

Context

  • The Toml11 source has been moved to /subprojects/toml11
  • The Toml11 source has been updated to offficial version 3.8.1
  • The Toml11 source has been patched to fix building.

@p01arst0rm
Copy link
Contributor Author

related:

#3160
NixOS/rfcs#132

@edolstra
Copy link
Member

Can't we just remove the vendored toml11 and pull it in as an external dependency?

@p01arst0rm
Copy link
Contributor Author

p01arst0rm commented Mar 26, 2024

Can't we just remove the vendored toml11 and pull it in as an external dependency?

yes, this is fixed in the meson build. for now, the files only have to be moved. assuming that the meson build is committed, there is no need to fix this issue twice in the old and new buildsystem.

@Ericson2314
Copy link
Member

Ericson2314 commented Mar 27, 2024

Yeah @p01arst0rm we really don't want to vendor large libraries like this. Developers can manually unpack them as a sub-project if they like, but they should not be committed in place.

@p01arst0rm
Copy link
Contributor Author

while i agree moving away from vendoring the files is ideal, and is exactly what the goal is, there are a lot of other annoying issues with allowing either nix or meson to provide toml11. toml11 is already currently vendored, so this PR changes nothing other than the file locations and versions to allow the meson build to use them.

If youre absolutely sure that these files are not movable and should be deleted, an issue should be opened and the nix flake updated.

@p01arst0rm
Copy link
Contributor Author

any update on merging?

@cole-h cole-h mentioned this pull request Jun 27, 2024
@Ericson2314
Copy link
Member

Closing in favor of #10972: un-vendoring is better than updating a vendor. :)

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.

3 participants