Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

chore: add ability to include custom protoc-gen-go dependency in nix flake#14728

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged
dannykopping merged 6 commits intocoder:mainfromjoobisb:fix/flake-dep
Sep 24, 2024

Conversation

joobisb
Copy link
Contributor

This PR can be a starting point tofix#14343

@cdr-botcdr-botbot added the communityPull Requests and issues created by the community. labelSep 19, 2024
@joobisbjoobisb changed the titlefeat: ability to add custom protoc-gen-go dependency to nix flakefeat: add ability to include custom protoc-gen-go dependency in nix flakeSep 19, 2024
Copy link
Contributor

@dannykoppingdannykopping 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!

joobisb reacted with thumbs up emoji
flake.nix Outdated
repo = "protobuf-go";
rev = rev;
# This should be updated whenever rev changes!
# To update, set to "", run nix-shell, insert new hash
Copy link
Contributor

Choose a reason for hiding this comment

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

Where does the new hash come from?

Copy link
ContributorAuthor

Choose a reason for hiding this comment

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

There are a couple of ways to generate the hash, can use either of them

  1. nix-prefetch-git https://github.com/protocolbuffers/protobuf-go --rev v1.30.0
  2. Runnix-shell withsha256 = ""; this will fail to load the shell and output the required hash in the log as seen in the image, use that
image

Copy link
Contributor

Choose a reason for hiding this comment

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

Can you rather integrate this intoscripts/update-flake.sh pls?
The less we make folks think, the more likely they are to do the right thing.

Copy link
ContributorAuthor

Choose a reason for hiding this comment

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

yeah that would be better, I will do that

@joobisb
Copy link
ContributorAuthor

@dannykopping i've addressed the comments, please have a look

@matifali
Copy link
Member

I agree with the resolution to align nix and CI. But I am afraid in long run this will become a resistance to easily upgrade dev tools and packages.

Why we can't update the version in CI in to match with nix?

Or best use nix to run our CI.

@dannykopping
Copy link
Contributor

Why we can't update the version in CI in to match with nix?

CI is the only one with a pinned version; either way we'd have to do what@joobisb has implemented.

Or best use nix to run our CI.

That's the dream, but it'll take some time. In the interim, we can start by aligning versions IMHO.

joobisb and matifali reacted with thumbs up emoji

@dannykopping
Copy link
Contributor

@joobisb please avoid rebasing once a review has begun. There's no need to maintain a clean history, since we squash-merge anyway.

joobisb reacted with thumbs up emoji

@joobisb
Copy link
ContributorAuthor

@dannykopping I've updated the script

Copy link
Contributor

@dannykoppingdannykopping left a comment

Choose a reason for hiding this comment

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

Thanks!


# Update protoc-gen-go sha256
echo "Updating protoc-gen-go sha256..."
PROTOC_GEN_GO_REV=$(grep -A 20 'pkgs.buildGoModule rec' flake.nix | grep -A 10 'repo = "protobuf-go"' | grep 'rev = "v' | sed 's/.*rev = "\(.*\)".*/\1/')
Copy link
Contributor

@dannykoppingdannykoppingSep 20, 2024•
edited
Loading

Choose a reason for hiding this comment

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

This is flimsy (and possibly not portable across Linux/Mac/Windows); is there a more robust way we can get this?

Copy link
ContributorAuthor

Choose a reason for hiding this comment

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

@joobisb
Copy link
ContributorAuthor

@dannykopping I've addressed the comments, could you please have a look

@dannykopping
Copy link
Contributor

@joobisb sure thing, I'm on PTO since yesterday but back tomorrow.
Please address the CI failures as well.
We'll get this merged soon 👍 thanks for your patience!

@matifalimatifali changed the titlefeat: add ability to include custom protoc-gen-go dependency in nix flakechore: add ability to include custom protoc-gen-go dependency in nix flakeSep 24, 2024
@matifali
Copy link
Member

It should be called achore as it's not a user-facing feature. Also, please runmake fmt to fix the formatting issue.

Signed-off-by: Danny Kopping <danny@coder.com>
Signed-off-by: Danny Kopping <danny@coder.com>
Copy link
Contributor

@dannykoppingdannykopping 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 much for your contribution@joobisb!

@dannykoppingdannykoppingenabled auto-merge (squash)September 24, 2024 12:56
@dannykoppingdannykopping merged commitc127d90 intocoder:mainSep 24, 2024
27 checks passed
@github-actionsgithub-actionsbot locked and limited conversation to collaboratorsSep 24, 2024
@joobisbjoobisb deleted the fix/flake-dep branchOctober 4, 2024 14:31
@coadler
Copy link
Contributor

@joobisb I'm seeing the following failure on my NixOS machine

$ nix developwarning: Git tree '/home/colin/Projects/coder/coder' is dirtyerror: builder for '/nix/store/wg14v4ylmjrf2krlxbk6a049nvc23zad-protoc-gen-go.drv' failed with exit code 1;       last 10 log lines:       > calling 'postUnpack' function hook '_updateSourceDateEpochFromSourceRoot'       > Running phase: patchPhase       > Running phase: updateAutotoolsGnuConfigScriptsPhase       > Running phase: configurePhase       > calling 'preConfigure' function hook '_multioutConfig'       > Running phase: buildPhase       > evaling implicit 'preBuild' string hook       > go: github.com/golang/protobuf@v1.5.0: Get "https://proxy.golang.org/github.com/golang/protobuf/@v/v1.5.0.info": dial tcp: lookup proxy.golang.org on [::1]:53: read udp [::1]:33535->[::1]:53: read: connection refused       > go: github.com/google/go-cmp@v0.5.5: Get "https://proxy.golang.org/github.com/google/go-cmp/@v/v0.5.5.info": dial tcp: lookup proxy.golang.org on [::1]:53: read udp [::1]:54889->[::1]:53: read: connection refused       > /nix/store/5r0df66ikad3xw06azlqvswcvncll8wa-stdenv-linux/setup: line 193: pop_var_context: head of shell_variables not a function context       For full logs, run 'nix log /nix/store/wg14v4ylmjrf2krlxbk6a049nvc23zad-protoc-gen-go.drv'.error: 1 dependencies of derivation '/nix/store/z9ik49f82idmhi99ra9zvd3q38cqsg9s-nix-shell-env.drv' failed to build

On Linux, nix builds restrict network access. I was able to fix by deleting the following lines:

           subPackages = [ "cmd/protoc-gen-go" ];           vendorHash = null;-          proxyVendor = true;-          preBuild = ''-            export GOPROXY=https://proxy.golang.org,direct-            go mod download-          '';-

Is there a reason thispreBuild hook is necessary?

Sign up for freeto subscribe to this conversation on GitHub. Already have an account?Sign in.
Reviewers

@dannykoppingdannykoppingdannykopping approved these changes

Assignees

@joobisbjoobisb

Labels
communityPull Requests and issues created by the community.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Ensure all dependencies defined inflake.nix and our GitHub Actions are aligned
4 participants
@joobisb@matifali@dannykopping@coadler
[8]ページ先頭
©2009-2025 Movatter.jp