Skip to content

grip-0.8: indexed grip#60182

Merged
JohnAZoidberg merged 1 commit intoNixOS:masterfrom
tex:grip
Nov 14, 2019
Merged

grip-0.8: indexed grip#60182
JohnAZoidberg merged 1 commit intoNixOS:masterfrom
tex:grip

Conversation

@tex
Copy link
Contributor

@tex tex commented Apr 24, 2019

Motivation for this change

New CLI application for indexed grepping. Fast. Useful. Can be cross-compiled for Windows.

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS)
  • 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 nix-review --run "nix-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)
  • Assured whether relevant documentation is up to date
  • Fits CONTRIBUTING.md.

@ofborg ofborg bot added 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin. 10.rebuild-linux: 0 This PR does not cause any packages to rebuild on Linux. labels Apr 24, 2019
@tex tex force-pushed the grip branch 3 times, most recently from 3fab489 to 1b91817 Compare April 25, 2019 20:57
@tex
Copy link
Contributor Author

tex commented Apr 25, 2019

Is there a clean way to solve project names clash?

@aanderse
Copy link
Member

aanderse commented Apr 25, 2019

Oh... I see what is going on... never realized there was a grip already. I feel like grip the gnome application has likely existed much longer than grip the search+indexing tool. I guess you should rename your app? 🤷‍♂️

ping @jtojnar as a gnome expert and all around great person 😄

@jtojnar
Copy link
Member

jtojnar commented Apr 25, 2019

I would suggest choosing grip-search or gripgen attribute for this package. Then remove the old abandoned package. And only rename this package after some cycles.

@tex
Copy link
Contributor Author

tex commented Apr 28, 2019

It is not my application :) I just found it useful in my job. Named attribute to grip-search. Probably no need to remove existing grip - at most if anyone use both there will be clash in names (grip), but I do not think too many people will want to use grip-search and grip together.

@ofborg ofborg bot added 8.has: package (new) This PR adds a new package 11.by: package-maintainer This PR was created by a maintainer of all the package it changes. 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux. and removed 10.rebuild-linux: 0 This PR does not cause any packages to rebuild on Linux. labels May 10, 2019
Copy link
Member

@aanderse aanderse left a comment

Choose a reason for hiding this comment

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

I'm sorry this I haven't got back to this until now. This dropped off my radar.

On top of the suggestions I have made you should probably rename the file from grip.nix to grip-search.nix so it matches the attribute name.

After you're done with the changes please squash all commits into a single commit with message grip-search: init at 0.8.

@tex
Copy link
Contributor Author

tex commented Nov 7, 2019

@aanderse Hi, please see changes.

@tex tex changed the title grip-v0.7: indexed grip grip-0.8: indexed grip Nov 8, 2019
@tex
Copy link
Contributor Author

tex commented Nov 11, 2019

Hi, is there anything else needed to to to get this merged?

@tex tex requested a review from aanderse November 11, 2019 07:45
@Azulinho
Copy link
Contributor

@JohnAZoidberg happy to be left out,
My Cmake skills are not that great,I'd rather focus on stuff I know well.

@tex
Copy link
Contributor Author

tex commented Nov 12, 2019 via email

@tex
Copy link
Contributor Author

tex commented Nov 14, 2019

I tested crosscompile when I first submitted this merge request, but trying it now (with updated nixpkgs) leads to:

[1] milan@nixos> nix-build '<nixpkgs>' -A pkgsCross.mingwW64.meson                                                   /etc/nixos/nixpkgs
warning: Nix search path entry '/etc/nixos/overlay' does not exist, ignoring
error: attribute 'targetPlatform' missing, at /etc/nixos/nixpkgs/pkgs/development/tools/build-managers/meson/default.nix:70:17
(use '--show-trace' to show detailed location information)

I submitted this as bug #73420

@tex
Copy link
Contributor Author

tex commented Nov 14, 2019

I tested crosscompile when I first submitted this merge request, but trying it now (with updated nixpkgs) leads to:

[1] milan@nixos> nix-build '<nixpkgs>' -A pkgsCross.mingwW64.meson                                                   /etc/nixos/nixpkgs
warning: Nix search path entry '/etc/nixos/overlay' does not exist, ignoring
error: attribute 'targetPlatform' missing, at /etc/nixos/nixpkgs/pkgs/development/tools/build-managers/meson/default.nix:70:17
(use '--show-trace' to show detailed location information)

I submitted this as bug #73420

Funny, it works, I tried to cross compile grip (the gui application) which failed. grip-search works.
See originaly it was named grip (and original grip removed), then I renamed to grip-search :D

@ofborg ofborg bot added 10.rebuild-darwin: 1-10 This PR causes between 1 and 10 packages to rebuild on Darwin. and removed 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin. labels Nov 14, 2019
@JohnAZoidberg
Copy link
Member

Works fine on NixOS and I'm happy with the derivation.

When I build it for Windows with: nix-build -A pkgsCross.mingwW64.grip-search, copy result/bin/* to Windows 10 and run it, this happens:

image

Did you perform other steps?

@JohnAZoidberg JohnAZoidberg merged commit c96e556 into NixOS:master Nov 14, 2019
@tex
Copy link
Contributor Author

tex commented Nov 15, 2019

Oh, I tested that crosscompilation when I first created this merge request.
This must be dependency created by #73195 . Fun thing is that I modified grip on master to remove dependency on multithreading and other C++11 things that the #73195 enabled in Nix when crosscompiling. Guess you need to install that missing library into system, it is not dll created by grip.

@tex tex deleted the grip branch November 15, 2019 20:57
@tex
Copy link
Contributor Author

tex commented Nov 15, 2019

Or is it problem of #73195 that it doesn't provide all in runtime required libraries in resulting output?

@Ericson2314
Copy link
Member

@tex We have always lacked a sufficiently good automated mechanism for copying all of the required DLLs into the bin/ directory. But if you build nix-build '<nixpkgs>' -A pkgsCross.mingwW64.threadsCross you can get the DLL and copy it yourself.

dtzWill pushed a commit to dtzWill/nixpkgs that referenced this pull request Nov 16, 2019
grip-0.8: indexed grip

(cherry picked from commit c96e556)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

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-linux: 1-10 This PR causes between 1 and 10 packages 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.

6 participants