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

recipes-bsp: libdmabufheap: Add recipe to compile libdmabufheap #766

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

quic-vjitta
Copy link

Add recipe to compile libdmabuf which provices apis to access dmabuf heap framework.

Copy link
Collaborator

@lumag lumag left a comment

Choose a reason for hiding this comment

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

What is the reason for the recipe being named qcom-libdmabufheap instead of just libdmabufheap ?

@lumag
Copy link
Collaborator

lumag commented Feb 22, 2025

There doesn't seem to be a versioned release tag. Please rename the recipe to foo_git.bb and set PV = "0.0+git"

@quic-vjitta
Copy link
Author

There doesn't seem to be a versioned release tag. Please rename the recipe to foo_git.bb and set PV = "0.0+git"
Added it.

@quic-vjitta
Copy link
Author

What is the reason for the recipe being named qcom-libdmabufheap instead of just libdmabufheap ?

This source code has some changes from qualcomm and is diverged from android open source libdmabufheap code, So it's being named with qcom-libdmabufheap.

Copy link
Collaborator

@lumag lumag left a comment

Choose a reason for hiding this comment

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

Other than the unnecessary FILES, what is the reason for the recipe being named qcom-libdmabufheap instead of just libdmabufheap ?

@lumag
Copy link
Collaborator

lumag commented Feb 25, 2025

Also please drop the gerrit tag from the commit message

@koenkooi
Copy link
Contributor

Other than the unnecessary FILES, what is the reason for the recipe being named qcom-libdmabufheap instead of just libdmabufheap ?

And if it's a variant of an existing upstream project, there need to the appropriate (R)CONFLICTS /(R)REPLACES/(R)PROVIDES stanzas to alert the parser to the conflicts.

Copy link

Test Results

 2 files  ±0   4 suites  ±0   1m 21s ⏱️ ±0s
19 tests ±0  19 ✅ ±0  0 💤 ±0  0 ❌ ±0 
38 runs  ±0  38 ✅ ±0  0 💤 ±0  0 ❌ ±0 

Results for commit a0f449f. ± Comparison against base commit 313aa65.

Copy link

Test jobs for commit a0f449f

Add recipe to build libdmabuf which provices apis to access
dmabuf heap framework.

Signed-off-by: Vijayanand Jitta <[email protected]>
@quic-vjitta
Copy link
Author

Also please drop the gerrit tag from the commit message

done

Copy link
Collaborator

@lumag lumag left a comment

Choose a reason for hiding this comment

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

  • Drop the qcom- prefix from the recipe name, everything in this layer is qcom-related.
  • Make it tested in the CI

@lumag
Copy link
Collaborator

lumag commented Feb 27, 2025

What is the reason for the recipe being named qcom-libdmabufheap instead of just libdmabufheap ?

This source code has some changes from qualcomm and is diverged from android open source libdmabufheap code, So it's being named with qcom-libdmabufheap.

In such a case, it might be better package libdmabufheap and add your patches on top of it. I strongly dislike the idea of qcom-specific forks of the existing projects. And qcom-specific recipes.

@lumag
Copy link
Collaborator

lumag commented Feb 27, 2025

Okay. After looking at the AOSP source... Don't package it. But the commit message should document that it's a fork of the AOSP library. Inside the recipe please add a brief comment describing Qualcomm changes, so that a random user browsing the repo tree would understand the difference and the reason. Please follow @koenkooi's suggestion and add 'PROVIDES' notes.

@lumag
Copy link
Collaborator

lumag commented Feb 27, 2025

Also, while I noticed it. Neither libdmabufheap nor libvmmem are BSP-related packages. Please move them to a more sensible location. Maybe recipes-support.

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.

5 participants