Include incompatible dependencies in `outdated` output #2254

iv-mexx · 2017-11-13T21:09:15Z

carthage outdated now lists all dependencies, even those that will not be updated because of the specified version in the Cartfile.

The output shows the current version, the latest compatible version to which the dependency will be updated when running carthage update as well as the latest available version.

Showing incompatible updates is very helpful to get a complete overview about a projects dependency. This is also the same behaviour as all the other dependency managers that I know of (E.g. Cocoapods, npm, yarn, bundler, ...).

In a follow-up PR (#2257) I will also add colors to the output to make it easier to read (which is also the behaviour of other existing dependency managers).

This PR is a continuation of #1369 (resolving #1260) which has not seen progress since more than a year.
I resumed the work from @mattprowse's branch to keep his commits, but in the meantime the codebase has changed considerably so I had to redo the most part. Actually, once I discovered that Resolver already does everything necessary, the latestDependencies all of a sudden was only 3 lines long 😌

Breaking Change

I had to change the signature of Project.updatedResolvedCartfile which is a public function in CarthageKit. In Carthage/CarthageKit it is not called from outsied of Project but since its public I don't know if it may be used somewhere else.

Earlier Comments

The two comments by @mdiep mentioned in #1369 (comment) have been discussed further down in this PR:

It doesn't work with dependencies that have no tagged versions.

Since this issue is not caused by this PR (it also happens before this PR), I will not try to fix it here.

It prints results for dependencies that are using a branch when there's nothing new on the branch. (Because it's not on the latest version.)

This has been taken care of in 5279bce

Replaced map() + flatten() with flatMap().

iv-mexx · 2017-11-14T12:49:01Z

Hi, I'm having a few questions:

Regarding the points raised by @mdiep in #1369 (comment)

It doesn't work with dependencies that have no tagged versions.

If i'm understanding correctly, the same does not work even before this PR. Carthage throws an error (No tagged versions found for <Dependency>) in Project.versions(for:), called by Project.updatedResolvedCartfile.

It prints results for dependencies that are using a branch when there's nothing new on the branch. (Because it's not on the latest version.)

E.g. FormatterKit "2ea246..." -> "2ea246..." (Latest: "1.8.0"). How should this be handled?

Keep as is
Omit from the output
Print separately (e.g. "There could be newer commits" so the user does not forget about those dependencies... Usually I pin to a branch or commit just when there is no release yet)
I guess we could also parse the git tree from that commit to find the actual newest version / commit (Seems overkill to me)

Regarding testing

Any hints how I would mock a Cartfile, Carfile.resolved and Latest Versions with specific Versions for a testcase?

mdiep · 2017-12-03T19:56:23Z

If i'm understanding correctly, the same does not work even before this PR. Carthage throws an error (No tagged versions found for ) in Project.versions(for:), called by Project.updatedResolvedCartfile.

If it's broken before, then I guess fixing it isn't a prerequisite here. But it'd be nice. 😉

E.g. FormatterKit "2ea246..." -> "2ea246..." (Latest: "1.8.0"). How should this be handled?

Ideally, it would fetch the branch and show you only if the branch resolves to a different SHA now.

But failing that, I think it could just be omitted.

Any hints how I would mock a Cartfile, Carfile.resolved and Latest Versions with specific Versions for a testcase?

Maybe look at ResolverSpec?

mdiep · 2017-12-04T17:42:47Z

When this is ready to be reviewed, please leave a comment saying so. ☺️

iv-mexx · 2017-12-04T17:43:41Z

Tests/CarthageKitTests/ProjectSpec.swift

+
+
+// swiftlint:enable identifier_name
+private struct DB {


This DB struct is copied 1:1 from ResolverSpec. Should it rather be extracted into a separate file?

Yes, please extract it and the github1, github2, etc. 👍

iv-mexx · 2017-12-04T17:55:06Z

@mdiep yup, will do! I still have to fix at least the issue with branches (the second note).
I will not tackle the first note currently though, maybe in a separate PR later.

…ositive outdated output

iv-mexx · 2017-12-04T18:39:39Z

@mdiep this is now ready for review! 🎉

iv-mexx · 2017-12-16T13:00:26Z

@mdiep 🏓

mdiep · 2017-12-16T17:52:51Z

Tests/CarthageKitTests/ProjectSpec.swift

+
+
+// swiftlint:enable identifier_name
+private struct DB {


Yes, please extract it and the github1, github2, etc. 👍

mdiep · 2017-12-16T17:53:33Z

Sorry for the delay!

This is looking good. Just need to extract the DB and resolve the conflict. 👍

mattprowse and others added 14 commits June 1, 2016 11:05

Modify carthage outdated to include incompatible dependencies.

2da9914

Merge upstream changes.

64565f0

Merge upstream changes.

a0cd215

Merge upstream changes.

3e1082c

Merge upstream changes.

71e2f78

Refactoring changes to Project.swift.

68fa6fc

Refactored Project's latestDependencies function.

7740c9b

Refactored Project's outdatedDependencies function.

4df02c2

Merge upstream changes.

cfdb135

Fixed formatting issues.

31b1051

Replaced map() + flatten() with flatMap().

Refactored latestDependencies().

4cf22fc

Refactor latestDependencies() and outdatedDependencies().

7ae6572

Merge branch 'master' into feature/outdated

1d81e1a

Outdated: Implement detection of the latest available version

4110d17

ikesyo mentioned this pull request Nov 14, 2017

[WIP] Include incompatible dependencies in outdated output #1369

Closed

iv-mexx added 5 commits November 14, 2017 11:14

Reduce latestDependencies into a dictionary right away

edc1730

Use typealias to fix large_tuple linter warning

c62f557

Clean up the condition for outdated dependencies

2a8008d

Keep the original pinned version when searching for the latest version

0b7164b

Add pending testcases

2367642

iv-mexx force-pushed the feature/outdated branch from d463047 to 2367642 Compare November 14, 2017 12:23

iv-mexx mentioned this pull request Nov 14, 2017

Outdated Colors #2257

Merged

iv-mexx added 3 commits December 4, 2017 16:53

Use Resolver to find the latest dependencies of the project

800684c

Respect includeNestedDependencies parameter of Project.outdated

6708f6e

Implement test for outdated dependencies

535236d

iv-mexx commented Dec 4, 2017

View reviewed changes

Handle case where a dependency is pinned to a branch to avoid false p…

5279bce

…ositive outdated output

iv-mexx changed the title ~~[WIP] Include incompatible dependencies in outdated output~~ Include incompatible dependencies in outdated output Dec 4, 2017

mdiep requested changes Dec 16, 2017

View reviewed changes

iv-mexx added 2 commits December 17, 2017 10:56

Merge branch 'master' into feature/outdated

406dd75

Extract DB into separate file for reuse from different test cases

e414eaf

mdiep approved these changes Dec 17, 2017

View reviewed changes

mdiep merged commit 1628d43 into Carthage:master Dec 18, 2017

ikesyo mentioned this pull request Dec 18, 2017

Should carthage outdated include incompatible dependencies? #1260

Closed

iv-mexx deleted the feature/outdated branch December 18, 2017 07:51

chuganzy mentioned this pull request Jan 19, 2018

[Bug] Fix a bug that carthage outdated fails #2328

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Include incompatible dependencies in `outdated` output #2254

Include incompatible dependencies in `outdated` output #2254

iv-mexx commented Nov 13, 2017 •

edited

iv-mexx commented Nov 14, 2017

mdiep commented Dec 3, 2017

mdiep commented Dec 4, 2017

iv-mexx Dec 4, 2017

mdiep Dec 16, 2017

iv-mexx commented Dec 4, 2017

iv-mexx commented Dec 4, 2017

iv-mexx commented Dec 16, 2017

mdiep Dec 16, 2017

mdiep commented Dec 16, 2017

Include incompatible dependencies in outdated output #2254

Include incompatible dependencies in outdated output #2254

Conversation

iv-mexx commented Nov 13, 2017 • edited

Breaking Change

Earlier Comments

iv-mexx commented Nov 14, 2017

Regarding the points raised by @mdiep in #1369 (comment)

Regarding testing

mdiep commented Dec 3, 2017

mdiep commented Dec 4, 2017

iv-mexx Dec 4, 2017

Choose a reason for hiding this comment

mdiep Dec 16, 2017

Choose a reason for hiding this comment

iv-mexx commented Dec 4, 2017

iv-mexx commented Dec 4, 2017

iv-mexx commented Dec 16, 2017

mdiep Dec 16, 2017

Choose a reason for hiding this comment

mdiep commented Dec 16, 2017

Include incompatible dependencies in `outdated` output #2254

Include incompatible dependencies in `outdated` output #2254

iv-mexx commented Nov 13, 2017 •

edited