-
-
Notifications
You must be signed in to change notification settings - Fork 14.5k
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
wait-for-it: init at unstable-2020-02-05 #89493
Conversation
Not sure what commit message to use given there is no version. |
Hi and welcome to Nixpkgs 🎉 :). Thank you for your contribution. Since the package you'd like to add to the collection is a pure Bash script, is there a particular reason you can't just clone the project's directory and link it to somewhere in your |
Hi there, That's a great question. I think my reasoning here is that it's a very popular bash script for a very popular need: waiting for arbitrary services to start. This is especially useful when running Nix inside docker: it unlocks the ability to run more than one process in a single image. It's such a popular task that it's even in the docker documentation, with a fragile, non-standard hack to do so: https://docs.docker.com/config/containers/multi-service_container/ My use case is the above, and just as you suggest, I'm basically using a checked-in derivation locally and doing with import <nixpkgs> { };
stdenv.mkDerivation rec {
name = "wait-for-it-${version}";
version = "1.0";
src = fetchFromGitHub {
owner = "vishnubob";
repo = "wait-for-it";
rev = "c096cface5fbd9f2d6b037391dfecae6fde1362e";
sha256 = "1k1il4bk8l2jmfrrcklznc8nbm69qrjgxm20l02k01vhv2m2jc85";
};
installPhase =
''
mkdir -p $out/bin
cp ./wait-for-it.sh $out/bin
'';
buildInputs = [];
} So to your point: if you think of this as a "binary" that waits for services, I argue it's very useful. The language it's written in being orthogonal. In a way, no different than python packages that we have in nix that require no compilation. Why don't we use pip or clone those as well? On the other hand, if you still oppose - which I completely understand - I'd love some guidance and ways to contribute towards making it even more lightweight to "source" shell scripts from GitHub repos without needing files like the one I pasted above. I love the nix infrastructure and plumbing, so a single one liner like importScript or something to add to repos would go a long way. Thanks for all the work on nix. No joke, it's changed my life. |
Not that popular according to: https://repology.org/projects/?search=wait-for-it&maintainer=&category=&inrepo=¬inrepo=&repos=&families=&repos_newest=&families_newest= Also, what bothers me is that it seems unmaintained according to the number of open issues and PRs.
Doesn't sound more useful for Nix vs other distros.
Because it's a pain to handle Python deps without Nix. Consider how many things
Dealing with Bash scripts is super easy compared to doing that for Python scripts without Nixpkgs' Python ecosystem. Note that mere Python scripts with no non-standard dependencies are sometimes rejected as well.
Definitely we are lacking such guidelines and that was the purpose of that discourse thread. I think most people consider it easy enough to |
So, for your popularity metric, I believe it measures projects that refer to it or depend on it somehow. It's low precisely because people are downloading and checking this into their repo. Bash does not have a standout package manager[1]. So I guess since I think nix is so useful, it should embrace the ability to package bash. The fact that the coreutils etc are already standardized inside nix is a very nice benefit. Regarding maintainability: I guess that's subjective. It's got recent commits and a few issues triaged. But yea, it's not super active. I suppose where you see "unmaintained" I see "finished". Regarding nix/docker: in other distros and docker image, the root images are highly specialized. Yes you can install and run node inside the python3.8 image, or vice versa. The NixOS image is amazing for having the same environment in production that you have in development. This is something fewer people realize and I plan to write about. Also totally a tangent, but I'm on a nix bender right now.[2]. So it's very easy to have your Node and Python servers inside the same docker image (negating the need for full-on orchestration). Regarding using git submodules: I think that's a personal thing. I'm not a huge fan. And I'm on the quixotic quest to achieve single So to summarize, all kind of related:
[1] I don't really see https://github.com/bpkg/bpkg as having taken off. [2] Images do tend to be much larger though. |
{ stdenv, lib, fetchFromGitHub }: | ||
|
||
stdenv.mkDerivation rec { | ||
name = "wait-for-it"; |
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.
name = "wait-for-it"; | |
pname = "wait-for-it"; | |
version = "unstable-2020-02-05"; |
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.
Agreed, but changed to
pname = "wait-for-it-unstable";
version = "2020-02-05";
so that it matches the Nix notion of derivation name/version:
nix-repl> builtins.parseDrvName "wait-for-it-unstable-2020-02-05"
{ name = "wait-for-it-unstable"; version = "2020-02-05"; }
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.
Would this affect say, nix-shell -p
? I'd assume users what the least surprise here, and typing nix-shell -p wait-for-it-unstable
is certainly more obscure than nix-shell -p wait-for-it
.
Please let me know!
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.
-p
uses attribute names so it behaves correctly. Only nix-env
without -A
will be problematic. I suggested using yyyy-mm-dd-unstable
version format to sidestep this elsewhere.
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.
See also #88023 (comment)
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.
@jtojnar I think the solution depends on the situation. Quite a few packages end up in Nixpkgs before upstream have ever made a versioned release. If the upstream then makes a release 1.0 we'd typically want to track that but then it is unfortunate to have, for example, foo-2020-06-10-unstable
since 2020-06-10-unstable > 1.0 using version comparison. Then nix-env --upgrade
would silently leave the old "unstable" version as-is. If instead the package name changes from foo-unstable
to foo
perhaps you'd at least get an error?
If the intent is to swap the package over to a stable version if one ever appears then perhaps some thing like foo-0.0.0-nix20200610
would be more suitable? And foo-unstable-2020-06-10
could then be reserved for packages really intended to forever track the unstable branch of foo.
I think that until we have inclusion policy this should qualify.
We can always just remove it when it breaks and nobody steps in to fix it. |
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.
Commit should be named following our guidelines.
Note a Debian package exists for it.
Thanks for your help! Still learning. |
Your PR should have eventually two commits:
Updating the PR title is nice but it's not a must. |
76b67b9
to
63e94ca
Compare
Hi all, I think this is ready to go. Please let me know and many thanks for all your help. |
Hi! First of all, thank you for the contribution! Sorry for the nitpick, but there should be 2 commits: one adding you as a maintainer and another adding the package. You can just run this:
in a clean checkout. |
63e94ca
to
846e2fa
Compare
Hi there, Not nitpicking at all! I went off of this random PR and they only had 1 commit, so I figured maybe that was ok: #57893 I restructured it as two commits. For posterity, these were the steps to do it cleanly while staying on the branch:
Addressing your other comments inline. Thanks for the feedback. |
846e2fa
to
8188a76
Compare
@balsoft : all done. Thanks again. |
bfb1e34
to
0bdbba1
Compare
Setting phases should be avoided and is really only for generic builders. Much better to pass things like |
hi @balsoft , is there anything else needed for this to go in? Thanks again for your help. |
Well it does have dependencies: netcat. It should probably be wrapped to include nc in its $PATH. EDIT: Sorry, this is only required when running from busybox. |
{ stdenvNoCC, lib, fetchFromGitHub }: | ||
|
||
stdenvNoCC.mkDerivation rec { | ||
pname = "wait-for-it-unstable"; |
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.
pname = "wait-for-it-unstable"; | |
pname = "wait-for-it"; |
rev = "c096cface5fbd9f2d6b037391dfecae6fde1362e"; | ||
sha256 = "1k1il4bk8l2jmfrrcklznc8nbm69qrjgxm20l02k01vhv2m2jc85"; | ||
}; | ||
phases = [ "unpackPhase" "installPhase" "fixupPhase" ]; |
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.
phases = [ "unpackPhase" "installPhase" "fixupPhase" ]; |
meta = { | ||
homepage = "https://github.com/vishnubob/wait-for-it"; | ||
description = | ||
"Pure bash script to test and wait on the availability of a TCP host and port"; | ||
license = lib.licenses.mit; | ||
maintainers = [ lib.maintainers.silviogutierrez ]; | ||
}; |
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.
meta = { | |
homepage = "https://github.com/vishnubob/wait-for-it"; | |
description = | |
"Pure bash script to test and wait on the availability of a TCP host and port"; | |
license = lib.licenses.mit; | |
maintainers = [ lib.maintainers.silviogutierrez ]; | |
}; | |
meta = with lib; { | |
homepage = "https://github.com/vishnubob/wait-for-it"; | |
description = "Pure bash script to test and wait on the availability of a TCP host and port"; | |
license = licenses.mit; | |
maintainers = with maintainers; [ silviogutierrez ]; | |
}; |
This is a semi-automatic executed nixpkgs-review which does not build all packages (e.g. lumo, tensorflow or pytorch) Result of 1 package built:
|
I marked this as stale due to inactivity. → More info |
Closing due to inactivity from author. |
Motivation for this change
Add wait-for-it package, a zero dependency battle-tested wait to wait for a service to be available.
Please note, the repo, while very popular, does not seem to use the concept of tags, versions, or releases. So I just pinned it to the latest build from master.
Things done
sandbox
innix.conf
on non-NixOS linux)nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
./result/bin/
)nix path-info -S
before and after)