limine-bootloader / limine-zig-template

A simple template for building a Limine-compliant kernel in Zig.
BSD Zero Clause License
28 stars 4 forks source link

Make use of Zig's package manager #1

Closed AnErrupTion closed 10 months ago

AnErrupTion commented 10 months ago

Requires https://github.com/limine-bootloader/limine-zig/pull/2.

48cf commented 10 months ago

Should we still clone https://github.com/limine-bootloader/limine-zig in kernel/GNUmakefile? Or is that taken care of by Zig's build system? I'm not sure how the new dependency system works yet.

AnErrupTion commented 10 months ago

Should we still clone https://github.com/limine-bootloader/limine-zig in kernel/GNUmakefile? Or is that taken care of by Zig's build system? I'm not sure how the new dependency system works yet.

Ah, that's automatically taken care of, I'll remove it.

48cf commented 10 months ago

I just tried this out locally - I don't like the fact that the package management depends on some hash. I'm not a fan of updating this repo every time we update https://github.com/limine-bootloader/limine-zig, but I'm unsure what the right solution is.

48cf commented 10 months ago

I think we should either start tagging https://github.com/limine-bootloader/limine-zig so we can have stable reference points for the package manager or use a commit hash and update build.zig.zon every time we make a major change.

AnErrupTion commented 10 months ago

I just tried this out locally - I don't like the fact that the package management depends on some hash. I'm not a fan of updating this repo every time we update https://github.com/limine-bootloader/limine-zig, but I'm unsure what the right solution is.

I don't think we should update everytime- I set it up to download trunk, so if everything goes to plan, it should fail on the hash check if a newer archive is detected.

AnErrupTion commented 10 months ago

I think we should either start tagging https://github.com/limine-bootloader/limine-zig so we can have stable reference points for the package manager or use a commit hash and update build.zig.zon every time we make a major change.

I'd use a commit hash but I can't predict what the commit hash is going to be before the PR is merged 😅 (unless you first merge that PR and I update this one)

But maybe tagging limine-zig is a good idea as well.

48cf commented 10 months ago

I set it up to download trunk, so if everything goes to plan, it should fail on the hash check if a newer archive is detected.

That's what I'm talking about, every time we update https://github.com/limine-bootloader/limine-zig we'll need to update the hash here also to ensure the build doesn't fail.

I'd use a commit hash but I can't predict what the commit hash is going to be when the PR is merged

It's a merge, so I believe the commit hash will be exactly the same as it is on your branch. Tagging is not a bad idea but I've never done it before so I'll have to read up on that 😅

AnErrupTion commented 10 months ago

That's what I'm talking about, every time we update https://github.com/limine-bootloader/limine-zig we'll need to update the hash here also to ensure the build doesn't fail.

Yeah, but my point is that the user should do it, it's not like it's very complicated.

It's a merge, so I believe the commit hash will be exactly the same as it is on your branch.

Hmm, I checked on a project I regularly contribute to and the commit hash on the PR vs the merged commit isn't the same...

48cf commented 10 months ago

We should use a commit hash for the Zig dependency - you can grab it after I merge limine-bootloader/limine-zig#2 😉 I think the user shouldn't have to update the hash manually to build the template.

AnErrupTion commented 10 months ago

Done!

I think the user shouldn't have to update the hash manually to build the template.

Yeah, but I don't think you can have the hash automatically.

48cf commented 10 months ago

I think it's enough if we update this when updating https://github.com/limine-bootloader/limine-zig. After all, it's not updated very frequently. Let's get this merged :^)

48cf commented 10 months ago

Thanks for both PRs! 😄