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

Marking file as type "starlark" fails to allow it to be treated as a Starlark file. #786

Closed
jtigger opened this issue Dec 28, 2022 · 2 comments · Fixed by #797
Closed

Marking file as type "starlark" fails to allow it to be treated as a Starlark file. #786

jtigger opened this issue Dec 28, 2022 · 2 comments · Fixed by #797
Assignees
Labels
bug This issue describes a defect or unexpected behavior priority/unprioritized-backlog Higher priority than priority/awaiting-more-evidence but not planned. Contributions are welcome.

Comments

@jtigger
Copy link
Contributor

jtigger commented Dec 28, 2022

Reported by @Zebradil in #carvel. Thank you. 🙏

Steps to Reproduce

  1. create the file star.test1:
    print(42)
  2. by default, ytt regards this file a "unknown type" or "data" and so it is not evaluated (expected):
    $ ytt -f test.star1
    
  3. rename the file to .star, and ytt regards this file a "starlark" file and evaluates it (expected):
    $ ytt -f test.star
    42
    
  4. mark the file directly as type starlark, and ytt should regard this file a "starlark" file and evaluate it:
    $ ytt -f test.star1 --file-mark='test.star1:type=starlark'
    
    (but it does not 🛑)

Anything else you would like to add:

Key symptom is the way in which a file is determined to be a "library file":

https://github.com/vmware-tanzu/carvel-ytt/blob/92f7de5ca3a11dfa5f341134f15cbc0a6b0fb31f/pkg/files/file.go#L242-L251

which only regards the extension of a file.

  • what can be done to respect the file mark for starlark type files?
  • should there be a file mark for YAML files that are also "library file"s (e.g. type=yaml-library)?

Environment:

  • ytt version (use ytt --version): 0.44.0 (likely introduced some versions back)
  • OS (e.g. from /etc/os-release): (all)

Vote on this request

This is an invitation to the community to vote on issues, to help us prioritize our backlog. Use the "smiley face" up to the right of this comment to vote.

👍 "I would like to see this addressed as soon as possible"
👎 "There are other more important things to focus on right now"

We are also happy to receive and review Pull Requests if you want to help working on this issue.

@jtigger jtigger added bug This issue describes a defect or unexpected behavior carvel triage This issue has not yet been triaged for relevance labels Dec 28, 2022
@Zebradil
Copy link
Member

Zebradil commented Jan 3, 2023

Thank you for creating the issue.

I have noticed that the code on the step 3 is incorrect. It should be something with the following:

ytt -f test.star

@jtigger
Copy link
Contributor Author

jtigger commented Jan 23, 2023

Whops! Thanks, @Zebradil. I updated the description.

@jtigger jtigger added priority/unprioritized-backlog Higher priority than priority/awaiting-more-evidence but not planned. Contributions are welcome. and removed carvel triage This issue has not yet been triaged for relevance labels Jan 23, 2023
jtigger added a commit that referenced this issue Jan 25, 2023
... and plumb UI's stderr to Starlark's Print() so that even output from
Starlark runtime respects UI settings.

Fixes #786
@jtigger jtigger self-assigned this Jan 25, 2023
jtigger added a commit that referenced this issue Jan 26, 2023
jtigger added a commit that referenced this issue Jan 26, 2023
jtigger added a commit that referenced this issue Jan 26, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug This issue describes a defect or unexpected behavior priority/unprioritized-backlog Higher priority than priority/awaiting-more-evidence but not planned. Contributions are welcome.
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

2 participants