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

freenet: rewrite wrapper to not depend on PATH #12107

Merged
merged 1 commit into from
Jan 13, 2016
Merged

Conversation

VShell
Copy link
Contributor

@VShell VShell commented Jan 3, 2016

Rewrite wrapper depended on $PATH containing bash and coreutils, so I've merged the wrapper and run script together and ensured that they don't. Can @spacekitteh test this, I've so far only tested that it starts.

@mention-bot
Copy link

By analyzing the blame information on this pull request, we identified @doublec, @viric and @edolstra to be potential reviewers

@spacekitteh
Copy link
Contributor

how to test it?

@doublec
Copy link
Contributor

doublec commented Jan 5, 2016

I can test but the original was working for me. I'll report back here shortly if things still work. Basic steps are run freenet:

$ freenet

Then visit http:/127.0.0.1:8888 in the browser and see if it shows the freenet installation wizard.

@spacekitteh
Copy link
Contributor

No I mean, how to grab this specific pull request and install it

@VShell
Copy link
Contributor Author

VShell commented Jan 5, 2016

@doublec I believe @spacekitteh was trying to run it under systemd, I assume as part of NixOS.

@spacekitteh I think, assuming you have Freenet as a systemd service somewhere in your config:

git clone --depth 1 https://github.com/VShell/nixpkgs.git
NIX_PATH=nixpkgs=`pwd`/nixpkgs nixos-rebuild switch

Should do it.

@doublec
Copy link
Contributor

doublec commented Jan 5, 2016

Seems broken with this commit. After installation running freenet fails due to freenet_jars not being substituted in the freenet script. It looks like this should be freenet inside freenetWrapper, not freenet-jars. If I replace that, reinstall, then it runs and works fine.

@VShell
Copy link
Contributor Author

VShell commented Jan 5, 2016

@doublec Fixed, sorry about that.

@doublec
Copy link
Contributor

doublec commented Jan 5, 2016

Thanks, I confirm that works running freenet standalone. I don't know how to get it running as a systemd service on NixOS but if someone can provide pointers I can test that.

url = https://github.com/freenet/fred;
rev = "refs/tags/${version}";
sha256 = "1b6e6fec2b9a729d4a25605fa142df9ea42e59b379ff665f580e32c6178c9746";
};
Copy link
Member

Choose a reason for hiding this comment

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

The diff of the section above looks really strange. What is going on there? Did you add tabs, maybe?

Generally speaking, please don't commit white-space changes as those make the diffs unnecessarily long and hard to read.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

What I've done here is moved a "base" derivation into the let block, and the main derivation is a wrapper around that. I've since figured out a way I can merge them into one derivation again by using substituteAll from stdenv directly, which I will probably do this evening, which would clear up the diff a lot.

@peti peti added 0.kind: enhancement Add something new 8.has: package (update) This PR updates a package to a newer version 9.needs: clean-up labels Jan 5, 2016
@jagajaga
Copy link
Member

Ping

peti added a commit that referenced this pull request Jan 13, 2016
freenet: rewrite wrapper to not depend on PATH
@peti peti merged commit e4cc370 into NixOS:master Jan 13, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
0.kind: enhancement Add something new 8.has: package (update) This PR updates a package to a newer version 9.needs: clean-up
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants