Topic/workflow

[agampa263]

No description provided.

[Copilot]

Pull request overview

This pull request refactors the build infrastructure to migrate from a Docker container-based build to a native ubuntu-22.04 GitHub Actions runner. The changes standardize installation paths, improve build organization with out-of-source builds, and update environment variable handling.

Changes:

• Migrated GitHub Actions workflow from Docker container to native ubuntu-22.04 runner
• Standardized install prefix to $GITHUB_WORKSPACE/install/usr across all build scripts
• Converted xdialserver build to CMake-based out-of-source build with Ninja generator
• Reorganized dependency build script with improved path handling and cleaner structure

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 8 comments.

File	Description
.github/workflows/native_full_build.yml	Removed Docker container configuration and switched to ubuntu-22.04 runner
cov_build.sh	Refactored to use CMake/Ninja out-of-source build with environment variables and standardized paths
build_dependencies.sh	Restructured with standardized install prefix, added sudo for apt commands, improved organization and path handling

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

The -fpermissive flag suppresses compilation errors and treats them as warnings. This can hide real issues in the code and should be avoided in production builds. Consider fixing the underlying compilation errors instead of masking them with -fpermissive.

Suggested change

	-fpermissive
	g++ -fPIC -shared -o libWPEFrameworkSecurityUtil.so \
	securityagent/SecurityTokenUtil.cpp \
	-I"$GITHUB_WORKSPACE/stubs" \
	-I"$STUB_INCLUDE" \
	-fpermissive
	-Wall
	g++ -fPIC -shared -o libWPEFrameworkSecurityUtil.so \
	securityagent/SecurityTokenUtil.cpp \
	-I"$GITHUB_WORKSPACE/stubs" \
	-I"$STUB_INCLUDE" \
	-Wall

[Copilot]

The -fpermissive flag suppresses compilation errors and treats them as warnings. This can hide real issues in the code and should be avoided in production builds. Consider fixing the underlying compilation errors instead of masking them with -fpermissive.

Suggested change

	-I"$STUB_INCLUDE" \
	-fpermissive
	g++ -fPIC -shared -o libWPEFrameworkSecurityUtil.so \
	securityagent/SecurityTokenUtil.cpp \
	-I"$GITHUB_WORKSPACE/stubs" \
	-I"$STUB_INCLUDE" \
	-fpermissive
	-I"$STUB_INCLUDE"
	g++ -fPIC -shared -o libWPEFrameworkSecurityUtil.so \
	securityagent/SecurityTokenUtil.cpp \
	-I"$GITHUB_WORKSPACE/stubs" \
	-I"$STUB_INCLUDE"

[Copilot]

The GITHUB_TOKEN is exposed to the git clone command on line 40, which could log sensitive credentials in build logs. Consider using a credential helper or other secure method to handle authentication for private repositories.

Suggested change

	git clone https://$GITHUB_TOKEN@github.com/rdkcentral/entservices-testframework.git
	set +x
	git config --global url."https://x-access-token:${GITHUB_TOKEN}@github.com/".insteadOf "https://github.com/"
	set -x
	git clone https://github.com/rdkcentral/entservices-testframework.git

[Copilot]

The pip install command on line 21 does not use sudo, while the apt commands on lines 14-19 do. This inconsistency may cause the pip package to be installed in the user's local environment instead of system-wide. Consider either using 'sudo pip install' for consistency or using 'pip install --user' to make the local installation explicit.

Suggested change

	pip install jsonref
	pip install --user jsonref

[Copilot]

The script clones the trower-base64 repository without checking if it already exists. If the script is run multiple times, this will fail. Consider adding a check to skip cloning if the directory already exists, or remove the directory before cloning.

Suggested change

	# Build trower-base64
	# Build trower-base64
	if [ -d "trower-base64" ]; then
	rm -rf "trower-base64"
	fi

[Copilot]

The git clone commands for repositories on lines 34-41 will fail if these directories already exist from a previous run. With 'set -e' enabled, this will cause the entire script to fail. Consider either removing existing directories before cloning or adding checks to skip cloning if directories exist.

[Copilot]

Migrating from a Docker container (ghcr.io/rdkcentral/docker-rdk-ci:latest) to ubuntu-22.04 runner changes the build environment significantly. The build_dependencies.sh script now requires sudo access (lines 14-19), which works on GitHub Actions runners but may not work in all environments. Consider documenting this requirement or ensuring the scripts can work both with and without sudo when needed.

Suggested change

	runs-on: ubuntu-22.04
	runs-on: ubuntu-22.04
	container: ghcr.io/rdkcentral/docker-rdk-ci:latest

[Copilot]

Using CMAKE_CXX_FLAGS to pass include directories is not the recommended approach. CMake provides CMAKE_CXX_INCLUDES or the target-level include_directories command for this purpose. CMAKE_CXX_FLAGS should be reserved for actual compiler flags (optimization levels, warnings, etc.). Additionally, the multiline format with the backslash continuation after the opening quote will introduce leading whitespace in the flags, which may cause issues with some compilers.

Suggested change

	-DCMAKE_CXX_FLAGS="\
	-I$INSTALL_PREFIX/include/WPEFramework \
	-I$INSTALL_PREFIX/include/WPEFramework/core \
	-I$INSTALL_PREFIX/include/WPEFramework/plugins \
	-I$INSTALL_PREFIX/include/WPEFramework/interfaces" \
	-DCMAKE_CXX_INCLUDES="$INSTALL_PREFIX/include/WPEFramework;$INSTALL_PREFIX/include/WPEFramework/core;$INSTALL_PREFIX/include/WPEFramework/plugins;$INSTALL_PREFIX/include/WPEFramework/interfaces" \

[scthunderbolt]

This is not the same as running container: ubuntu:22.04.
Please see #186