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

speedometer: init at 2.8 #33627

Merged
merged 2 commits into from Nov 17, 2018
Merged

speedometer: init at 2.8 #33627

merged 2 commits into from Nov 17, 2018

Conversation

Baughn
Copy link
Contributor

@Baughn Baughn commented Jan 8, 2018

Motivation for this change

Adds a useful bandwidth monitoring CLI tool.

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option build-use-sandbox in nix.conf on non-NixOS)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • 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.

@bjornfor
Copy link
Contributor

bjornfor commented Jan 8, 2018

Something's up with how the 'speedometer' program is made (the one without the .py extension):

$ ./result/bin/speedometer
Traceback (most recent call last):
  File "/nix/store/rya13rvcr49p2hsnv099143dh7rwam6m-speedometer-2.8/bin/.speedometer-wrapped", line 8, in <module>
    from speedometer import console
  File "/nix/store/rya13rvcr49p2hsnv099143dh7rwam6m-speedometer-2.8/bin/speedometer.py", line 2
    export PATH='/nix/store/6yb5rvr6rvgvx8ylpchwz808djfw07rb-python-2.7.14/bin:/nix/store/rya13rvcr49p2hsnv099143dh7rwam6m-speedometer-2.8/bin:/nix/store/i5ig950zqwln4qi8wfaivgp3bjzzl3jz-python2.7-setuptools-38.2.5/bin'${PATH:+':'}$PATH
              ^
SyntaxError: invalid syntax

@Baughn
Copy link
Contributor Author

Baughn commented Jan 8, 2018

Agreed. That happened when I rebased it on top of master, however; it worked perfectly on the installed commit. I'm trying to track it down.

@Baughn
Copy link
Contributor Author

Baughn commented Jan 8, 2018

The script is broken by commit d945b3e... somehow.

@Baughn
Copy link
Contributor Author

Baughn commented Jan 8, 2018

Two wrapper scripts. One of them works:

#! /nix/store/ski5znw2b8bj5qnsncrcq6128hkrwgvq-bash-4.4-p12/bin/bash -e
export PATH='/nix/store/9n2npaz7xy8b4q0a8k28nr3sjzfy625i-python-2.7.14/bin:/nix/store/jw95yh771yqpqkzwr1f6vkwm2790cknv-speedometer-2.8/bin:/nix/store/vq33bxxy9hyv2cj7w0sxlhg3rj0px0kc-python2.7-setuptools-36.7.1/bin'${PATH:+':'}$PATH
export PYTHONNOUSERSITE='true'
exec -a "$0" "/nix/store/jw95yh771yqpqkzwr1f6vkwm2790cknv-speedometer-2.8/bin/.speedometer-wrapped"  "${extraFlagsArray[@]}" "$@"

And one of them doesn't.

#! /nix/store/nkq0n2m4shlbdvdq0qijib5zyzgmn0vq-bash-4.4-p12/bin/bash -e
export PATH='/nix/store/6yb5rvr6rvgvx8ylpchwz808djfw07rb-python-2.7.14/bin:/nix/store/rya13rvcr49p2hsnv099143dh7rwam6m-speedometer-2.8/bin:/nix/store/i5ig950zqwln4qi8wfaivgp3bjzzl3jz-python2.7-setuptools-38.2.5/bin'${PATH:+':'}$PATH
export PYTHONNOUSERSITE='true'
exec -a "$0" "/nix/store/rya13rvcr49p2hsnv099143dh7rwam6m-speedometer-2.8/bin/.speedometer-wrapped"  "${extraFlagsArray[@]}" "$@"

As far as I can tell, they're identical, but bash fails out at the export PATH.

@Baughn
Copy link
Contributor Author

Baughn commented Jan 8, 2018

...wait a second, that's not bash. Why is Python attempting to execute a bash script?

@Baughn
Copy link
Contributor Author

Baughn commented Jan 8, 2018

Okay! I think I've figured it out. In short...

Speedometer, upstream, ships with only one meaningful file: speedometer.py, which is what should be packaged as an application. The application is named speedometer. So far, so good, but buildPythonApplication creates two additional files: .speedometer-wrapped and speedometer. The former is a python script wrapping speedometer.py, the latter is a bash script wrapping .speedometer-wrapped.

(At least, I'm assuming that's what does it. Something seems to have parsed speedometer.py in a complex way in order to create it, which is worrying.)

.speedometer-wrapped runs from speedometer import console, and that's where it all goes pear-shaped. Python imports prefer files without extension to .py files, so it attempts to import the bash script, but it prefers .pyc files to everything else -- which is why this worked prior to commit d945b3e.

I don't think name clashes of this sort are uncommon, so probably that commit should be reverted until the bug is fixed. @FRidh, thoughts?

@FRidh FRidh self-assigned this Jan 9, 2018
@FRidh
Copy link
Member

FRidh commented Jan 9, 2018

The setup.py is incorrect:

    35     'scripts': ['speedometer.py'],
    36     'entry_points': {
    37         'console_scripts': ['speedometer = speedometer:console'],},

They create a console script even though the file is already marked as to being installed as a script.

@Baughn
Copy link
Contributor Author

Baughn commented Jan 9, 2018

Ah, I see. I'll report it upstream. :)

@FRidh
Copy link
Member

FRidh commented Jan 20, 2018

@Baughn are you going to fix this PR or wait for a new release?

@Baughn
Copy link
Contributor Author

Baughn commented Jan 21, 2018

Well, I was hoping for a quick fix from the author, but he seems to be AWOL. Not sure what to do now... fork the repo, maybe.

@FRidh
Copy link
Member

FRidh commented Feb 18, 2018

For in here you could remove the console script line.

@c0bw3b
Copy link
Contributor

c0bw3b commented Nov 17, 2018

@GrahamcOfBorg build speedometer

@GrahamcOfBorg
Copy link

Success on x86_64-linux (full log)

Attempted: speedometer

Partial log (click to expand)

writing dependency_links to Speedometer.egg-info/dependency_links.txt
reading manifest file 'Speedometer.egg-info/SOURCES.txt'
writing manifest file 'Speedometer.egg-info/SOURCES.txt'
running build_ext

----------------------------------------------------------------------
Ran 0 tests in 0.000s

OK
/nix/store/3hajywdmsclsz62v3m1yk875pdxinlpk-speedometer-2.8

@GrahamcOfBorg
Copy link

Success on aarch64-linux (full log)

Attempted: speedometer

Partial log (click to expand)

writing dependency_links to Speedometer.egg-info/dependency_links.txt
reading manifest file 'Speedometer.egg-info/SOURCES.txt'
writing manifest file 'Speedometer.egg-info/SOURCES.txt'
running build_ext

----------------------------------------------------------------------
Ran 0 tests in 0.000s

OK
/nix/store/0qs8pc83wln5m8vrqdk77a2a63g4br0d-speedometer-2.8

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