show latest major version available in `gleam deps update` #3876

andho · 2024-11-22T04:01:44Z

This PR aims to solve #3843.
The sample output from that issue:

 % gleam deps update
  Resolving versions
Downloading packages
 Downloaded 2 packages in 0.01s
 
Hint: the following dependencies have new major versions available...

- wibble@2
- wobble@3

Todos

~~Handle Async: fireoff the checks and show the results at the end.~~ Didn't make sense to do this.
Telemetry? Not sure it's needed for this optional part.
~~Merge version check into a single call~~ Hexpm API doesn't have this. Went with cache instead.
Get the direct dependencies only

lpil · 2024-12-02T15:40:16Z

Hello! Are you still working on this one? Thanks

andho · 2024-12-05T12:27:10Z

Hello! Are you still working on this one? Thanks

I was hoping to work on it this weekend but some things have come up. I can only get back to it next weekend.

andho · 2024-12-06T15:32:25Z

Just the major version hist without any other output is weird though:

Helpful, but looks weird.

inoas · 2024-12-09T13:41:40Z

Could a flag be provided that forces a non-zero exit-code if major deps updates are available?

Would that make sense?

The idea would be to have a CI check that goes red if deps are not up to date.

lpil

Very nice! I've left some small notes inline.

lpil · 2024-12-09T15:47:50Z

compiler-core/src/dependency.rs

+            ]
+            .into_iter()
+            .collect(),
+            //requirements: HashMap::new(),


Unused code here

Haha, this is embarrassing.
PS. Fixed.

compiler-cli/src/dependencies.rs

lpil · 2024-12-09T15:50:10Z

compiler-core/src/dependency.rs

+            let Some(latest) = &hexpackage
+                .releases
+                .iter()
+                .map(|release| release.version.clone())


Can take a reference here and avoid cloning

lpil · 2024-12-09T15:50:35Z

compiler-core/src/dependency.rs

+                .sorted_by(|a, b| b.cmp(a))
+                .next()


Using .max will be faster than sorting and taking the first

lpil · 2024-12-09T15:51:38Z

compiler-cli/src/dependencies.rs

+
+        // lazily assuming the longest version could be 8 characters. eg: 1.1234.2
+        // excluding qualifiers other than x.y.z
+        eprintln!("{name}:{padding} {v1:<8} -> {v2:<8}");


Let's render this to a string and then snapshot test the function. This function can then just print the string and not be responsible for formatting and printing

andho · 2024-12-13T05:34:52Z

Could a flag be provided that forces a non-zero exit-code if major deps updates are available?

Would that make sense?

The idea would be to have a CI check that goes red if deps are not up to date.

@inoas I think the flag would make more sense in a gleam deps outdated command instead.

lpil

Great, thank you.

I gave it a try and while the information is there and useful I don't think it fits in at all with any of the other information printed. How can we show this better?

Should it be shown for all resolution or only for gleam update? If the latter it would be easier to make a good output format for it.

andho · 2024-12-18T03:36:35Z

Should it be shown for all resolution or only for gleam update?

It makes sense to show for gleam update and gleam deps download

Although it was helpful when I recently did gleam build and I saw that decode package was updated to 1.0.0. But due to the execution order it would take some rearranging to get this output to come last.

I'd say for an MVP show only in gleam update and gleam deps download. I'll only have to add a flag to download fn.

lpil · 2024-12-19T11:55:54Z

Sounds great!

lpil

Looks great!! Thank you. I've left a few tiny notes inline.

lpil · 2024-12-28T17:25:56Z

compiler-cli/src/dependencies.rs

@@ -219,73 +221,75 @@ pub fn download<Telem: Telemetry>(
    // manifest which will result in the latest versions of the dependency
    // packages being resolved (not the locked ones).
    use_manifest: UseManifest,
+    // If true we check for major version updates and print them to the console.
+    check_major_versions: bool,


Create an enum instead of using bools please. true and false mean nothing without context so they are hard to understand. See the other arguments for examples.

lpil · 2024-12-28T17:26:48Z

compiler-cli/src/dependencies.rs

-        tracing::debug!("writing_manifest_toml");
-        write_manifest_to_disc(paths, &manifest)?;
+    output_string
+        .push_str("\nHint: the following dependencies have new major versions available:\n\n");


Suggested change

.push_str("\nHint: the following dependencies have new major versions available:\n\n");

.push_str("\nThe following dependencies have new major versions available:\n\n");

compiler-core/src/dependency.rs

lpil · 2024-12-28T17:31:36Z

compiler-cli/src/dependencies.rs

+    fn should_use_manifest(mut self) -> Self {
+        self.use_manifest = UseManifest::Yes;
+        self
+    }


This is cool!

Another option which can be convenient at times is to have a struct with public fields and have an into_dependency_manager() method or something like that

I looked into builder pattern and init pattern (misnamed but similar to what you're suggesting). But as the tokio runtime, Package Fetcher and Telemetry needs to be specified, I cannot implement Default. Which means I'll have to define all struct fields at callsite. Which means any addition or change to the struct will require changes at all callsites.

Alternatively I could go for a builder, but seems too small right now.

Although...I could make it DependencyManagerConfig.into_dependecy_manager(runtime: ..., package_fetcher: ..., telemetry: ...).

lpil · 2025-01-09T13:59:45Z

Are you still planning to work on this? Thank you 🙏

andho · 2025-01-14T15:26:37Z

Are you still planning to work on this? Thank you 🙏

Hey yeah, will be working on this 🙂. Was a bit busy over the last 2 weeks, will be resuming it this week 🤞

lpil

Thank you! A few small things

lpil · 2025-01-20T19:29:20Z

compiler-cli/src/build.rs

+        telemetry,
+        None,
+        Vec::new(),
+        dependencies::DependencyManagerConfig::default(),


This is much less clear than before- please explicitly state each item of config. The programmer should be able to tell what the desired behaviour is at each call-site. This will mean removing the default implementation.

lpil · 2025-01-20T19:31:23Z

compiler-cli/src/dependencies.rs

+    pub mode: Mode,
+    pub use_manifest: UseManifest,
+    pub check_major_versions: CheckMajorVersions,


Document what each of these do please 🙏

lpil · 2025-01-20T19:32:12Z

compiler-cli/src/dependencies.rs

+    }
+}
+
+pub struct DependencyManager<Telem: Telemetry, P: dependency::PackageFetcher> {


Remove these constraints please 🙏 Constraints are only needed on the functions.

…st ✨

…err instead of stdout ♻️

…ns ✨

…eable ♻️

… calls ✨

…of `sort` ♻️

…cy resolution and major version resolution to reduce network calls to hexpm ♻️

Changed PackageFetcher to use impl or a generic. This will avoid dynamic dispatch and there is only one implementation for production so monomorphization will not increase the bin size.

… struct impl ♻️ The struct now holds the relevant dependencies that were being passed around with each function call.

…ownload ♻️

…s to reduce API churn ♻️

Although Rc is used, it is still cloned in dependency::DependencyProvider as it needs to sort the releases

…on message 💬

…ean ♻️

…ager with defaults ♻️

…ig ♻️

lpil

Beautiful work, thank you so much!

I've been manually testing this and it's really nice and useful, but it prints about dependencies being out of date before they are reported as having been added.

$ gleam add lustre@2 gleam_json@1
  Resolving versions
Downloading packages
 Downloaded 5 packages in 0.48s

Hint: the following dependencies have new major versions available:

gleam_json 1.0.1 -> 2.3.0
lustre     2.0.1 -> 4.6.3

      Added lustre v2.0.1
      Added gleam_json v1.0.1

I think it would make more sense the other way around

$ gleam add lustre@2 gleam_json@1
  Resolving versions
Downloading packages
 Downloaded 5 packages in 0.48s
      Added lustre v2.0.1
      Added gleam_json v1.0.1

The following dependencies have new major versions available:

gleam_json 1.0.1 -> 2.3.0
lustre     2.0.1 -> 4.6.3

What do you think?

andho · 2025-02-13T10:43:25Z

Beautiful work, thank you so much!

I've been manually testing this and it's really nice and useful, but it prints about dependencies being out of date before they are reported as having been added.
$ gleam add lustre@2 gleam_json@1
  Resolving versions
Downloading packages
 Downloaded 5 packages in 0.48s

Hint: the following dependencies have new major versions available:

gleam_json 1.0.1 -> 2.3.0
lustre     2.0.1 -> 4.6.3

      Added lustre v2.0.1
      Added gleam_json v1.0.1
I think it would make more sense the other way around
$ gleam add lustre@2 gleam_json@1
  Resolving versions
Downloading packages
 Downloaded 5 packages in 0.48s
      Added lustre v2.0.1
      Added gleam_json v1.0.1

The following dependencies have new major versions available:

gleam_json 1.0.1 -> 2.3.0
lustre     2.0.1 -> 4.6.3
What do you think?

Yeah the current output is confusing. But that's actually a bug. Major version availability should be shown only on update and deps download command as we discussed. I'll take a look.

PS. I'm traveling until 28th feb, so updates might be a bit slow.

andho mentioned this pull request Nov 22, 2024

Notify of new major versions available when running gleam update #3843

Closed

lpil marked this pull request as draft November 22, 2024 13:50

andho force-pushed the notify-major-version branch from 320e39f to fa633dc Compare December 6, 2024 03:33

andho marked this pull request as ready for review December 6, 2024 15:25

andho force-pushed the notify-major-version branch from 300e9eb to 2f1055d Compare December 9, 2024 11:02

lpil reviewed Dec 9, 2024

View reviewed changes

lpil marked this pull request as draft December 9, 2024 15:55

andho force-pushed the notify-major-version branch from 0781fc3 to c2e7c65 Compare December 12, 2024 16:21

andho force-pushed the notify-major-version branch 2 times, most recently from 100bf86 to 839318a Compare December 13, 2024 12:54

andho marked this pull request as ready for review December 13, 2024 13:03

andho changed the title ~~Draft: show latest major version available in gleam deps update~~ show latest major version available in gleam deps update Dec 14, 2024

andho requested a review from lpil December 14, 2024 13:05

lpil reviewed Dec 17, 2024

View reviewed changes

andho force-pushed the notify-major-version branch from ad3d34d to 22b1135 Compare December 28, 2024 06:45

andho requested a review from lpil December 28, 2024 06:47

lpil reviewed Dec 28, 2024

View reviewed changes

andho force-pushed the notify-major-version branch from 22b1135 to 4cf6967 Compare January 17, 2025 10:08

andho requested a review from lpil January 17, 2025 10:42

lpil reviewed Jan 20, 2025

View reviewed changes

andho force-pushed the notify-major-version branch from 8ba89b8 to 4a66951 Compare January 24, 2025 04:05

andho added 27 commits February 9, 2025 20:10

feat: only check for major version for direct dependencies ✨

555b303

feat: ignore pre release versions and sort versions before getting la…

40adf43

…st ✨

refactor(dependencies): output major version availability hint to std…

fa467ff

…err instead of stdout ♻️

refactor: move major version check into a separate testable unit ♻️

9464570

feat(dependencies): pretty print major version update hint into colum…

d2c3c62

…ns ✨

refactor: improve the test case so the direct dependency is not updat…

17a8251

…eable ♻️

feat(dependencies): cache package info from hexpm to reduce duplicate…

156b1f8

… calls ✨

refactor(dependencies): return formatted string and eprint ♻️

2533903

refactor(dependency): remove unnecessary clone and use max instead …

9cdeadb

…of `sort` ♻️

refactor(dependencies): use the same PackageFetcher during dependen…

9254144

…cy resolution and major version resolution to reduce network calls to hexpm ♻️

docs(dependencies): fix doc comment for cache_package 🐛 📝

2505df2

refactor: remove dyn for PackageFetcher ♻️

d2162c2

Changed PackageFetcher to use impl or a generic. This will avoid dynamic dispatch and there is only one implementation for production so monomorphization will not increase the bin size.

refactor(dependencies): move get_manifest and resolve_versions into a…

5fa6f96

… struct impl ♻️ The struct now holds the relevant dependencies that were being passed around with each function call.

refactor(dependencies): only check for major versions on update and d…

e515703

…ownload ♻️

refactor(dependencies): move DependencyManager config stuff to method…

9484bdd

…s to reduce API churn ♻️

refactor(dependencies): share package using Rc instead of cloning ♻️

f1367fe

Although Rc is used, it is still cloned in dependency::DependencyProvider as it needs to sort the releases

refactor(dependencies): Removed "Hint:" from major version notificati…

67ec003

…on message 💬

refactor(dependencies): use enum (CheckMajorVersions) instead of bool…

3640126

…ean ♻️

refactor(dependencies): add better api for initializing DependencyMan…

0ea9e6e

…ager with defaults ♻️

fix(dependencies): use PackageVersionDiffs everywhere applicable :fix:

7742a14

refactor(dependencies): add into_dependency_manager function ♻️

7175862

refactor(dependencies): remove default impl for DependencyManagerConf…

81eb8fe

…ig ♻️

docs(dependencies): add docs for DependencyManagerConfig struct fields 📝

78ab07f

fix(dependencies): remove unnecessary constraints 🏷️

703cbde

refactor(dependencies): remove mode from public interface ♻️

2b1c9df

refactor(dependencies): make DependencyManager private ♻️

3d0f10e

refactor(dependencies): move DependencyManager into a submodule ♻️

c347c0a

andho force-pushed the notify-major-version branch from 385bc90 to c347c0a Compare February 9, 2025 16:36

lpil reviewed Feb 11, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

show latest major version available in `gleam deps update` #3876

show latest major version available in `gleam deps update` #3876

andho commented Nov 22, 2024 •

edited

Loading

lpil commented Dec 2, 2024

andho commented Dec 5, 2024

andho commented Dec 6, 2024

inoas commented Dec 9, 2024 •

edited

Loading

lpil left a comment •

edited

Loading

lpil Dec 9, 2024

andho Dec 12, 2024 •

edited

Loading

lpil Dec 9, 2024

andho Dec 13, 2024

lpil Dec 9, 2024

andho Dec 13, 2024

lpil Dec 9, 2024

andho Dec 13, 2024

andho commented Dec 13, 2024

lpil left a comment

andho commented Dec 18, 2024

lpil commented Dec 19, 2024

lpil left a comment •

edited

Loading

lpil Dec 28, 2024

lpil Dec 28, 2024

lpil Dec 28, 2024

andho Dec 31, 2024

andho Dec 31, 2024

lpil commented Jan 9, 2025

andho commented Jan 14, 2025

lpil left a comment

lpil Jan 20, 2025

andho Jan 24, 2025

lpil Jan 20, 2025

andho Jan 24, 2025

lpil Jan 20, 2025

andho Jan 24, 2025

lpil left a comment

andho commented Feb 13, 2025

	.push_str("\nHint: the following dependencies have new major versions available:\n\n");
	.push_str("\nThe following dependencies have new major versions available:\n\n");

show latest major version available in gleam deps update #3876

Are you sure you want to change the base?

show latest major version available in gleam deps update #3876

Conversation

andho commented Nov 22, 2024 • edited Loading

Todos

lpil commented Dec 2, 2024

andho commented Dec 5, 2024

andho commented Dec 6, 2024

inoas commented Dec 9, 2024 • edited Loading

lpil left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

andho Dec 12, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

andho commented Dec 13, 2024

lpil left a comment

Choose a reason for hiding this comment

andho commented Dec 18, 2024

lpil commented Dec 19, 2024

lpil left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lpil commented Jan 9, 2025

andho commented Jan 14, 2025

lpil left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lpil left a comment

Choose a reason for hiding this comment

andho commented Feb 13, 2025

show latest major version available in `gleam deps update` #3876

show latest major version available in `gleam deps update` #3876

andho commented Nov 22, 2024 •

edited

Loading

inoas commented Dec 9, 2024 •

edited

Loading

lpil left a comment •

edited

Loading

andho Dec 12, 2024 •

edited

Loading

lpil left a comment •

edited

Loading