Skip to content

Conversation

NewFuture
Copy link
Owner

No description provided.

@NewFuture NewFuture requested a review from Copilot June 7, 2025 21:03
Copilot

This comment was marked as outdated.

@NewFuture NewFuture requested a review from Copilot June 8, 2025 13:55
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR adds support for producing a statically linked musl-based ddns binary, refines existing CI steps, and ensures the correct MutableMapping import on Python 2/3.

  • Annotate Python version branches in util/cache.py.
  • Update GitHub Actions: switch test -e to test -f for stricter file checks, and introduce a pack-musl job.
  • Add an install script for patchelf and a multi-stage Dockerfile to build the musl binary.

Reviewed Changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.

File Description
util/cache.py Added inline comments to clarify Python 3 vs. Python 2 import paths.
.github/workflows/build.yml Changed file-existence tests, removed ambiguity, and added pack-musl job.
.github/install-patchelf.sh New script to fetch and install a specific patchelf release.
.build/musl.Dockerfile New Dockerfile for building a static musl-linked binary via Nuitka.
Comments suppressed due to low confidence (2)

.build/musl.Dockerfile:10

  • The fallback installs mix version 0.17.2.1 via pip while the install script targets 0.18.0—this version mismatch may lead to unexpected behavior. Align versions or centralize the version constant.
RUN apk add --update --no-cache patchelf\    || /tmp/install-patchelf.sh\    || pip3 install patchelf==0.17.2.1

.github/workflows/build.yml:290

  • This run: step mixes a commented command at the same indentation, which likely breaks YAML parsing. Consider using a multiline literal (|) for the script block or removing the inline comment.
run: find output -mindepth 2 -type f -exec mv -t output {} +

run: ${{env.PY}} run.py -h
- name: test config generation
run: ${{env.PY}} run.py || test -e config.json
run: ${{env.PY}} run.py || test -f config.json
Copy link
Preview

Copilot AI Jun 8, 2025

Choose a reason for hiding this comment

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

[nitpick] This run: step is duplicated across multiple jobs—consider extracting it into a reusable workflow step or using YAML anchors to reduce repetition.

Copilot uses AI. Check for mistakes.

@NewFuture NewFuture merged commit 28e2748 into master Jun 8, 2025
25 checks passed
@NewFuture NewFuture deleted the musl-build branch June 8, 2025 15:55
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.

1 participant