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: rewrite makefile in python #52788

Closed
wants to merge 1 commit into from

Conversation

RedYetiDev
Copy link
Member

This pull request introduces a Python build file to replace the Node.js Makefile. In my opinion, maintaining both vcbuild and Makefile (and keeping them feature-compatible) seems difficult. This change should, in my opinion, make maintenance easier, but there may be a learning curve.

The Python version assumes that the user, even on Windows, has Make installed. However, it doesn't rely on Unix commands except in the v8 target (which I aim to change soon) and MacOS-specific cases.

@nodejs-github-bot
Copy link
Collaborator

Review requested:

  • @nodejs/actions
  • @nodejs/gyp
  • @nodejs/security-wg
  • @nodejs/tsc

@RedYetiDev RedYetiDev marked this pull request as draft May 1, 2024 18:48
@nodejs-github-bot nodejs-github-bot added build Issues and PRs related to build files or the CI. doc Issues and PRs related to the documentations. meta Issues and PRs related to the general management of the project. needs-ci PRs that need a full CI run. tools Issues and PRs related to the tools directory. windows Issues and PRs related to the Windows platform. labels May 1, 2024
@RedYetiDev
Copy link
Member Author

This PR is a draft, as it will probably take time to prepare and implement.

@RedYetiDev RedYetiDev added the review wanted PRs that need reviews. label May 1, 2024
@RedYetiDev RedYetiDev added the discuss Issues opened for discussions and feedbacks. label May 1, 2024
@tniessen
Copy link
Member

tniessen commented May 1, 2024

Is there a discussion/issue leading up to this? I wasn't aware that the Makefile was introducing additional requirements that aren't already there due to dependencies, native addons, or so.

In any case, I assume that various external build processes depend on the Node.js Makefile.

@RedYetiDev
Copy link
Member Author

RedYetiDev commented May 1, 2024

Is there a discussion/issue leading up to this? I wasn't aware that the Makefile was introducing additional requirements that aren't already there due to dependencies, native addons, or so.

There wasn't a discussion around this previously, I wanted to start a discussion, and get some opinions. In my opinion, the makefile is hard to read (maybe I'm just inexperienced with makefiles). I also think that the vcbuild file is confusing to use, as IIRC it's not feature-equal to the makefile.

In any case, I assume that various external build processes depend on the Node.js Makefile.

I would imagine that would be a concern, but this is still a draft for now, and the issues can be worked out.

Copy link
Member

@benjamingr benjamingr left a comment

Choose a reason for hiding this comment

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

Tools interface with the makefile and vcbuild file.. so do consumers.

What motivated you to do this prior to asking/opening an issue?

@RedYetiDev
Copy link
Member Author

What motivated you to do this prior to asking/opening an issue?

It wasn't a big project, just a curious idea. I wondered if Makefiles could work in Python, so I tested it with Node.js. After finishing, I decided to open PR, because there really isn't anything to lose from opening one.

@MoLow
Copy link
Member

MoLow commented May 2, 2024

+1 on opening an issue to discuss before such big changes

@RedYetiDev RedYetiDev closed this May 6, 2024
@RedYetiDev RedYetiDev deleted the python-makefile branch May 6, 2024 17:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
build Issues and PRs related to build files or the CI. discuss Issues opened for discussions and feedbacks. doc Issues and PRs related to the documentations. meta Issues and PRs related to the general management of the project. needs-ci PRs that need a full CI run. review wanted PRs that need reviews. tools Issues and PRs related to the tools directory. windows Issues and PRs related to the Windows platform.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants