Repository navigation
fix(preprocess): report empty metadata.yaml clearly - #2975
abhiramshaji-cyber wants to merge 1 commit into
Conversation
An empty metadata.yaml, or one holding only comments, loads as nothing and was reported as an invalid file that does not meet the Juju specification. That message sends people looking for a schema problem when the file has no content at all. Report it as an empty file and suggest filling or deleting it. Fixes canonical#2071
e3c6251 to
eb7ca82
Compare
|
I rebased this onto the latest main. The diff-schema, lint / files and Read the Docs failures all came from the python-apt wheel download returning 403 during setup, and #2976 fixed that on main. I also ran the schema generator, the preprocess unit tests, ruff and the docs build (warnings treated as errors) locally, and they all pass. The OSV scanner and snap-tests failures look unrelated to this change, since they also fail on main right now, for example https://github.com/canonical/charmcraft/actions/runs/40889779582 (OSV, flagging filelock) and https://github.com/canonical/charmcraft/actions/runs/40889778815 (snap-tests). If you'd like anything changed in the fix, the test or the release note wording, just let me know. |
|
Hi, hope you're doing well! Thanks for your time. When someone has a moment, could the workflow runs be approved? The OSV and snap test failures also happen on main, as noted above, and I'm happy to change anything you'd prefer. |
|
Thanks for approving and running CI. I went through each failing job, and as far as I can tell none of them come from this change:
Happy to change anything if you see it differently. |
Fixes #2071
When
metadata.yamlis empty, packing a charm now says the file is empty instead of claiming it breaks the Juju specification.What this code is for
add_metadataincharmcraft/preprocess.pyruns when Charmcraft loads a project, beforecharmcraft packand other commands. If ametadata.yamlsits next tocharmcraft.yaml, it reads that file withutil.safe_yaml_loadand copies its fields into the project data.Before this PR
A charm author has a valid
charmcraft.yamland a leftovermetadata.yamlthat holds only a license comment, then runscharmcraft pack.add_metadatafindsmetadata.yamland loads it withutil.safe_yaml_load.None.Noneis not adict, so theif not isinstance(metadata_yaml, dict)check raises.Invalid file: 'metadata.yaml'withEnsure metadata.yaml meets the juju metadata.yaml specification.After this PR
Same author, same command.
if metadata_yaml is Nonecheck runs first and raisesEmpty file: 'metadata.yaml'.Add the charm metadata to metadata.yaml or delete the file.and knows what to do.metadata.yamlthat holds a list or a plain string still getsInvalid file: 'metadata.yaml', as before.The diagram
flowchart LR classDef box fill:#eef1ff,stroke:#4c5bd4,color:#111827 classDef bad fill:#ffd6d6,stroke:#cc0000,color:#111827,stroke-width:3px classDef good fill:#c8f0c8,stroke:#008800,color:#111827,stroke-width:3px subgraph BEFORE["BEFORE: empty file reported as invalid"] direction TB B1["charmcraft pack"] --> B2["add_metadata"] B2 --> B3["util.safe_yaml_load returns None"] B3 --> B4{"not isinstance dict"} B4 -->|"None is not a dict"| B5["Invalid file: 'metadata.yaml'"] B4 -->|"mapping"| B6["fields copied into project"] end subgraph AFTER["AFTER: empty file reported as empty"] direction TB A1["charmcraft pack"] --> A2["add_metadata"] A2 --> A3["util.safe_yaml_load returns None"] A3 --> A4{"THE CHANGE: metadata_yaml is None"} A4 -->|"empty or only comments"| A5["Empty file: 'metadata.yaml'"] A4 -->|"mapping"| A6["fields copied into project"] end B1 ~~~ A1 class B1,B2,B3,B6 box class A1,A2,A3,A6 box class B4,B5 bad class A4,A5 good linkStyle 3 stroke:#cc0000,stroke-width:5px linkStyle 8 stroke:#008800,stroke-width:5px style BEFORE fill:#fff5f5,stroke:#cc0000,color:#111827 style AFTER fill:#f3fff3,stroke:#008800,color:#111827The change
add_metadatagets anif metadata_yaml is Nonecheck before the existingisinstancecheck. It raisesCraftValidationErrorwith the messageEmpty file: 'metadata.yaml', a resolution telling the author to fill or delete the file, the same docs link, and the same exit code 65.tests/unit/test_preprocess.pynow expects the new message for an empty file and a comment only file, and keeps a case for a list that still getsInvalid file. The 4.6 release notes list the fix.What changes for a user
metadata.yamlnow gives a message that names the real problem.metadata.yamlwith the wrong shape still getsInvalid file: 'metadata.yaml'.metadata.yamlis read exactly as before.make lint && make test.