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

port to zig package manager #50

Merged
merged 9 commits into from
Jun 10, 2024
Merged

port to zig package manager #50

merged 9 commits into from
Jun 10, 2024

Conversation

lun-4
Copy link
Contributor

@lun-4 lun-4 commented Jun 8, 2024

following my work on kivikakk/libpcre.zig#18, I attempted to port koino to the package manager for my personal project, https://github.com/lun-4/obsidian2web/tree/zig012

this would be a large change to how koino is distributed as there wouldn't be a need for vendoring the dependencies anymore. dependencies would need respective PRs to port them (which this PR is dependent upon). because of all of this, the PR is unfinished (README was not updated, build.zig.zon refers to my forks of the libraries, etc), and I would be asking for your opinion on such (including dismissing the port altogether)

this also adds a fix to the parser because @memcpy will safety check on the length of the slices and safety-panic if the lengths differ (I looked at the two other @memcpy users and they seem fine), since the parser explicitly wants to add a newline to the end of the input, it would always panic (bug found by doing this porting work. if the zigpkg port is dismissed, I'll submit another PR to fix that specifically lol)

dependency PRs (should be merged first):

TODO (if this is accepted):

  • update README
  • remove zig dependencies from vendor directory, still keep cmark-gfm for make spec functionality

@kivikakk
Copy link
Owner

kivikakk commented Jun 9, 2024

this would be a large change to how koino is distributed as there wouldn't be a need for vendoring the dependencies anymore. dependencies would need respective PRs to port them (which this PR is dependent upon). because of all of this, the PR is unfinished (README was not updated, build.zig.zon refers to my forks of the libraries, etc), and I would be asking for your opinion on such (including dismissing the port altogether)

I'd consider this a strict improvement, absolutely! Am very happy and grateful for all these PRs. I haven't been keeping up with the Zig ecosystem for a while until just recently, so this's been super helpful to me personally, too, in seeing how the package manager hangs together.

since the parser explicitly wants to add a newline to the end of the input, it would always panic

Oh lol, thank you. The recent updates to use @memcpy were in the beginning pretty widely broken; thanks for catching one more!

@lun-4
Copy link
Contributor Author

lun-4 commented Jun 9, 2024

should be ready to merge now then! many thanks

@kivikakk kivikakk enabled auto-merge June 10, 2024 09:18
@kivikakk kivikakk merged commit 605f22c into kivikakk:main Jun 10, 2024
4 checks passed
@kivikakk
Copy link
Owner

Yay! Thanks so much!

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.

2 participants