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

build: ease DragonFlyBSD build #29284

Closed
wants to merge 2 commits into from
Closed

build: ease DragonFlyBSD build #29284

wants to merge 2 commits into from

Conversation

devnexen
Copy link
Contributor

Implicitly pretending being FreeBSD and disable
large pages for this platform.

@nodejs-github-bot nodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. lib / src Issues and PRs related to general changes in the lib or src directory. labels Aug 23, 2019
@devnexen devnexen changed the title build: Ease DragonFlyBSD build build: ease DragonFlyBSD build Aug 23, 2019
Implicitly pretending being FreeBSD and disable
large pages for this platform.
Copy link
Member

@bnoordhuis bnoordhuis left a comment

Choose a reason for hiding this comment

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

LGTM with a suggestion. Can you break out the change to tools/gyp/pylib/gyp/common.py into its own commit? That makes gyp upgrades a little easier.

(Not that I expect that there will be many but still.)

@@ -24,6 +24,10 @@
#include "util.h"
#include "uv.h"

#if defined(__DragonFly__)
#error "Large pages is not supported by this platform"
#endif
Copy link
Member

Choose a reason for hiding this comment

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

It would be nicer to error out in configure.py rather than defer until build time. It should preferably also be its own separate commit.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That comes more from the fact dragonflybsd "is" FreeBSD from configure standpoint.

@nodejs-github-bot
Copy link
Collaborator

@fhinkel
Copy link
Member

fhinkel commented Oct 28, 2019

@devnexen ping. Should we merge or close this?

@devnexen
Copy link
Contributor Author

Can be merged if good as is

@nodejs-github-bot
Copy link
Collaborator

@fhinkel
Copy link
Member

fhinkel commented Nov 1, 2019

@devnexen Can you squash your commits and rebase please. Thanks

@devnexen
Copy link
Contributor Author

devnexen commented Nov 1, 2019

AS I lost the original fork here it is #30201

@devnexen devnexen closed this Nov 1, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c++ Issues and PRs that require attention from people who are familiar with C++. lib / src Issues and PRs related to general changes in the lib or src directory.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants