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

viztracer C code requires C99 #227

Closed
comzyh opened this issue May 1, 2022 · 3 comments · Fixed by #228
Closed

viztracer C code requires C99 #227

comzyh opened this issue May 1, 2022 · 3 comments · Fixed by #228

Comments

@comzyh
Copy link
Contributor

comzyh commented May 1, 2022

Reproduce:

In the viztracer directory

docker run --rm -it  -v $(pwd):/root/viztracer centos/python-36-centos7  pip install /root/viztracer

Last few lines of error:

gcc -pthread -Wno-unused-result -Wsign-compare -DDYNAMIC_ANNOTATIONS_ENABLED=1 -DNDEBUG -O2 -g -pipe -Wall -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector-strong --param=ssp-buffer-size=4 -grecord-gcc-switches -m64 -mtune=generic -D_GNU_SOURCE -fPIC -fwrapv -I/opt/rh/rh-python36/root/usr/include -O2 -g -pipe -Wall -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector-strong --param=ssp-buffer-size=4 -grecord-gcc-switches -m64 -mtune=generic -D_GNU_SOURCE -fPIC -fwrapv -fPIC -I/opt/rh/rh-python36/root/usr/include/python3.6m -c src/viztracer/modules/snaptrace.c -o build/temp.linux-x86_64-3.6/src/viztracer/modules/snaptrace.o -Werror
src/viztracer/modules/snaptrace.c: In function ‘snaptrace_tracefunc’:
src/viztracer/modules/snaptrace.c:306:17: error: ‘for’ loop initial declarations are only allowed in C99 mode
                 for (int i = 0; i < length; i++) {
                 ^
src/viztracer/modules/snaptrace.c:306:17: note: use option -std=c99 or -std=gnu99 to compile your code
src/viztracer/modules/snaptrace.c:337:21: error: ‘for’ loop initial declarations are only allowed in C99 mode
                     for (size_t i = 0; i < sizeof(curr_task_getters)/sizeof(curr_task_getters[0]); i++) {
                     ^
error: command 'gcc' failed with exit status 1
----------------------------------------
 Command "/opt/app-root/bin/python3 -u -c "import setuptools, tokenize;__file__='/tmp/pip-n3jn03ib-build/setup.py';f=getattr(tokenize, 'open', open)(__file__);code=f.read().replace('\r\n', '\n');f.close();exec(compile(code, __file__, 'exec'))" install --record /tmp/pip-83jv757d-record/install-record.txt --single-version-externally-managed --compile --install-headers /opt/app-root/include/site/python3.6/viztracer" failed with error code 1 in /tmp/pip-n3jn03ib-build/

I will raise a PR later.

comzyh added a commit to comzyh/viztracer that referenced this issue May 1, 2022
@comzyh comzyh changed the title viztracer C code required C99 viztracer C code requires C99 May 1, 2022
@comzyh
Copy link
Contributor Author

comzyh commented May 1, 2022

I found this issue because centos 7 is still using pip 9.0.1 and it only supports manylinux1. viztracer only publish manylinux2014 wheel
https://github.com/gaogaotiantian/viztracer/pull/210/files

@gaogaotiantian
Copy link
Owner

There are a couple of aspects in this issue.

First of all, pip is not bound to OS. You can and probably should upgrade your pip version.

Also, I noticed you are using python3.6, which was EOL last year. I would highly suggest using a more recent version. Even though viztracer still builds 3.6 wheels and runs tests on 3.6, it might be deprecated at any time.

You are right about manylinux1, but manylinux1 was EOL too. They dropped support for manylinux1 this year.

Overall, your extra argument change might be valid and I will merge the PR once it passes the test, but I do think the fundamental reason that you had this issue was that you were using a lot of deprecated things.

@comzyh
Copy link
Contributor Author

comzyh commented May 2, 2022

Agree. I couldn't even reproduce this issue on my personal PC before I found that docker image. But you know, some companies still have EOL software in some environments. 😆
The only issue here is the assumption of the default value of C standard, so I raise that PR.

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 a pull request may close this issue.

2 participants