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.

Sign up for GitHub

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

Jump to bottom

feat: add NetBSD as a supported platform, use case for PLATFORM test #434

Merged
merged 2 commits into from
Feb 5, 2025
Merged

feat: add NetBSD as a supported platform, use case for PLATFORM test #434

merged 2 commits into from
Feb 5, 2025

Conversation

iMilnb
Copy link
Contributor

@iMilnb iMilnb commented Jan 28, 2025

I tested dinit on NetBSD as far as replacing /sbin/init with it and it works! FYI I packaged it for pkgsrc in sysutils/dinit

@mobin-2008 mobin-2008 added Enhancement/New Feature Improving things or introduce new feature P-BSD Things related to BSD A-Importance: Normal C-dinit Things about the main parts of dinit C-shutdown Things about shutdown/reboot/halt utilities labels Jan 29, 2025
@mobin-2008
Copy link
Collaborator

Hi. Thanks a lot. Can you confirm that dinit works when building it with ./configure CPPFLAGS="-DDASYNQ_HAVE_KQUEUE=1"?

@iMilnb
Copy link
Contributor Author

iMilnb commented Jan 29, 2025

It builds and works with CPPFLAGS="-DDASYNQ_HAVE_KQUEUE=1", I booted a virtual machine with it. Would you suggest to build with this defined as default?
If you're interested, my NetBSD-dinit system is here: https://github.com/NetBSDfr/smolBSD/tree/main/service/systembsd

@mobin-2008
Copy link
Collaborator

mobin-2008 commented Jan 29, 2025 •
edited
Loading

It builds and works with CPPFLAGS="-DDASYNQ_HAVE_KQUEUE=1", I booted a virtual machine with it. Would you suggest to build with this defined as default?

Probably, Based on https://man.netbsd.org/kqueue.2#HISTORY.

Also it would be great if you send a patch (after testing) to https://github.com/davmac314/dasynq and change the default "if" (https://github.com/davmac314/dasynq/blob/7cf74df6b758fa11876d26b5173c382a976a7917/include/dasynq/config.h#L56) to include NetBSD.

mobin-2008
mobin-2008 reviewed Jan 29, 2025
View reviewed changes
configure Outdated Show resolved Hide resolved
@iMilnb
Copy link
Contributor Author

iMilnb commented Jan 29, 2025

Also it would be great if you send a patch (after testing) to https://github.com/davmac314/dasynq and change the default "if" (https://github.com/davmac314/dasynq/blob/7cf74df6b758fa11876d26b5173c382a976a7917/include/dasynq/config.h#L56) to include NetBSD.

NetBSD patch submitted for dasynq

mobin-2008
mobin-2008 reviewed Jan 29, 2025
View reviewed changes
configure Outdated Show resolved Hide resolved
mobin-2008
mobin-2008 previously approved these changes Jan 29, 2025
View reviewed changes
Copy link
Collaborator

@mobin-2008 mobin-2008 left a comment

Choose a reason for hiding this comment

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

Except the mentioned thing, This PR is

Reviewed-by: Mobin Aydinfar <[email protected]>

@mobin-2008 mobin-2008 requested a review from davmac314 January 29, 2025 16:44
mobin-2008
mobin-2008 previously approved these changes Jan 29, 2025
View reviewed changes
Copy link
Collaborator

@mobin-2008 mobin-2008 left a comment

Choose a reason for hiding this comment

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

Reviewed-by: Mobin Aydinfar <[email protected]>

@davmac314
Copy link
Owner

Pleas fix commit messages (we don't use "feat:"; "Update configure" should be more specific and not duplicated.

Since shutdown works, should building it be enabled by default on NetBSD (as it is for Linux)?

davmac314
davmac314 requested changes Feb 2, 2025
View reviewed changes
Copy link
Owner

@davmac314 davmac314 left a comment

Choose a reason for hiding this comment

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

(Please respond to earlier comment).

@iMilnb
Copy link
Contributor Author

iMilnb commented Feb 4, 2025 •
edited
Loading

Pleas fix commit messages (we don't use "feat:"; "Update configure" should be more specific and not duplicated.

Done.

Since shutdown works, should building it be enabled by default on NetBSD (as it is for Linux)?

I think so, I've been using it for a couple of days with no issues.

davmac314
davmac314 approved these changes Feb 5, 2025
View reviewed changes
Copy link
Owner

@davmac314 davmac314 left a comment

Choose a reason for hiding this comment

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

Accepting as is, change to build shutdown by default on NetBSD can be made later if appropriate.
Thanks for the contribution!

@davmac314 davmac314 merged commit b2ca1a4 into davmac314:master Feb 5, 2025
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@davmac314 davmac314 davmac314 approved these changes

@mobin-2008 mobin-2008 mobin-2008 left review comments

Assignees
No one assigned
Labels
A-Importance: Normal C-dinit Things about the main parts of dinit C-shutdown Things about shutdown/reboot/halt utilities Enhancement/New Feature Improving things or introduce new feature P-BSD Things related to BSD
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@iMilnb @mobin-2008 @davmac314