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

Improve display of information for outdated crates #34

Merged
merged 4 commits into from
Jan 21, 2024

Conversation

jamessan
Copy link
Contributor

@jamessan jamessan commented Nov 27, 2023

With #32 merged, we now have more information to use when deciding how to display the dependency information.

Let's use this to provide more details when a crate is in Debian/NEW,but the package is incompatible with the current version of the crate being inspected.

In particular, we should display the dependency information for outdated/incompatible crates, since it's likely there have been changes. Also, provide more visual clues about the incompatibility -- crate names are yellow, rather than green/blue, "outdated" is included in the "(in debian" clause, and the version is red.

0.5.1:
2023-11-27-122050_618x360_scrot

After #32:
2023-11-27-112919_792x62_scrot

This PR:
2023-11-27-115934_810x365_scrot

Since 0.x.y versions have slightly different semantics than 1.0 and
later, add specific tests for those.
src/debian.rs Outdated
}

if let Some(deb) = &self.debinfo {
!deb.exact_match
Copy link
Owner

Choose a reason for hiding this comment

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

This would potentially show a lot more packages that we don't need to care about right now when uploading a new package/upgrading one. If Debian has a semver compatible crate available we currently stop because we can usually just use that and don't need to keep traversing the tree.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is sort of tricky. Crate A may make an API compatible update which introduces new dependencies. Hiding those gives a false sense of the work needed.

However, I guess we could say the new update doesn't matter since it's API compatible, so we just stick with the older version of the crate.

This is currently an area where debstatus is lacking, since we rely on how cargo is resolving dependencies, which is greedy. It may say we need a new version of a crate (which has the new dependency) when we really don't.

If we instead worked with Cargo.toml versions, we could provide a more accurate picture and I'd agree more with your stance.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If Debian has a semver compatible crate available we currently stop because we can usually just use that and don't need to keep traversing the tree.

That's actually what my "This PR" image above shows, but then I made the change to use exact_match instead. I've changed the behavior now so that it only shows dependencies for a package in Debian if it's outdated/incompatible. This can be overridden with the "-a" switch.

@jamessan jamessan force-pushed the outdated-crates branch 2 times, most recently from cb16c51 to 8512a72 Compare November 29, 2023 23:24
Just because a crate currently exists in Debian doesn't mean we don't
need the information about its dependencies.  Especially in the case
where there is significant version skew (outdated or incompatible), we
should provide information about the dependency tree.
Signed-off-by: James McCoy <jamessan@jamessan.com>
When a crate is already packaged for Debian but is outdated, provide
more information about this discrepancy.

* Include "outdated" in the Debian information
* Color the crate name as yellow, rather than green, since the package
  can't be used as is
* Color the version as red
@alexanderkjall
Copy link
Collaborator

I just tested this, and to it seems like a clear improvement to my workflow.

@kpcyrd what do you think about merging it?

@kpcyrd
Copy link
Owner

kpcyrd commented Jan 21, 2024

Fine with me 👍

@kpcyrd kpcyrd merged commit ada6cbb into kpcyrd:main Jan 21, 2024
3 checks passed
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.

3 participants