On Wed, 13 Sep 2017 10:10:39 +0200
ludo@gnu.org (Ludovic Courtès) wrote:

> Hi Eric,
> 
> Eric Bavier <bavier@member.fsf.org> skribis:
> 
> > * gnu/packages/password-utils.scm (john-the-ripper-jumbo): New variable.  
> 
> [...]
> 
> > +       `(#:configure-flags
> > +         (list (string-append
> > +                "CFLAGS="  
> 
> Is this to clear CFLAGS?  We should make sure to get “-O2 -g”.

The macro definitions below must be put in CFLAGS, but yes, -O2 should
be included.

> 
> > +                "-DJOHN_SYSTEMWIDE=1 "
> > +                "-DJOHN_SYSTEMWIDE_EXEC='\"" %output "/libexec/john\"' "
> > +                "-DJOHN_SYSTEMWIDE_HOME='\"" %output "/share/john\"'")
> > +               "--disable-native-tests"
> > +               "--disable-native-macro")  
> 
> Perhaps add a comment on these two flags.

Sure.  John supports a "cpu fallback" feature to match, at runtime, the
current processor with an executable appropriate for its supported
instruction set.  This involves configuring and building multiple times
with different optimization flags.  I wanted to implement this later
after getting the rest of things working, so these two flags turn off
the "-march=native" flag and instruction set discovery during
'configure'.

> 
> > +         #:tests? #f ;tests assume /etc/passwd has a pw_dir for the build user  
> 
> /etc/passwd in the build environment has this line (see
> nix/libstore/build.cc:1859):
> 
>   nixbld:x:%1%:%2%:Nix build user:/:/noshell
> 
> Isn’t it sufficient to have “/” as the home directory?

It seems the build user noes not have sufficient permissions to create a
directory there?  The tests want to create "~/.john" (specifically
"//.john").

> 
> > +      (description "John the Ripper is a fast password cracker.  Its primary
> > +purpose is to detect weak Unix passwords.  Besides several crypt(3) password  
> 
> Rather @code{crypt}.

ok.

> 
> Otherwise LGTM, thank you!

Thanks, updated patch attached,
`~Eric