Skip to content

fix: nil dereference in archive priority processing#269

Open
lczyk wants to merge 9 commits intocanonical:mainfrom
lczyk:panic-for-invalid-archives
Open

fix: nil dereference in archive priority processing#269
lczyk wants to merge 9 commits intocanonical:mainfrom
lczyk:panic-for-invalid-archives

Conversation

@lczyk
Copy link
Contributor

@lczyk lczyk commented Mar 3, 2026

  • Have you signed the CLA?

very niche, but the test by itself leads to a panic since we collect keys from yamlArchives but then index release.Archives with them

@lczyk
Copy link
Contributor Author

lczyk commented Mar 3, 2026

sorry for the empty commit noise. the github workers were loosing their minds and i needed to retrigger a couple of times

@lczyk lczyk requested a review from upils March 3, 2026 22:42
@lczyk lczyk added Simple Nice for a quick look on a minute or two Bug An undesired feature ;-) labels Mar 3, 2026
@github-actions
Copy link

github-actions bot commented Mar 3, 2026

Command Mean [s] Min [s] Max [s] Relative
BASE 13.657 ± 0.084 13.553 13.783 1.00 ± 0.01
HEAD 13.624 ± 0.061 13.523 13.700 1.00

Copy link
Collaborator

@upils upils left a comment

Choose a reason for hiding this comment

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

Thanks @lczyk, good catch!

The test just need a bit of tuning and then it shall be good to move to core review.

@lczyk
Copy link
Contributor Author

lczyk commented Mar 4, 2026

@upils both comments addressed. i wasn't 100% sure where to put the test, but i think i found a decent place.

also, i realized there is no equivalent test for v2/v3 so i've added that too (it's passing without any changes. the bug addressed here is with v1): df1581d

@lczyk lczyk requested a review from upils March 4, 2026 12:43
Copy link
Collaborator

@upils upils left a comment

Choose a reason for hiding this comment

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

Thanks for rework @lczyk. See my other comment regarding the tests.

},
relerror: `chisel.yaml: cannot parse maintenance: expected format for "standard" is YYYY-MM-DD`,
}, {
summary: "Maintenance: invalid pro in v2-archives ignored",
Copy link
Collaborator

Choose a reason for hiding this comment

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

I believe this test is not tied to the "maintenance" feature. Unless I am missing something, it looks like the name (and the content of the chisel.yaml are making it difficult to actually know what is tested.

Also, to be inline with the last thing discussed in #267, I suppose a single test for V3 would be enough since the tested behavior is common for all formats.

@lczyk lczyk requested a review from upils March 6, 2026 12:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Bug An undesired feature ;-) Simple Nice for a quick look on a minute or two

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants