Draft PR covering Canonical packaging dev changes#466
Conversation
Signed-off-by: Liu, Xiangquan <xiangquan.liu@intel.com>
* Implement the proposed solution of library versioning note: main branch missing a delete of unused QVE_VER?
note: to be merged * Update build system to link system libcrypto Signed-off-by: Xiangquan Liu <xiangquan.liu@intel.com>
| USE_PREBUILT_OPENSSL ?= 0 | ||
| ifeq ($(USE_PREBUILT_OPENSSL), 0) | ||
| CRYPTO_LIB = $(shell pkg-config --libs libcrypto 2>/dev/null) | ||
| CRYPTO_INC = $(shell pkg-config --cflags libcrypto 2>/dev/null) |
There was a problem hiding this comment.
Adding warning about potential pkg-config issues (and/or not installed) to be considered plus fallback mechanism:
| CRYPTO_INC = $(shell pkg-config --cflags libcrypto 2>/dev/null) | |
| CRYPTO_INC = $(shell pkg-config --cflags libcrypto 2>/dev/null) | |
| # Fallback if pkg-config fails or not installed | |
| ifeq ($(CRYPTO_LIB),) | |
| CRYPTO_LIB := -lcrypto | |
| endif | |
| ifeq ($(CRYPTO_INC),) | |
| CRYPTO_INC := | |
| endif |
There was a problem hiding this comment.
I think in case CRYPTO_LIB and CRYPTO_INC cannot be retrieved with pkg-config, we can stop and output an error message
| -I$(DCAP_TOPDIR)/QuoteGeneration/common/inc/internal/ \ | ||
| -I$(DCAP_TOPDIR)/QuoteGeneration/common/inc/internal/linux/ \ | ||
| -I$(PREBUILD_OPENSSL_PATH)/inc \ | ||
| -I../common |
There was a problem hiding this comment.
| -I../common | |
| -I../common \ |
| ifneq ($(wildcard $(SGX_SDK)),) | ||
| include $(SGX_SDK)/buildenv.mk | ||
| else ifneq ($(wildcard /usr/share/sgxsdk/buildenv.mk),) | ||
| include /usr/share/sgxsdk/buildenv.mk |
There was a problem hiding this comment.
It would be beneficial to have access to proposed buildenv.mk used in the local system. That would clarify which areas are intended to be configurable and enable Intel to provide more comprehensive test coverage. This might help prevent accidental regressions or issues affecting the build.
There was a problem hiding this comment.
hi @hector-cao! Is it ok for you to share the content of /usr/share/sgxsdk/buildenv.mk that you were thinking of? Maybe that could be added as a sample env mk file too for others to use.
| INCLUDE += -I. -I../inc | ||
| INCLUDE += -I$(SGX_SDK)/include \ | ||
| -I$(COMMON_DIR)/inc/internal \ | ||
| INCLUDE += -I$(SGX_TRUSTED_INCLUDE_PATH) \ |
There was a problem hiding this comment.
Can we have access to specific location (path) and content of the specific SDK include path? Is it simply a regular SGX SDK's include directory (built from Open Source or installed from Intel provided bin) but located outside of default installation location? E.g., SDK sub-dirs are located in Ubuntu-specific areas depending on the nature of the files?
|
|
||
| CFLAGS += -fPIC -Werror -g | ||
| Link_Flags := $(SGX_COMMON_CFLAGS) -L$(ROOT_DIR)/build/linux -L$(SGX_SDK)/lib64 -lsgx_urts -lpthread -ldl | ||
| Link_Flags := $(SGX_COMMON_CFLAGS) -L$(ROOT_DIR)/build/linux -L$(SGX_LIBRARY_PATH) -lsgx_urts -lpthread -ldl |
There was a problem hiding this comment.
Can we have access to specific location (path) and content of the specific SDK library path? Is it simply a regular SGX SDK's library directory (built from Open Source or installed from Intel provided bin) but located outside of default installation location? E.g., SDK sub-dirs are located in Ubuntu-specific areas depending on the nature of the files?
There was a problem hiding this comment.
Could you elaborate a little bit more on your question ?
The idea of this diff, I believe, is to use the right variable SGX_LIBRARY_PATH that points to the SGX libs
There was a problem hiding this comment.
I was curious if this is indeed simply a wrapper over the regular path to sgx/libs or maybe the libs directory is for you totally outside of the SGX SDK.
PS. Between lines, I was considering that maybe you preferred to store SDK's libs and includes separately, not under same SGX SDK installation dir (and thus needed separate path to libs and separate to includes to manage separately).
| p_eid, | ||
| NULL); | ||
| if (SGX_SUCCESS != sgx_status) { | ||
| #if defined(__GNUC__) |
There was a problem hiding this comment.
While #if defined(__GNUC__) will be true for many supported OSes, the solution provided std::string enclave_path_for_ubuntu("/usr/lib/x86_64-linux-gnu/"); is for debian systems only. We might want to consider extending the coverage.
4 participants
No description provided.