Skip to content

neohtop: init at 1.1.1#356826

Closed
HannesGitH wants to merge 1 commit into
NixOS:masterfrom
HannesGitH:neohtop
Closed

neohtop: init at 1.1.1#356826
HannesGitH wants to merge 1 commit into
NixOS:masterfrom
HannesGitH:neohtop

Conversation

@HannesGitH

@HannesGitH HannesGitH commented Nov 17, 2024

Copy link
Copy Markdown
Member

Things done

  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)
    • sandbox = relaxed
    • sandbox = true
  • 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/)
  • 25.05 Release Notes (or backporting 24.11 and 25.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.

Add a 👍 reaction to pull requests you find important.

@HannesGitH HannesGitH force-pushed the neohtop branch 2 times, most recently from ff162e7 to d7db841 Compare November 18, 2024 15:03
@HannesGitH HannesGitH marked this pull request as ready for review November 18, 2024 15:03
@HannesGitH HannesGitH force-pushed the neohtop branch 2 times, most recently from 3715bdb to 3bd3da6 Compare November 18, 2024 15:16
@HannesGitH HannesGitH marked this pull request as draft November 18, 2024 15:16
@HannesGitH HannesGitH marked this pull request as ready for review November 18, 2024 15:22
@HannesGitH

HannesGitH commented Nov 18, 2024

Copy link
Copy Markdown
Member Author

@NixOS/nix-formatting the formatting check fails if formatted with
nix run '.#nixfmt' 'pkgs/by-name/ne/neohtop/package.nix'
but then recommands using exactly that to format, but it actually requires
nix run '.#nixfmt-rfc-style' 'pkgs/by-name/ne/neohtop/package.nix'

@HannesGitH HannesGitH mentioned this pull request Nov 18, 2024
12 tasks
@ambroisie

Copy link
Copy Markdown
Contributor

A new fetcher has been introduced and its usage is encouraged: #349360.

@HannesGitH

Copy link
Copy Markdown
Member Author

A new fetcher has been introduced and its usage is encouraged: #349360.

the docs say to use that new fetcher only if cargo lock has git dependencies:

Exception: If the application has cargo git dependencies, the cargoHash approach will not work by default. In this case, you can set useFetchCargoVendor = true to use an improved fetcher that supports handling git dependencies.

but if you say I should use that.. does simply adding useFetchCargoVendor = true; do the trick, or do i need to something else? @ambroisie

@ambroisie

Copy link
Copy Markdown
Contributor

Hmm, my understanding was that it avoiding us vendoring the Cargo.lock was a plus in all cases.

I'll defer to more knowledgeable Rust maintainers to weight in the.

@ofborg ofborg Bot added 8.has: package (new) This PR adds a new package 10.rebuild-darwin: 1-10 This PR causes between 1 and 10 packages to rebuild on Darwin. 10.rebuild-darwin: 1 This PR causes 1 package to rebuild on Darwin. 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux. 10.rebuild-linux: 1 This PR causes 1 package to rebuild on Linux. labels Nov 19, 2024
Comment thread pkgs/by-name/ne/neohtop/package.nix Outdated

@JohnRTitor JohnRTitor Nov 19, 2024

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Why not use cargoHash and get rid of the lockfile?

@HannesGitH HannesGitH Nov 19, 2024

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

because this does not work (cargo cannot find reqwest and the build fails) I think its related to:
https://github.com/HannesGitH/nixpkgs/blob/c43b2b1ea412767dd01e4354f4ca05ff942df4e4/pkgs/by-name/su/surrealist/package.nix#L52-L57

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

@FliegendeWurst FliegendeWurst left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

A meta attribute is required

Comment thread pkgs/by-name/ne/neohtop/package.nix Outdated
Comment thread pkgs/by-name/ne/neohtop/package.nix Outdated
Comment thread pkgs/by-name/ne/neohtop/package.nix Outdated
@ofborg ofborg Bot added the 11.by: package-maintainer This PR was created by a maintainer of all the package it changes. label Nov 21, 2024

@FliegendeWurst FliegendeWurst left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think you need to include openssl in buildInputs

@ghost

ghost commented Nov 21, 2024

Copy link
Copy Markdown

#354007

@HannesGitH

Copy link
Copy Markdown
Member Author

I think you need to include openssl in buildInputs

why? it builds and runs as it currently is

@FliegendeWurst

Copy link
Copy Markdown
Member

why? it builds and runs as it currently is

Not on Linux (see ofborg)

@HannesGitH

HannesGitH commented Nov 21, 2024

Copy link
Copy Markdown
Member Author

why? it builds and runs as it currently is

Not on Linux (see ofborg)

aight i'll quickly add openssl
is added now ✅
lets see

@FliegendeWurst

Copy link
Copy Markdown
Member

It is a little hidden because it will mark a build failure as "neutral".

Comment thread pkgs/by-name/ne/neohtop/package.nix Outdated
Comment thread pkgs/by-name/ne/neohtop/package.nix Outdated
Comment thread pkgs/by-name/ne/neohtop/package.nix Outdated
Comment thread pkgs/by-name/ne/neohtop/package.nix Outdated
Comment thread pkgs/by-name/ne/neohtop/package.nix Outdated
Comment thread pkgs/by-name/ne/neohtop/package.nix Outdated
Comment thread pkgs/by-name/ne/neohtop/package.nix Outdated

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think the normal way to do this is simply to copy the entire .app folder to $out(maybe share)?

Comment thread pkgs/by-name/ne/neohtop/package.nix Outdated
Comment thread pkgs/by-name/ne/neohtop/package.nix Outdated
Comment thread pkgs/by-name/ne/neohtop/package.nix Outdated
@HannesGitH HannesGitH marked this pull request as draft November 21, 2024 20:49
@HannesGitH

Copy link
Copy Markdown
Member Author

thanks for your feedback, ill hopefully be able to integrate that tomorrow!

@wegank wegank added the 2.status: merge conflict This PR has merge conflicts with the target branch label Apr 2, 2025
@ghost ghost marked this as a duplicate of #391823 Apr 3, 2025
@ghost ghost closed this Apr 3, 2025
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

2.status: merge conflict This PR has merge conflicts with the target branch 8.has: package (new) This PR adds a new package 10.rebuild-darwin: 1-10 This PR causes between 1 and 10 packages to rebuild on Darwin. 10.rebuild-darwin: 1 This PR causes 1 package to rebuild on Darwin. 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux. 10.rebuild-linux: 1 This PR causes 1 package to rebuild on Linux. 11.by: package-maintainer This PR was created by a maintainer of all the package it changes.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants