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

tdlib: 1.6.0 -> 1.6.9 #101948

Merged
merged 4 commits into from Oct 30, 2020
Merged

tdlib: 1.6.0 -> 1.6.9 #101948

merged 4 commits into from Oct 30, 2020

Conversation

@matthew-piziak
Copy link
Contributor

@matthew-piziak matthew-piziak commented Oct 28, 2020

Motivation for this change

This dependency is required for the current version of the Emacs package telega.

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.
@matthew-piziak
Copy link
Contributor Author

@matthew-piziak matthew-piziak commented Oct 28, 2020

I'm having trouble testing this. I'm trying to allow the telega client to be run on NixOS. I run tdlib = fetchWithConfig "https://github.com/NixOS/nixpkgs/archive/pull/101948/head.tar.gz" and then I install pkgs.tdlib.emacsPackagesNg.telega to hopefully pull in the updated dependency. This gives me the following stack trace:

error: while evaluating the attribute 'activationScript' of the derivation 'nixos-system-mus-20.09.1469.13d0c311e3a' at /nix/var/nix/profiles/per-user/root/channels/nixpkgs/nixos/modules/system/activation/top-level.nix:95:5:
while evaluating the attribute 'system.activationScripts.script' at /nix/var/nix/profiles/per-user/root/channels/nixpkgs/nixos/modules/system/activation/activation-script.nix:68:9:
while evaluating 'textClosureMap' at /nix/var/nix/profiles/per-user/root/channels/nixpkgs/lib/strings-with-deps.nix:70:35, called from /nix/var/nix/profiles/per-user/root/channels/nixpkgs/nixos/modules/system/activation/activation-script.nix:89:18:
while evaluating 'id' at /nix/var/nix/profiles/per-user/root/channels/nixpkgs/lib/trivial.nix:14:5, called from undefined position:
while evaluating the attribute 'text' at /nix/var/nix/profiles/per-user/root/channels/nixpkgs/nixos/modules/system/activation/activation-script.nix:9:5:
while evaluating the attribute 'text' at /nix/var/nix/profiles/per-user/root/channels/nixpkgs/lib/strings-with-deps.nix:77:38:
while evaluating the attribute 'sources' of the derivation 'etc' at /nix/var/nix/profiles/per-user/root/channels/nixpkgs/nixos/modules/system/etc/etc.nix:12:5:
while evaluating anonymous function at /nix/var/nix/profiles/per-user/root/channels/nixpkgs/nixos/modules/system/etc/etc.nix:20:20, called from undefined position:
while evaluating the attribute 'source' at undefined position:
while evaluating 'g' at /nix/var/nix/profiles/per-user/root/channels/nixpkgs/lib/attrsets.nix:276:19, called from undefined position:
while evaluating anonymous function at /nix/var/nix/profiles/per-user/root/channels/nixpkgs/lib/modules.nix:98:72, called from /nix/var/nix/profiles/per-user/root/channels/nixpkgs/lib/attrsets.nix:279:20:
while evaluating the attribute 'value' at /nix/var/nix/profiles/per-user/root/channels/nixpkgs/lib/modules.nix:465:9:
while evaluating the option `environment.etc.crontab.source':
while evaluating the attribute 'mergedValue' at /nix/var/nix/profiles/per-user/root/channels/nixpkgs/lib/modules.nix:497:5:
while evaluating anonymous function at /nix/var/nix/profiles/per-user/root/channels/nixpkgs/lib/modules.nix:499:17, called from /nix/var/nix/profiles/per-user/root/channels/nixpkgs/lib/modules.nix:499:12:
while evaluating 'check' at /nix/var/nix/profiles/per-user/root/channels/nixpkgs/lib/types.nix:256:15, called from /nix/var/nix/profiles/per-user/root/channels/nixpkgs/lib/modules.nix:499:22:
while evaluating the attribute 'allFiles' of the derivation 'crontabs' at /nix/store/3pxdiif5f4qams8ss18sq7i7728j03y1-nixpkgs-20.09.1469.13d0c311e3a/nixpkgs/pkgs/build-support/trivial-builders.nix:7:7:
while evaluating the attribute 'text' of the derivation 'system-crontab' at /nix/store/3pxdiif5f4qams8ss18sq7i7728j03y1-nixpkgs-20.09.1469.13d0c311e3a/nixpkgs/pkgs/build-support/trivial-builders.nix:7:7:
while evaluating the attribute 'passAsFile' of the derivation 'system-path' at /nix/store/3pxdiif5f4qams8ss18sq7i7728j03y1-nixpkgs-20.09.1469.13d0c311e3a/nixpkgs/pkgs/build-support/trivial-builders.nix:7:7:
while evaluating the attribute 'deps' of the derivation 'emacs-gcc-with-packages-20201026.0' at /nix/store/3pxdiif5f4qams8ss18sq7i7728j03y1-nixpkgs-20.09.1469.13d0c311e3a/nixpkgs/pkgs/build-support/trivial-builders.nix:7:7:
while evaluating the attribute 'explicitRequires' of the derivation 'emacs-packages-deps' at /nix/store/3pxdiif5f4qams8ss18sq7i7728j03y1-nixpkgs-20.09.1469.13d0c311e3a/nixpkgs/pkgs/build-support/trivial-builders.nix:7:7:
while evaluating the attribute 'buildInputs' of the derivation 'emacs-telega-20200814.1652' at /nix/store/gwcmrldyqjb1pcj2w2msi61xwmy3mh3p-source/pkgs/build-support/emacs/generic.nix:30:3:
cannot coerce a set to a string, at /nix/store/gwcmrldyqjb1pcj2w2msi61xwmy3mh3p-source/pkgs/build-support/emacs/generic.nix:30:3

@makefu
Copy link
Contributor

@makefu makefu commented Oct 28, 2020

@GrahamcOfBorg build tdlib

fix hash

version 1.6.9 is not released, so use revision hash
@matthew-piziak
Copy link
Contributor Author

@matthew-piziak matthew-piziak commented Oct 28, 2020

nix-shell -p nix-review --run 'nix-review pr 101948' succeeded:

$ git -c fetch.prune=false fetch --force https://github.com/NixOS/nixpkgs master:refs/nixpkgs-review/0 pull/101948/head:refs/nixpkgs-review/1
From https://github.com/NixOS/nixpkgs
 + a0140addeb1...8eeaad9129e refs/pull/101948/head -> refs/nixpkgs-review/1  (forced update)
$ git worktree add /root/.cache/nixpkgs-review/pr-101948-3/nixpkgs 2b06415ca1854e1f8993a7a4e760ef88e1c98a7b
Preparing worktree (detached HEAD 2b06415ca18)
Updating files: 100% (23066/23066), done.
HEAD is now at 2b06415ca18 Merge pull request #101370 from m1cr0man/ssl-test-certs
$ nix-env -f /root/.cache/nixpkgs-review/pr-101948-3/nixpkgs -qaP --xml --out-path --show-trac
e$ git merge --no-commit 8eeaad9129e265d36c7879765865793e66eee9a0
Automatic merge went well; stopped before committing as requested
$ nix-env -f /root/.cache/nixpkgs-review/pr-101948-3/nixpkgs -qaP --xml --out-path --show-trace --meta
1 package updated:
tdlib (1.6.0 → 1.6.9)

$ nix build --no-link --keep-going --option build-use-sandbox relaxed -f /root/.cache/nixpkgs-review/pr-101948-3/build.nix
https://github.com/NixOS/nixpkgs/pull/101948
1 package built:
tdlib

$ nix-shell /root/.cache/nixpkgs-review/pr-101948-3/shell.nix

@SuperSandro2000
Copy link
Member

@SuperSandro2000 SuperSandro2000 commented Oct 28, 2020

Result of nixpkgs-review pr 101948 run on x86_64-darwin 1

2 packages built:
  • haskellPackages.tdlib
  • tdlib

makefu
makefu approved these changes Oct 28, 2020
@makefu
Copy link
Contributor

@makefu makefu commented Oct 28, 2020

maybe you could add a comment to the rev why we are not using v${version} anymore.
replacing the version with the hash results results in nixpkgs-update not being able to update the package automatically.

fix hash

version 1.6.9 is not released, so use revision hash

add a comment to the rev about why we are not using `v${version}` anymore
@matthew-piziak
Copy link
Contributor Author

@matthew-piziak matthew-piziak commented Oct 28, 2020

Good idea @makefu. Done.

@matthew-piziak
Copy link
Contributor Author

@matthew-piziak matthew-piziak commented Oct 28, 2020

@GrahamcOfBorg build tdlib

@SuperSandro2000
Copy link
Member

@SuperSandro2000 SuperSandro2000 commented Oct 28, 2020

@GrahamcOfBorg build tdlib

You don't have permission to do that.

@matthew-piziak
Copy link
Contributor Author

@matthew-piziak matthew-piziak commented Oct 28, 2020

@SuperSandro2000 Oops, my mistake. Monkey see, monkey do. I thought it started a test build.

@@ -1,14 +1,20 @@
{ fetchFromGitHub, gperf, openssl, readline, zlib, cmake, stdenv }:

stdenv.mkDerivation rec {
version = "1.6.0";
version = "1.6.9";
Copy link
Member

@veprbl veprbl Oct 28, 2020

Since there is no tag and tdlib/td@32f2338 is just a random commit

Suggested change
version = "1.6.9";
version = "unstable-2020-10-25";

fix hash

version 1.6.9 is not released, so use revision hash

add a comment to the rev about why we are not using `v${version}` anymore

use unstable-2020-10-25
@matthew-piziak
Copy link
Contributor Author

@matthew-piziak matthew-piziak commented Oct 28, 2020

Thanks @veprbl! Fixed.

@veprbl
Copy link
Member

@veprbl veprbl commented Oct 28, 2020

The rev would need to stay the same, the suggestion was for the version.

fix hash

version 1.6.9 is not released, so use revision hash

add a comment to the rev about why we are not using `v${version}` anymore

use unstable-2020-10-25

update version, not revision
@matthew-piziak
Copy link
Contributor Author

@matthew-piziak matthew-piziak commented Oct 29, 2020

@veprbl Oops, my mistake. Fixed.

@nixos-discourse
Copy link

@nixos-discourse nixos-discourse commented Oct 29, 2020

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/prs-in-distress/3604/36

@veprbl
Copy link
Member

@veprbl veprbl commented Oct 29, 2020

@GrahamcOfBorg build haskellPackages.tdlib

veprbl
veprbl approved these changes Oct 29, 2020
Copy link
Member

@veprbl veprbl left a comment

LGTM, builds on x86_64-darwin

@matthew-piziak
Copy link
Contributor Author

@matthew-piziak matthew-piziak commented Oct 30, 2020

/marvin opt-in

@marvin-mk2 marvin-mk2 bot added the marvin label Oct 30, 2020
@marvin-mk2
Copy link

@marvin-mk2 marvin-mk2 bot commented Oct 30, 2020

Hi! I'm an experimental bot. My goal is to guide this PR through its stages, hopefully ending with a merge. You can read up on the usage here.

@matthew-piziak
Copy link
Contributor Author

@matthew-piziak matthew-piziak commented Oct 30, 2020

/status needs_merger

@marvin-mk2
Copy link

@marvin-mk2 marvin-mk2 bot commented Oct 30, 2020

The PR author cannot set the status to needs_merger. Please wait for an external review.

If you are not the PR author and you are reading this, please review the usage of this bot. You may be able to help. Please make an honest attempt to resolve all outstanding issues before setting to needs_merger.

@matthew-piziak
Copy link
Contributor Author

@matthew-piziak matthew-piziak commented Oct 30, 2020

Strange, thought it counted as reviewed.

@matthew-piziak
Copy link
Contributor Author

@matthew-piziak matthew-piziak commented Oct 30, 2020

/status awaiting_reviewer

@matthew-piziak
Copy link
Contributor Author

@matthew-piziak matthew-piziak commented Oct 30, 2020

/status needs_reviewer

@matthew-piziak
Copy link
Contributor Author

@matthew-piziak matthew-piziak commented Oct 30, 2020

/status needs_merger

@marvin-mk2
Copy link

@marvin-mk2 marvin-mk2 bot commented Oct 30, 2020

The PR author cannot set the status to needs_merger. Please wait for an external review.

If you are not the PR author and you are reading this, please review the usage of this bot. You may be able to help. Please make an honest attempt to resolve all outstanding issues before setting to needs_merger.

@Mic92 Mic92 merged commit 6793ec8 into NixOS:master Oct 30, 2020
21 of 22 checks passed
@matthew-piziak
Copy link
Contributor Author

@matthew-piziak matthew-piziak commented Oct 30, 2020

Thank you @Mic92!!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

6 participants