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

fix: update mechanism to pass cgo to golang #32

Merged
merged 1 commit into from
Jan 28, 2025
Merged

fix: update mechanism to pass cgo to golang #32

merged 1 commit into from
Jan 28, 2025

Conversation

dbarrosop
Copy link
Member

@dbarrosop dbarrosop commented Jan 28, 2025

PR Type

Enhancement


Description

  • Update CGO_ENABLED setting in package overrides

  • Remove cgoEnabled parameter from go.nix

  • Correct typo in CGO_ENABLED for amd64-darwin

  • Streamline package definitions in flake.nix


Changes walkthrough 📝

Relevant files
Enhancement
flake.nix
Update CGO settings in package overrides                                 

lib/go/example/flake.nix

  • Replace cgoEnabled = 0 with CGO_ENABLED = "0" in package overrides
  • Fix typo: GO_ENABLED = "0" to CGO_ENABLED = "0" for amd64-darwin
  • Simplify package definitions by moving CGO_ENABLED to overrideAttrs
  • +4/-8     
    go.nix
    Remove cgoEnabled parameter from go.nix                                   

    lib/go/go.nix

  • Remove cgoEnabled parameter from function definition
  • Remove CGO_ENABLED = cgoEnabled; line from buildGoModule
  • +0/-3     

    Need help?
  • Type /help how to ... in the comments thread for any questions about PR-Agent usage.
  • Check out the documentation for more information.
  • Copy link
    Contributor

    PR Reviewer Guide 🔍

    Here are some key observations to aid the review process:

    ⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
    🧪 No relevant tests
    🔒 No security concerns identified
    ⚡ Recommended focus areas for review

    Typo

    There's a typo in the CGO_ENABLED setting for amd64-darwin. It's set to GO_ENABLED instead of CGO_ENABLED.

    }).overrideAttrs (old: old // { GOOS = "darwin"; GOARCH = "amd64"; GO_ENABLED = "0"; });

    Copy link
    Contributor

    PR Code Suggestions ✨

    Explore these optional code suggestions:

    CategorySuggestion                                                                                                                                    Score
    Possible issue
    Fix typo in CGO setting

    Correct the typo in the 'example-amd64-darwin' package configuration. Change
    'GO_ENABLED' to 'CGO_ENABLED' to ensure consistent CGO settings across all package
    configurations.

    lib/go/example/flake.nix [66-68]

     example-amd64-darwin = (nixops-lib.go.package {
       inherit name src version ldflags buildInputs nativeBuildInputs;
    -}).overrideAttrs (old: old // { GOOS = "darwin"; GOARCH = "amd64"; GO_ENABLED = "0"; });
    +}).overrideAttrs (old: old // { GOOS = "darwin"; GOARCH = "amd64"; CGO_ENABLED = "0"; });
    Suggestion importance[1-10]: 9

    Why: The suggestion correctly identifies and fixes a critical typo in the 'example-amd64-darwin' package configuration. Changing 'GO_ENABLED' to 'CGO_ENABLED' ensures consistency with other package configurations and prevents potential build issues.

    9

    @dbarrosop dbarrosop merged commit a75d764 into main Jan 28, 2025
    3 checks passed
    @dbarrosop dbarrosop deleted the cgo branch January 28, 2025 13:12
    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.

    2 participants