Not sure what's wrong with the test.

@FRidh is the PR 👍 for you now?

I'm seeing python_requires='>=3.6', in the setup.py so we need to disable this for interpreter versions older than 3.6. (pythonAtLeast is useful here)

Also looks like it might want solc in PATH https://github.com/trailofbits/slither/blob/99767a1bb46e59c86f6b69933b56a251e6a89004/README.md#configuration

@worldofpeace how would you do this? I don't see a buildPython36Package function.

@worldofpeace added disabled = pythonOlder "3.6"; - is that the right way?

Alternatively, I could use disabledIf in python-packages.nix.

Is this ready to be merged?

Is this ready to be merged?

This issue still stands

Also looks like it might want solc in PATH https://github.com/trailofbits/slither/blob/99767a1bb46e59c86f6b69933b56a251e6a89004/README.md#configuration

I'm not familiar with this software, so I'm not sure if this is appropriate, but this can be achieved with:

nativeBuildInputs = [ makeWrapper ];

postFixup = ''
  wrapProgram $out/bin/slither \
    --prefix PATH : "${stdenv.lib.makeBinPath [ solc ]}"
'';

@worldofpeace good catch, I had missed that comment.

Why is makeBinPath preferred to something like "${solc}/bin"?

Why is makeBinPath preferred to something like "${solc}/bin"?

makeBinPath basically does that.

It's well commented here

Why is makeBinPath preferred to something like "${solc}/bin"?

makeBinPath basically does that.

It's well commented here

nixpkgs/lib/strings.nix

Line 163 in 475d653
makeBinPath = makeSearchPathOutput "bin" "bin";

Yeah, but because the implementation is so simple, I'm not sure what the benefit is of hiding it behind a function :)

Anyway, this is probably OT.

This program has no python tests so can you add?

# Has no tests
doCheck = false

Also since we're going with calling this slither-analyzer we need to change the commit msg.

This program has no python tests so can you add?

Does it make sense? If they're added later on, we need to remember to set doCheck = false; does leaving it as is create any inconvenience?

This program has no python tests so can you add?

Does it make sense? If they're added later on, we need to remember to set doCheck = false; does leaving it as is create any inconvenience?

I'm not sure what you mean.
doCheck value controls if the check phase is executed.
By default tests are run in python (in nixpkgs) and because they're no tests we are running the checkPhase unnecessarily. So we should disable the execution of the checkPhase.

I'm not sure what you mean.

I mean that if tests are added in a future release, someone has to notice and remember to remove doCheck = false;.

Leaving doCheck to its default, true, seems to have no negative side-effects, so why change it?

I'm not sure what you mean.

I mean that if tests are added in a future release, someone has to notice and remember to remove doCheck = false;.

Leaving doCheck to its default, true, seems to have no negative side-effects, so why change it?

I would hope that our reviewers would be astute to catch that.
Additionally this is done in many other packages that don't have tests.

I respectfully disagree. I don't see the benefit of disabling the tests when there are none, as it brings virtually no benefits while exposing the package (and nixpkgs) to unnecessary risk.

But if you feel strongly about it, I'll add it. I am just not convinced it's a good idea.

I respectfully disagree. I don't see the benefit of disabling the tests when there are none, as it brings virtually no benefits while exposing the package (and nixpkgs) to unnecessary risk.

But if you feel strongly about it, I'll add it. I am just not convinced it's a good idea.

I don't feel particularly strong about it, but just for consistency with other packages like this we'll need to disable them.

@FRidh Any comments you could contribute to this would be interesting since I've seen you instruct people to do this.

Merged in 6283971