Skip to content

fix: don't attempt to sort affected events#36

Merged
G-Rath merged 1 commit into
mainfrom
sort/dont-events
Apr 28, 2025
Merged

fix: don't attempt to sort affected events#36
G-Rath merged 1 commit into
mainfrom
sort/dont-events

Conversation

@G-Rath

@G-Rath G-Rath commented Apr 25, 2025

Copy link
Copy Markdown
Collaborator

The events should already be sorted since we process version constraints from left to right, and the lower bounds always comes first, and our current sorting implementation both ignores the stability constraint and actually attempts to deduplicate events by their version resulting in some advisories losing their introduced event

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull Request Overview

This PR removes the unnecessary sorting of affected version events in the OSV advisory generation process to prevent the inadvertent deduplication of events.

  • Removed the sort_affected_versions function and the corresponding sorting call in build_affected_range.
  • Now directly uses parse_version_constraint to preserve the original order of events.
Files not reviewed (4)
  • advisories/acquia_dam/osv-sa-contrib-2024-025.json: Language not supported
  • advisories/diff/osv-sa-contrib-2024-042.json: Language not supported
  • advisories/social/osv-sa-contrib-2024-038.json: Language not supported
  • advisories/symfony_mailer/osv-sa-contrib-2023-031.json: Language not supported
Comments suppressed due to low confidence (1)

scripts/generate_osv_advisories.py:96

  • [nitpick] Ensure there are tests verifying that the ordering of events produced by parse_version_constraint remains consistent with the intended left-to-right constraint processing.
'events': parse_version_constraint(constraint),

@jonholdsworth jonholdsworth left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Good catch

@G-Rath G-Rath force-pushed the sort/dont-events branch from 02ee5dd to 0981d43 Compare April 28, 2025 02:27

@Unifex Unifex left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

LGTM :)

@G-Rath G-Rath merged commit c1f5acb into main Apr 28, 2025
@G-Rath G-Rath deleted the sort/dont-events branch April 28, 2025 03:55
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