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

cntb: init at 1.4.6 #255854

Merged
merged 1 commit into from
Sep 22, 2023
Merged

cntb: init at 1.4.6 #255854

merged 1 commit into from
Sep 22, 2023

Conversation

aciceri
Copy link
Member

@aciceri aciceri commented Sep 18, 2023

Description of changes

Things done

  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandbox = true set in nix.conf? (See Nix manual)
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 23.11 Release Notes (or backporting 23.05 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
  • Fits CONTRIBUTING.md.

@AndersonTorres
Copy link
Member

Have you read the ofborg failure messages?

@aciceri
Copy link
Member Author

aciceri commented Sep 18, 2023

@AndersonTorres there was written "all checks are passed" but apparently it was because some of them (which were failing) were "neutral" (why?)

Anyway fixed (I hope), I was experimenting with a fork and when I came back to the upstream source I restored only the hash but not the rev attribute. So it was even working on my computer!

Also, do you prefer if I avoid using rec? Usually I prefer using let but I rec seems to be widely used on nix

@AndersonTorres
Copy link
Member

Ofborg usually does a quick syntax check before running the code. If it founds syntax errors (e.g. mismatched parens) it emits red marks.

The grey marks occur at "runtime", in which the code truly runs: fetch source code, patch, compilation, installation...

Nix the language does not control this, it just delegates it to the inferior OS.

@aciceri
Copy link
Member Author

aciceri commented Sep 18, 2023

It makes definitely sense.

But why now it still says "2 neutral checks"? If I click on those checks they are still referring to the previous failure even if I force pushed now. I can understand that probably there is a longer queue for darwin but shouldn't it say something like "queued" instead of referencing the old failed check?

@K900
Copy link
Contributor

K900 commented Sep 19, 2023

@ofborg eval

@aciceri
Copy link
Member Author

aciceri commented Sep 19, 2023

Ok problem solved, apparently fetchFromGitHub needs a different hash on darwin. Now it's fixed.
Moreover I can confirm that the program itself perfectly works, at least on x86_64-linux.

As far as I'm concerned it's mergeable.

@AndersonTorres
Copy link
Member

This is too strange. Why the same source code generates different hashes in different OSes? It deserves a bug report.
Since it is working OK on x86_64, I suggest you set meta.broken = stdenv.isDarwin; and open a bug report to Nixpkgs later.

@aciceri
Copy link
Member Author

aciceri commented Sep 20, 2023

I asked what was going on #nix:nixos.org and @alyssais suggested that it could be that darwin cannot distinguish between files with the same name modulo case.

Indeed running find . | perl -ne 's!([^/]+)$!lc $1!e; print if 1 == $seen{$_}++' I found out these two files:

./openapi/docs/AddOnResponse.md
./openapi/docs/AddonResponse.md

I don't know what happens when these files are fetched on darwin, probably one of them is overwritten. Anyway, since they are documentation it shouldn't be a problem.

pkgs/by-name/cn/cntb/package.nix Outdated Show resolved Hide resolved
pkgs/by-name/cn/cntb/package.nix Show resolved Hide resolved
pkgs/by-name/cn/cntb/package.nix Outdated Show resolved Hide resolved
@slavni96
Copy link

Run ok on macOS 13.5.2 (x86_64-darwin)

(base) ➜  ~ system_profiler SPSoftwareDataType

Software:

    System Software Overview:

      System Version: macOS 13.5.2 (22G91)
      Kernel Version: Darwin 22.6.0
(base) ➜  ~ uname -a
Darwin MacBook-Pro 22.6.0 Darwin Kernel Version 22.6.0: Wed Jul  5 22:21:56 PDT 2023; root:xnu-8796.141.3~6/RELEASE_X86_64 x86_64

pkgs/by-name/cn/cntb/package.nix Outdated Show resolved Hide resolved
Copy link
Member

@alyssais alyssais left a comment

Choose a reason for hiding this comment

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

LGTM other than this.

pkgs/by-name/cn/cntb/package.nix Outdated Show resolved Hide resolved
@alyssais alyssais merged commit 044a8a9 into NixOS:master Sep 22, 2023
@aciceri aciceri deleted the cntb branch September 22, 2023 12:22
@aciceri aciceri mentioned this pull request Feb 27, 2024
13 tasks
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.

5 participants