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

deps: fetch build artefacts using a proxy where necessary #52

Closed
wants to merge 3 commits into from

Conversation

jnsgruk
Copy link
Contributor

@jnsgruk jnsgruk commented Nov 4, 2023

I've been struggling to build Signal from source where the build machine is behind a proxy.

This change will have no effect during build unless the HTTPS_PROXY variable is set, in which case it establishes an agent that will ensure the https.get calls use the proxy correctly.

I'm open to suggestions on a different approach if the addition of another dependency is too undesirable.

@jnsgruk
Copy link
Contributor Author

jnsgruk commented Nov 4, 2023

(I have now signed the CLA!)

Copy link
Contributor

@rashad-signal rashad-signal left a comment

Choose a reason for hiding this comment

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

Thanks for the contribution! The general approach looks good. There are just a few small things to address.

src/node/package.json Outdated Show resolved Hide resolved
src/node/scripts/fetch-prebuild.js Outdated Show resolved Hide resolved
@jnsgruk
Copy link
Contributor Author

jnsgruk commented Nov 8, 2023

Updated per your review, thanks! I've done the same over on signalapp/better-sqlite3#12

@rashad-signal
Copy link
Contributor

Thanks for making the changes. It's now available in 9dca1e6 which is part of RingRTC v2.34.4.

@jnsgruk
Copy link
Contributor Author

jnsgruk commented Nov 10, 2023

Thanks @rashad-signal! Do I need to do anything with the similar pull on better-sqlite3?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants