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

XRT failing to compile on Ubuntu 22.04 #6640

Closed
keryell opened this issue Apr 27, 2022 · 12 comments · Fixed by #6737
Closed

XRT failing to compile on Ubuntu 22.04 #6640

keryell opened this issue Apr 27, 2022 · 12 comments · Fixed by #6737
Assignees

Comments

@keryell
Copy link
Member

keryell commented Apr 27, 2022

Besides mild problem #6098 there are some failures due to warnings as errors

[...]
/home/rkeryell/Xilinx/Projects/Vitis/XRT/src/runtime_src/core/pcie/tools/cloud-daemon/azure/azure.cpp: In member function ‘int AzureDev::Sha256AndSplit(const string&, std::vector<std::__cxx11::basic_string<char> >&, std::string&)’:
/home/rkeryell/Xilinx/Projects/Vitis/XRT/src/runtime_src/core/pcie/tools/cloud-daemon/azure/azure.cpp:580:21: error: ‘int SHA256_Init(SHA256_CTX*)’ is deprecated: Since OpenSSL 3.0 [-Werror=deprecated-declarations]
  580 |     if (!SHA256_Init(&context)) {
      |          ~~~~~~~~~~~^~~~~~~~~~
In file included from /home/rkeryell/Xilinx/Projects/Vitis/XRT/src/runtime_src/core/pcie/tools/cloud-daemon/azure/azure.cpp:26:
/usr/include/openssl/sha.h:73:27: note: declared here
   73 | OSSL_DEPRECATEDIN_3_0 int SHA256_Init(SHA256_CTX *c);
      |                           ^~~~~~~~~~~
/home/rkeryell/Xilinx/Projects/Vitis/XRT/src/runtime_src/core/pcie/tools/cloud-daemon/azure/azure.cpp:590:26: error: ‘int SHA256_Update(SHA256_CTX*, const void*, size_t)’ is deprecated: Since OpenSSL 3.0 [-Werror=deprecated-declarations]
  590 |         if(!SHA256_Update(&context, segment.c_str(), segment.size()))
      |             ~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
In file included from /home/rkeryell/Xilinx/Projects/Vitis/XRT/src/runtime_src/core/pcie/tools/cloud-daemon/azure/azure.cpp:26:
/usr/include/openssl/sha.h:74:27: note: declared here
   74 | OSSL_DEPRECATEDIN_3_0 int SHA256_Update(SHA256_CTX *c,
      |                           ^~~~~~~~~~~~~
/home/rkeryell/Xilinx/Projects/Vitis/XRT/src/runtime_src/core/pcie/tools/cloud-daemon/azure/azure.cpp:601:21: error: ‘int SHA256_Final(unsigned char*, SHA256_CTX*)’ is deprecated: Since OpenSSL 3.0 [-Werror=deprecated-declarations]
  601 |     if(!SHA256_Final(result, &context)) {
      |         ~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~
In file included from /home/rkeryell/Xilinx/Projects/Vitis/XRT/src/runtime_src/core/pcie/tools/cloud-daemon/azure/azure.cpp:26:
/usr/include/openssl/sha.h:76:27: note: declared here
   76 | OSSL_DEPRECATEDIN_3_0 int SHA256_Final(unsigned char *md, SHA256_CTX *c);
      |                           ^~~~~~~~~~~~
[...]
::string Container::calculate_md5(char*, size_t)’:
/home/rkeryell/Xilinx/Projects/Vitis/XRT/src/runtime_src/core/pcie/tools/cloud-daemon/container/container.cpp:230:13: error: ‘int MD5_Init(MD5_CTX*)’ is deprecated: Since OpenSSL 3.0 [-Werror=deprecated-declarations]
  230 |     MD5_Init(&context);
      |     ~~~~~~~~^~~~~~~~~~
In file included from /home/rkeryell/Xilinx/Projects/Vitis/XRT/src/runtime_src/core/pcie/tools/cloud-daemon/container/container.cpp:38:
/usr/include/openssl/md5.h:49:27: note: declared here
   49 | OSSL_DEPRECATEDIN_3_0 int MD5_Init(MD5_CTX *c);
      |                           ^~~~~~~~
/home/rkeryell/Xilinx/Projects/Vitis/XRT/src/runtime_src/core/pcie/tools/cloud-daemon/container/container.cpp:231:15: error: ‘int MD5_Update(MD5_CTX*, const void*, size_t)’ is deprecated: Since OpenSSL 3.0 [-Werror=deprecated-declarations]
  231 |     MD5_Update(&context, buf, len);
      |     ~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~
In file included from /home/rkeryell/Xilinx/Projects/Vitis/XRT/src/runtime_src/core/pcie/tools/cloud-daemon/container/container.cpp:38:
/usr/include/openssl/md5.h:50:27: note: declared here
   50 | OSSL_DEPRECATEDIN_3_0 int MD5_Update(MD5_CTX *c, const void *data, size_t len);
      |                           ^~~~~~~~~~
/home/rkeryell/Xilinx/Projects/Vitis/XRT/src/runtime_src/core/pcie/tools/cloud-daemon/container/container.cpp:232:14: error: ‘int MD5_Final(unsigned char*, MD5_CTX*)’ is deprecated: Since OpenSSL 3.0 [-Werror=deprecated-declarations]
  232 |     MD5_Final(s, &context);
      |     ~~~~~~~~~^~~~~~~~~~~~~
In file included from /home/rkeryell/Xilinx/Projects/Vitis/XRT/src/runtime_src/core/pcie/tools/cloud-daemon/container/container.cpp:38:
/usr/include/openssl/md5.h:51:27: note: declared here
   51 | OSSL_DEPRECATEDIN_3_0 int MD5_Final(unsigned char *md, MD5_CTX *c);
      |                           ^~~~~~~~~
cc1plus: all warnings being treated as errors
make[2]: *** [runtime_src/core/pcie/tools/cloud-daemon/container/CMakeFiles/container_plugin.dir/build.make:76: runtime_src/core/pcie/tools/cloud-daemon/container/CMakeFiles/container_plugin.dir/container.cpp.o] Error 1
make[1]: *** [CMakeFiles/Makefile2:2352: runtime_src/core/pcie/tools/cloud-daemon/container/CMakeFiles/container_plugin.dir/all] Error 2
make[1]: *** Waiting for unfinished jobs....
@maxzhen maxzhen removed their assignment Apr 27, 2022
@keryell
Copy link
Member Author

keryell commented Apr 28, 2022

Probably OpenSSH decided that using MD5 considered broken since 2008 is to be really deprecated now :-)

@keryell
Copy link
Member Author

keryell commented Apr 28, 2022

If you need an Ubuntu 22.04 machine, use xsjsycl40.

@anupcs-xlnx anupcs-xlnx assigned chvamshi-xilinx and unassigned xuhz Apr 28, 2022
@anupcs-xlnx
Copy link

We don't support 22.04 officially yet. This is being worked on for a future release of XRT. We have internal Jira tickets for this.

@xuhz
Copy link
Contributor

xuhz commented Apr 28, 2022

since I spent some time investigating, i am sharing the findings
yes, in ubuntu22.04, the openssl version is 3.0 in which the sha, md5, etc calls are deprecated. instead the EVP_MD style call is expected.
the change is straigntforward, an example here
https://stackoverflow.com/questions/2262386/generate-sha256-with-openssl-and-c

@keryell
Copy link
Member Author

keryell commented Apr 28, 2022

@anupcs-xlnx do not close this ticket just because you can. This is an open-source project. You should keep it opened waiting for this to be solved some day.
I am happy you have internal Jira tickets for this. :-)
Note that this is working for us by just removing the 2 -Werror in src/CMakeLists.txt.

@keryell
Copy link
Member Author

keryell commented Apr 28, 2022

This reminds me ROCm/ROCm#1730

@keryell
Copy link
Member Author

keryell commented Apr 29, 2022

@xuhz Great so you can send a PR. :-)
The problem is not to break the code for obsolete OS which are supported. But I guess they are checked at least in AMD AECG CI.

@stsoe
Copy link
Collaborator

stsoe commented Apr 29, 2022

Agree with @keryell . This should not be closed. Leave it open until ultimately fixed. I made the mistake of assigning the issue, when that is the role of the PRT based on schedules etc.

@keryell Disabling warnings in makefiles is a horrible work-around, if you can fix the compilation issues with a PR that would be greatly appreciated.

@stsoe stsoe reopened this Apr 29, 2022
@keryell
Copy link
Member Author

keryell commented Apr 29, 2022

It is not about disabling warning messages but warnings are errors in production because you do not control the environment this will be compiled for, so it might possibly break. Of course, keep the warnings are errors when developing or building supported packages.

@stsoe
Copy link
Collaborator

stsoe commented Apr 29, 2022

What do you suggest? There is no production per say, it is an Open Source project that you are welcome to contribute to, but we sure as heck don't want your warnings committed or even caught just by the CI; we want them in your face :-) Plus we are fully capable of producing and comitting our own warnings.

@keryell
Copy link
Member Author

keryell commented May 3, 2022

Let's start with something simple:

  • add an option to build/build.sh to control whether -Werror is wanted or not;

  • add a few builds in GitHub Action to compile on a few Linux distributions × versions, specially the ones not tested internally, with -Werror if possible;

  • the most complex aspect is how to test also the kernel driver compilation and installation, since all this runs in a Docker;

    • but this should be optional anyway, for people using XRT with hardware or software emulation and does not use any hardware. So even if it would have not compiled successfully for a given kernel version, it does not matter.

That way it should be easier to screen external contributions.

stsoe added a commit to stsoe/XRT that referenced this issue May 30, 2022
Provide build.sh option `-disable-werror` to disable warnings
as errors.

Not willing to disable `-Werror` by default, since I can't seem to
find a good way to have it enabled for all internal builds including
all developers.

Fixes Xilinx#6640, maybe not exactly per comments, but close enough.
stsoe added a commit that referenced this issue May 31, 2022
* WIP

* Issue #6640 Option to disable warnings as error

Provide build.sh option `-disable-werror` to disable warnings
as errors.

Not willing to disable `-Werror` by default, since I can't seem to
find a good way to have it enabled for all internal builds including
all developers.

Fixes #6640, maybe not exactly per comments, but close enough.
@wilderfield
Copy link
Collaborator

Let's start with something simple:

  • add an option to build/build.sh to control whether -Werror is wanted or not;

  • add a few builds in GitHub Action to compile on a few Linux distributions × versions, specially the ones not tested internally, with -Werror if possible;

  • the most complex aspect is how to test also the kernel driver compilation and installation, since all this runs in a Docker;

    • but this should be optional anyway, for people using XRT with hardware or software emulation and does not use any hardware. So even if it would have not compiled successfully for a given kernel version, it does not matter.

That way it should be easier to screen external contributions.

I love the suggestion here from @keryell and appreciate that @stsoe implemented the optional usage of WError.
I am experimenting building XRT as part of a conda recipe...
In the conda recipe I don't want to force users to bind to a certain compiler version... so... newer gcc will throw all kinds of unexpected warnings... its nice to be able to build without solving all warnings.

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.

7 participants