image_types_qcom.bbclass: Allow QCOMFLASH_DIR to be overridden #755
Conversation
Users may need to override QCOMFLASH_DIR, the flashable artifacts path, as per their build needs. so use the ?= operator to set the default value. Signed-off-by: Viswanath Kraleti <[email protected]>
quic-vkraleti
requested review from
ricardosalveti,
sbanerjee-quic and
ndechesne
as code owners
| @@ -18,7 +18,7 @@ QCOM_DTB_FILE ?= "dtb.bin" | |||
| QCOM_ROOTFS_FILE ?= "rootfs.img" | |||
| IMAGE_QCOMFLASH_FS_TYPE ??= "ext4" | |||
|
|
|||
| QCOMFLASH_DIR = "${IMGDEPLOYDIR}/${IMAGE_NAME}.qcomflash" | |||
| QCOMFLASH_DIR ?= "${IMGDEPLOYDIR}/${IMAGE_NAME}.qcomflash" | |||
There was a problem hiding this comment.
What for? Please provide an example rather than a vague "users may need to".
There was a problem hiding this comment.
To use here: qualcomm-linux/meta-qcom-distro@d045d04
There was a problem hiding this comment.
What is the point? You can still use the .qcomflash subdir.
There was a problem hiding this comment.
What is wrong if qcomflash subdir is not used? This is not about what is correct and what is not. When a variable is defined, there is a general expectation that it can be overridden as needed. Making a hard assignment is not correct. If the intention is not to allow modifying the path, then it should not have been a variable and should have been hard-coded instead.
There was a problem hiding this comment.
The variable can be defined in order to make reading the code easier, like the const in other languages. Some variables differ from configuration to the configuration, there is a need to override them. Others are common and as such there is no need to override them. If you think that the existing value is not optimal, please change it instead of overriding it just for the distro: it makes user experience much more difficult if the deploy subdirs start to depend on the $DISTRO.
There was a problem hiding this comment.
In house tools used by Qualcomm don't understand this directory organization. They are coded to work with the paths used in Kirkstone & Scarthgap branches of meta-qcom-hwe. (i.e. builddir/tmpdir/deploy/images/machine/imgName)
I am not comfortable discussing beyond this in a public forum.
There was a problem hiding this comment.
So, either those tools need to be fixed, or you can change a default value here. Both options are fine with me.
P.S. This is a public layer now, so such discussions are to be public.
There was a problem hiding this comment.
I don't think we should have a different behavior for this in meta-qcom-distro, we should have a default and use the same on both, otherwise this will cause confusion to our users (e.g. documentation would be different when using just the BSP layer).
Can we evaluate changing the internal tools? We can discuss the internal tooling needs via email.
There was a problem hiding this comment.
[...]If you think that the existing value is not optimal, please change it instead of overriding it just for the distro: it makes user experience much more difficult if the deploy subdirs start to depend on the
$DISTRO.
Tangentially related to this: in Angstrom we moved DEPLOY_DIR out of TMPDIR to the top level to make it easier to find, since it contains the things people actually consume. So the DISTRO cleaning up how a BSP behaves is not unprecedented, but it does need careful consideration.
There was a problem hiding this comment.
Well, Angstrom moved the whole DEPLOY_DIR.
Test jobs for commit 67652fb |
Users may need to override QCOMFLASH_DIR, the flashable artifacts path, as per their build needs. so use the ?= operator to set the default value.