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

Datefudge #16610

Merged
merged 1 commit into from Aug 12, 2016
Merged

Datefudge #16610

merged 1 commit into from Aug 12, 2016

Conversation

leenaars
Copy link
Contributor

@leenaars leenaars commented Jun 29, 2016

Motivation for this change

This pull request is not meant to be merged without further work.

Datefudge is a tool for faking system time used by e.g. recent gnutls for testing what happens at certain times. There is an updated version of gnutls currently in nixpkgs unstable, which breaks due to the absence of datefudge.

Given that it is a simple but useful tool anyway (it is used by other applications like gitlab during tests as well), I'm trying to add it to nixpkgs. Currently the datefudge library gets builds and the application gets installed. It is a small wrapper app written in shell, which calls the compiled library.

Subsequently however one gets a segmentation error when running a test e.g.
something like:

  datefudge "2006-09-23" date -u +%s 

The tool is 13 years old, and runs apparently flawlessly on Debian, Arch, etc.

Currently gnutls, and everything that depends on it, might be broken in unstable without datefudge. Not a happy thought. Is there anyone that would have the good heart to have a look at what could be wrong with the datefudge "recipe"?

Things done
  • Tested using sandboxing
    (nix.useChroot on NixOS,
    or option build-use-chroot in nix.conf
    on non-NixOS)
  • Built on platform(s)
    • NixOS
    • OS X
    • Linux
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nox --run "nox-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Fits CONTRIBUTING.md.

@mention-bot
Copy link

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

@@ -0,0 +1,40 @@
{ stdenv, fetchgit, dpkg }:
Copy link
Member

Choose a reason for hiding this comment

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

I don't think dpkg is actually needed?

@matthewbauer
Copy link
Member

matthewbauer commented Jun 29, 2016

That test is working for me on NixOS:

$ cd ~/Projects/nixpkgs
$ git remote add leenaars 
$ git fetch leenaars datefudge
$ git checkout datefudge
$ nix-build -A datefudge
$ ./result/bin/datefudge 2006-09-23 date
Sat Sep 23 00:00:00 UTC 2006

What distribution is Nix installed on? Perhaps it has something to do with permissions.

@leenaars
Copy link
Contributor Author

@matthewbauer: it works for me on NixOS too, so at least there we could get things working again.
The segfault happens on Debian.

Indeed one of the things the Nix recipe does is change the installer which involves a small permission related snippet.

I guess the dpkg can go - in the Makefile this only serves to get the version number from the Debian metadata also included.

@vcunat
Copy link
Member

vcunat commented Jun 30, 2016

As the PR is, it seems not to fix the expiration problem. I suppose it will be best to just disable that single test, at least for now.

vcunat added a commit that referenced this pull request Jun 30, 2016
- disable a test that started failing due to date expiration, see #16610
- bash doesn't need adding
- defining patchPhase was overriding passed postPatch and patches
@vcunat
Copy link
Member

vcunat commented Jun 30, 2016

I disabled that test on staging: 17faf91.

@leenaars
Copy link
Contributor Author

leenaars commented Jul 1, 2016

Removed the dpkg.

@leenaars
Copy link
Contributor Author

leenaars commented Aug 9, 2016

@vcunat, @matthewbauer:

It was indeed a rights issue. Fixed. Should be ready to merge now.
I've tested successfully on Debian and NixOS.

Should I try to reenable this test in GnuTLS?

@matthewbauer
Copy link
Member

@leenaars We should definitely do that but probably in a separate PR on staging so we don't have massive rebuild issues again.

@lucabrunox lucabrunox merged commit 909b036 into NixOS:master Aug 12, 2016
@lucabrunox
Copy link
Contributor

I've heard this was ready to merge :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants