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

android: convert .mk files to .bp #371

Merged
merged 1 commit into from
Jan 29, 2024

Conversation

massonju
Copy link
Contributor

Android use by default Soong Build System, *.mk files are deprecated in favor of Android.bp [1].

All the logics present in android mk files have been ported to a single Android.bp

optee_client.device.mk set the same default flags as we did in the old Android *.mk files.

Example of configs in device/VENDOR/BOARD/device.mk:

include $(PATH_OPTEE_CLIENT)/optee_client.device.mk
PRODUCT_PACKAGES += \
    libteec \
    tee-supplicant

[1] https://source.android.com/docs/setup/build

@massonju
Copy link
Contributor Author

FYI it has been tested with TI am62x board on Android 14 (kmgk, tee-supplicant, libteec ...).

https://baylibre.pages.baylibre.com/ti/android/doc/index.html

@jforissier
Copy link
Contributor

Thanks for the contribution. Is it easy enough for someone currently using the .mk files to switch to the new build system? If so I have no objections.

@massonju
Copy link
Contributor Author

Hi @jforissier,

All the external *.mk files which depends on binary/libraries from optee-client don't need to be updated.

For example keymaster from kmgk is still using .mk format and depends on libteec, it doesn't need to be updated.

However all the flags defined in Android.bp of optee-client project must be set in "soong" way.

On our TI board we modify some of these flags.
Before: (device/ti/am62x/optee/device-optee.mk)

CFG_TEE_FS_PARENT_PATH := /mnt/vendor/persist/tee

PRODUCT_PACKAGES += \
    libteec \
    tee-supplicant

https://gitlab.baylibre.com/baylibre/ti/android/aosp/device/ti/am62x/-/blob/ti-android-14/optee/device-optee.mk?ref_type=heads#L28

After:

include vendor/linaro/optee_client/optee_client.device.mk
$(call soong_config_set,optee_client,cfg_tee_fs_parent_path,/mnt/vendor/persist/tee)

PRODUCT_PACKAGES += \
    libteec \
    tee-supplicant

I added optee_client.device.mk in this project to facilitate the integration and keep the same default behavior/flags.

I can also contribute to the documentation if it's needed:
https://optee.readthedocs.io/en/latest/building/aosp/aosp.html

@massonju
Copy link
Contributor Author

I've pushed a new version.

one condition was missing for TEEC_LOG_FILE.
if TEEC_LOG_FILE is not set and TEE_FS_PARENT_PATH is set to XXXX:
cflags: ["-DTEEC_LOG_FILE="XXXX/teec.log""]

@massonju
Copy link
Contributor Author

I have no more changes, this patch is ready to review.
Please let me know if you need more infos

Copy link
Contributor

@jforissier jforissier left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @massonju,

However all the flags defined in Android.bp of optee-client project must be set in "soong" way.

On our TI board we modify some of these flags. Before: (device/ti/am62x/optee/device-optee.mk)

CFG_TEE_FS_PARENT_PATH := /mnt/vendor/persist/tee

PRODUCT_PACKAGES += \
    libteec \
    tee-supplicant

[...]

After:

include vendor/linaro/optee_client/optee_client.device.mk
$(call soong_config_set,optee_client,cfg_tee_fs_parent_path,/mnt/vendor/persist/tee)

PRODUCT_PACKAGES += \
    libteec \
    tee-supplicant

I believe this example (before/after) would be good to have in the commit description.

I added optee_client.device.mk in this project to facilitate the integration and keep the same default behavior/flags.

Sounds good.

I can also contribute to the documentation if it's needed: https://optee.readthedocs.io/en/latest/building/aosp/aosp.html

You're welcome to do so!

With my comments addressed:

Acked-by: Jerome Forissier <[email protected]>

Android.bp Outdated
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could a SPDX tag be used here instead of the full license text?

// SPDX-License-Identifier: Apache-2.0
//
// Copyright (C) 2024 The Android Open Source Project

package ...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes sure I've replaced with the SPDX tag in my last push

Copy link
Contributor

@jforissier jforissier left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One more comment sorry :-/ also no need to add the markdown quotes (```) in the commit message, it is just plain text, use a blank line instead.

optee_client.device.mk Outdated Show resolved Hide resolved
Android use by default Soong Build System, *.mk files are deprecated
in favor of Android.bp [1].

All the logics present in android mk files have been ported to a
single Android.bp

optee_client.device.mk set the same default flags as we did in the
old Android *.mk files.

Example of configs in device/VENDOR/BOARD/device.mk:
Before:

CFG_TEE_FS_PARENT_PATH := /mnt/vendor/persist/tee

PRODUCT_PACKAGES += \
    libteec \
    tee-supplicant

After:

include $(PATH_OPTEE_CLIENT)/optee_client.device.mk
$(call soong_config_set,optee_client,cfg_tee_fs_parent_path,/mnt/vendor/persist/tee)

PRODUCT_PACKAGES += \
    libteec \
    tee-supplicant

[1] https://source.android.com/docs/setup/build

Acked-by: Jerome Forissier <[email protected]>
Signed-off-by: Julien Masson <[email protected]>
@jforissier jforissier merged commit afbd31d into OP-TEE:master Jan 29, 2024
3 checks passed
@massonju
Copy link
Contributor Author

Thanks for the review/merge @jforissier

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.

2 participants