8000 Allow disabling of all network accesses by ibbem · Pull Request #285 · jekyll/github-metadata · GitHub
[go: up one dir, main page]

Skip to content

Allow disabling of all network accesses #285

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

Merged
merged 3 commits into from
Mar 24, 2025

Conversation

ibbem
Copy link
Contributor
@ibbem ibbem commented Feb 21, 2025

Context

We use Nix to build our project reproducibly, including the github-pages documentation. This setup is also used in our CI to build the github-pages which are eventually deployed. Nix prohibits network connections (except if the output hash is specified, which is unpractical in this case) because the network is not reproducible. However, in some circumstances (e.g., the default installation on macOS) the Nix sandbox is disabled or weakened. Hence, github-metadata accesses the network which results in failing builds or differing outputs.

Feature Request

We would like to reliably disable network accesses without actually disabling the network.

Implemented solution

Network accesses are skipped if the environment variable PAGES_DISABLE_NETWORK is set.

Related issues

@ashmaroli
Copy link
Member

Hello @ibbem, please revert the change to History.markdown. It gets updated automatically with the pull-request-title when the pull request is merged. Additionally, this requires a test to verify that setting the env var does what is intended.

P.S. You need not rebase your branch. All commits will be squashed automatically at merge.

@ashmaroli ashmaroli requested review from parkr and mattr- February 23, 2025 13:32
@ibbem
Copy link
Contributor Author
ibbem commented Mar 24, 2025

From my side, this PR is ready to be (squash) merged.
Is there anything else that I need to address?

@ashmaroli
Copy link
Member

No @ibbem, this LGTM!
Thank you for your contribution.

@jekyllbot: merge +minor

@jekyllbot jekyllbot merged commit 17cc5af into jekyll:main Mar 24, 2025
8 checks passed
jekyllbot added a commit that referenced this pull request Mar 24, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants
0