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

Bring in missing python (system dependency) #1639

Merged
merged 2 commits into from
Mar 28, 2024

Conversation

vraravam
Copy link
Contributor

@vraravam vraravam commented Mar 23, 2024

Pre-flight Checklist

Please ensure you've completed all of the following.

Description of Change

Use the upgrade of sqlite as a litmus test to bring in missing python (system dependency)

Motivation and Context

For most native dependencies, if a pre-built binary is not available, npm will then try to build from source, which includes using python and node-gyp to compile. The intention of this PR is to resurrect python if needed to be able to support those modules that need native compilation on different OSes. I am trying to see if sqlite3 (at the current moment, no pre-built binaries are present for the latest version) can be used as a litmus test for the above build setup on both local and CI.

Screenshots

Checklist

  • My pull request is properly named
  • The changes respect the code style of the project (pnpm prepare-code)
  • pnpm test passes
  • I tested/previewed my changes locally

Release Notes

@vraravam vraravam self-assigned this Mar 23, 2024
@vraravam vraravam changed the title temp Bring in missing python (system dependency) Mar 23, 2024
@vraravam vraravam force-pushed the sqlite-and-python branch 7 times, most recently from a2aa57e to e92e8fa Compare March 24, 2024 09:51
@vraravam vraravam marked this pull request as ready for review March 24, 2024 11:21
(needed by node-gyp to compile native modules)
Introduce node-pre-gyp as a dependency
Copy link
Member

@SpecialAro SpecialAro left a comment

Choose a reason for hiding this comment

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

LGTM 😁 Great work!

@vraravam vraravam merged commit 53c72ea into ferdium:develop Mar 28, 2024
4 checks passed
@vraravam vraravam deleted the sqlite-and-python branch March 28, 2024 10:12
@@ -141,6 +143,7 @@
"@electron/notarize": "1.2.3",
"@formatjs/cli": "6.2.7",
"@jest/types": "29.6.3",
"@mapbox/node-pre-gyp": "1.0.11",
Copy link
Member

Choose a reason for hiding this comment

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

The sqlite3 module has moved away from using node-pre-gyp and is using prebuild-install instead (cf TryGhost/node-sqlite3#1735), so I do not think this dependency is needed nowadays.

vraravam added a commit that referenced this pull request Mar 31, 2024
Since its causing all builds to fail

This reverts commit 53c72ea.
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.

3 participants