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

Unify python executables to use #!/usr/bin/env python shebang #2102

Merged
merged 7 commits into from Jan 3, 2019

Conversation

SaveTheRbtz
Copy link
Contributor

Currently some python executables either do not have shebang or have it hardcoded to #!/usr/bin/python. This is causing some problems on our production systems.

RPM specs currently workaround that problem by seding the resulting files:

bcc/SPECS/bcc.spec

Lines 91 to 94 in dccc4f2

# mangle shebangs
find %{buildroot}/usr/share/bcc/{tools,examples} -type f -exec \
sed -i -e '1 s|^#!/usr/bin/python$|#!'%{__python}'|' \
-e '1 s|^#!/usr/bin/env python$|#!'%{__python}'|' {} \;

This diff:

  • rewrites shebangs across all executable python files to: #!/usr/bin/env python.
  • removes the mangling code from RPM specs.
  • adds a travis-ci test to verify that all python executables continue to have proper shabangs.

PS. It seems like the travis build is not enabled for bcc: https://travis-ci.org/iovisor/bcc

@yonghong-song
Copy link
Collaborator

[buildbot, test this please]

1 similar comment
@yonghong-song
Copy link
Collaborator

[buildbot, test this please]

@yonghong-song
Copy link
Collaborator

The change looks good. But there is a timeout for py_test_tools_smoke with lib/ustat.py on fc25/26/27. Not sure why yet.

@yonghong-song
Copy link
Collaborator

[buildbot, test this please]

@yonghong-song
Copy link
Collaborator

I have no idea why timeout happens with ustat.py in test_tools_smoke.py. I tried on my local fc27 and it works fine. Maybe the test VM has higher load? Maybe you could try to increase timeout value to see whether it works or not?

diff --git a/tests/python/test_tools_smoke.py b/tests/python/test_tools_smoke.py
index 211dbdb..14f72ee 100755
--- a/tests/python/test_tools_smoke.py
+++ b/tests/python/test_tools_smoke.py
@@ -363,7 +363,7 @@ class SmokeTests(TestCase):
 
     @skipUnless(kernel_version_ge(4,4), "requires kernel >= 4.4")
     def test_ustat(self):
-        self.run_with_duration("lib/ustat.py 1 1")
+        self.run_with_duration("lib/ustat.py 1 1", 20)
 
     @skipUnless(kernel_version_ge(4,4), "requires kernel >= 4.4")
     def test_uthreads(self):

@yonghong-song
Copy link
Collaborator

@SaveTheRbtz usdt.py fix has been merged. Now you can rebase and resubmit. Thanks!

@SaveTheRbtz
Copy link
Contributor Author

@yonghong-song can you ping the buildbot? he is ignoring my pleas for some reason =)

@yonghong-song
Copy link
Collaborator

[buildbot, test this please]

@yonghong-song
Copy link
Collaborator

Just pinged and the test started!

@SaveTheRbtz
Copy link
Contributor Author

fc27 failed on:

// the program slept one second between perf_event attachment and detachment
// in the above, so the enabled counter should be 1000000000ns or
// more. But in reality, most of counters (if not all) are 9xxxxxxxx,
// and I also saw one 8xxxxxxxx. So let us a little bit conservative here.
REQUIRE(counter.enabled >= 800000000);

with:

3: /home/fedora/jenkins/workspace/bcc-pr/label/fc27/tests/cc/test_perf_event.cc:139: FAILED:
3:   REQUIRE( counter.enabled >= 800000000 )
3: with expansion:
3:   794617069 (0x2f5ce4ed)
3:   >=
3:   800000000 (0x2faf0800)

Seems like counters are not deterministic there.

@yonghong-song
Copy link
Collaborator

Right. Let me do a test again.

@yonghong-song
Copy link
Collaborator

[buildbot, test this please]

@SaveTheRbtz
Copy link
Contributor Author

SaveTheRbtz commented Jan 3, 2019

@yonghong-song Ok, now it is a fc28 with:

removing 'bcc-0.7.0' (and everything under it)
removing 'bcc-0.7.0' (and everything under it)
error: [Errno 2] No such file or directory: 'bcc-0.7.0'
make[2]: *** [src/python/CMakeFiles/bcc_py_python2.dir/build.make:62: src/python/dist-python2/bcc-0.7.0.tar.gz] Error 1
make[2]: *** Deleting file 'src/python/dist-python2/bcc-0.7.0.tar.gz'
make[1]: *** [CMakeFiles/Makefile2:650: src/python/CMakeFiles/bcc_py_python2.dir/all] Error 2
make[1]: *** Waiting for unfinished jobs....

seems like the resulting makefile is racy (it builds bcc_py_python2 and bcc_py_python3 in the same dir) but still is ran in parallel:

+ make -j2

@yonghong-song
Copy link
Collaborator

Right. We need to fix this as well. It happens quite frequently.

@yonghong-song
Copy link
Collaborator

[buildbot, test this please]

@yonghong-song yonghong-song merged commit 777e802 into iovisor:master Jan 3, 2019
@SaveTheRbtz
Copy link
Contributor Author

Yey! Thanks, will wait for a release now (no rush).

As for the build error something fishy is going on as it does generate same targets twice:

[ 17%] Generating dist-python2/bcc-0.7.0.tar.gz
...
[ 18%] Generating dist-python2/bcc-0.7.0.tar.gz

and indeed generated makefile for the python3 distribution (src/python/CMakeFiles/bcc_py_python3.dir/build.make) also generates python2 archive:

src/python/dist-python3/bcc-0.7.0.tar.gz: ../src/python/bcc/__init__.py
src/python/dist-python3/bcc-0.7.0.tar.gz: src/python/setup.py
src/python/dist-python3/bcc-0.7.0.tar.gz: src/python/dist-python2/bcc-0.7.0.tar.gz
    @$(CMAKE_COMMAND) -E cmake_echo_color --switch=$(COLOR) --blue --bold --progress-dir=/Users/rbtz/c0d3/bcc/build/CMakeFiles --progress-num=$(CMAKE_PROGRESS_1) "Generating dist-python3/bcc-0.7.0.tar.gz"
    cd /Users/rbtz/c0d3/bcc/build/src/python && python3 setup.py sdist --dist-dir dist-python3

src/python/dist-python2/bcc-0.7.0.tar.gz: ../src/python/bcc/__init__.py
src/python/dist-python2/bcc-0.7.0.tar.gz: src/python/setup.py
    @$(CMAKE_COMMAND) -E cmake_echo_color --switch=$(COLOR) --blue --bold --progress-dir=/Users/rbtz/c0d3/bcc/build/CMakeFiles --progress-num=$(CMAKE_PROGRESS_2) "Generating dist-python2/bcc-0.7.0.tar.gz"
    cd /Users/rbtz/c0d3/bcc/build/src/python && python2 setup.py sdist --dist-dir dist-python2

Something is likely wrong with #1951

@SaveTheRbtz SaveTheRbtz deleted the fix_shebangs branch January 3, 2019 22:57
palexster pushed a commit to palexster/bcc that referenced this pull request Jul 7, 2019
…sor#2102)

* fixed shebangs in tools (and lib)

* fixed shebangs in examples

* do not mangle shebangs in rpm spec

* renamed style-check.sh to c-style-check.sh

* factored out python linter to a separate file

* added shebang validation to the py-style-check

* added shebangs to all python executables
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants