-
-
Notifications
You must be signed in to change notification settings - Fork 14k
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
detect-it-easy: init at 3.09 #349841
detect-it-easy: init at 3.09 #349841
Conversation
f803e53
to
71ed014
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall your PR looks good :)
Please reorder your commits, your maintainers commit has to be first:
Lines 506 to 509 in bd95a9a
- When adding yourself as maintainer in the same pull request, make a separate | |
commit with the message `maintainers: add <handle>`. | |
Add the commit before those making changes to the package or module. | |
See [Nixpkgs Maintainers](./maintainers/README.md) for details. |
Signed-off-by: Ivy Fan-Chiang <dev@ivyfanchiang.ca>
71ed014
to
b0f4a2c
Compare
Thanks for the advice @zi3m5f, made the changes you requested |
b0f4a2c
to
ff222bd
Compare
cc @emilytrau |
ff222bd
to
0323018
Compare
Thanks again @zi3m5f, didn't spot the issues in Detect It Easy's |
0323018
to
73afd2e
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems to be working a lot better with the changes you suggested.
Unfortunately there are still problems:
$ nix shell nixpkgs#desktop-file-utils -c desktop-file-validate result/share/applications/die.desktop
> error: value "#VERSION#" for key "Version" in group "Desktop Entry" is not a known version
It seems die.desktop
in $out
differs from LINUX/die.desktop
in $src
.
Could you investigate why? Generally we have at least two options here:
- fix copied/modified desktop file
- remove and generate our own again
Also $out/lib/{bin,share/applications,icons}
should be removed.
@zi3m5f the |
Another solution would be to use patches = [
(fetchpatch {
url = "https://github.com/horsicq/DIE-engine/commit/<commit>.patch";
hash = "";
})
]; for both Your choice ;) |
73afd2e
to
f9aa607
Compare
Looks like a patch of XOptions/xoptions.cpp in the |
f9aa607
to
221db6e
Compare
Signed-off-by: Ivy Fan-Chiang <dev@ivyfanchiang.ca>
221db6e
to
d6cd290
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I didn't run/test any binaries but it compiles and result looks normal.
nixpkgs-review
result
Generated using nixpkgs-review
.
Command: nixpkgs-review pr 349841
x86_64-linux
✅ 1 package built:
- detect-it-easy
Thank you so much for the advice on this first PR @emilytrau and @zi3m5f!!! ❤️ |
Successfully created backport PR for |
Program for determining types of files for Windows, Linux and MacOS. https://github.com/horsicq/Detect-It-Easy/
Used for reverse engineering and binary analysis work to identify compiler versions and other file information.
Note that there is support for aarch64-linux and aarch64-darwin was recently added to the DIE-engine repository but no support exists in the latest stable 3.09 version.
Things done
nix.conf
? (See Nix manual)sandbox = relaxed
sandbox = true
nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD"
. Note: all changes have to be committed, also see nixpkgs-review usage./result/bin/
)Add a 👍 reaction to pull requests you find important.