Skip to content

feat: auto detect license files from prefix directory as well #1648

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

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

wolfv
Copy link
Member

@wolfv wolfv commented May 11, 2025

This adds auto-detection from the $PREFIX as well.

@mfansler do you think this is acceptable? It would fix the Rust case. I would have to think a bit longer to find a good solution for ${{ PREFIX }}/foobar case because the way we render the recipe, these variables are not available.

@mfansler
Copy link

@wolfv thanks for the heads up. Yes, this would suffice. 👍

So, our translation will be, for example:

v0

about:
  # other stuff
  license_file:
    - {{ environ["PREFIX"] }}/lib/R/share/licenses/MIT
    - LICENSE

v1

about:
  # other stuff
  license_file:
    - lib/R/share/licenses/MIT
    - LICENSE

I.e., simply remove {{ environ["PREFIX"] }}/ from any about.license_file entries.

@wolfv
Copy link
Member Author

wolfv commented May 12, 2025

Yes, with this PR that would be the case, indeed.

@wolfv
Copy link
Member Author

wolfv commented May 13, 2025

I've had some extra thoughts on this. I think it might be better to compute the directories first (without creating them) so that we can already resolve ${{ PREFIX }}, ${{ SRC_DIR }}and${{ RECIPE_DIR }}` when rendering, so that users can be really explicit about where the license is supposed to come from.

For this I would also modify the folder that we create (currently it looks like package-name_1234513425134) where the number is the timestamp. The number is not easy to understand anyways (hard to know which is the latest build), so if we have e.g. a running index it might be better regardless.

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