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.

Sign up for GitHub

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

Jump to bottom

gnutls: remove autogen from build dependencies #110056

Merged
merged 1 commit into from Jan 27, 2021
Merged

gnutls: remove autogen from build dependencies #110056

merged 1 commit into from Jan 27, 2021

Conversation

bobrik
Copy link
Contributor

@bobrik bobrik commented Jan 20, 2021

Motivation for this change

There's an error when compiling autogen on macos Big Sur with #105026,
and it compiles fine without autogen, so I see no reason to keep it.

The dependency on autogen was originally introduced in 31a128b,
but unfortunately there's no explanation for the reason and no linked issue.

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.

@ofborg ofborg bot requested review from fpletz and edolstra January 20, 2021 02:06
@SuperSandro2000
Copy link
Member

/rebase-staging

@bobrik @github-actions
gnutls: remove autogen from build dependencies
47f4eb0 
There's an error when compiling autogen on macos Big Sur with NixOS#105026,
and it compiles fine without autogen, so I see no reason to keep it.

The dependency on autogen was originally introduced in 31a128b,
but unfortunately there's no explanation for the reason and no linked issue.
@github-actions github-actions bot changed the base branch from master to staging January 20, 2021 07:36
@vcunat
Copy link
Member

vcunat commented Jan 22, 2021

So, which -darwin have you tested? Both? (and I assume you kept default withSecurity = false)

vcunat
vcunat approved these changes Jan 22, 2021
View reviewed changes
Copy link
Member

@vcunat vcunat left a comment

Choose a reason for hiding this comment

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

Nice. At least for Linux platforms I'm confident it's good. (and reducing unnecessary dependencies is good generally)

@SuperSandro2000
Copy link
Member

but unfortunately there's no explanation for the reason and no linked issue.

It replaced the autoreconfHook. No explanation required.

@bobrik
Copy link
Contributor Author

bobrik commented Jan 22, 2021

So, which -darwin have you tested? Both?

I tested on aarch64-darwin (#105026), the only thing I have.

and I assume you kept default withSecurity = false

I haven't changed any defaults other than adding aarch64-darwin to extra-platforms.

@vcunat vcunat merged commit e6a8527 into NixOS:staging Jan 27, 2021
@bobrik
Copy link
Contributor Author

bobrik commented Feb 16, 2021

Is there a doc describing how long it should take for changes to make their way from staging into master branch? I've seen this one from @vcunat, but it doesn't have the info:

It's been 20 days now for this change and I'm not sure if it's expected or not.

@SuperSandro2000
Copy link
Member

It will be on master with the next merge of staging-next which will most likely happen with #112095.

@bobrik
Copy link
Contributor Author

bobrik commented Feb 16, 2021

This search seems to be the way to discover the PRs from staging-next into master:

Should I open a PR for README to add this info?

@bobrik bobrik deleted the ivan/disable-gnutls branch March 6, 2021 22:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@vcunat vcunat vcunat approved these changes

@fpletz fpletz Awaiting requested review from fpletz

@edolstra edolstra Awaiting requested review from edolstra

Assignees
No one assigned
Labels
10.rebuild-darwin: 501+ 10.rebuild-darwin: 1001-2500 10.rebuild-linux: 501+ 10.rebuild-linux: 5001+
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@bobrik @SuperSandro2000 @vcunat