Fix #18: add support for deprecated-versions #49

yvan-sraka · 2023-04-02T22:29:07Z

This is a modest work in progress Pull-Request, I wanted to double-check with you in first place if I effectively edited the right parts of the codebase?

app/Foliage/PreparePackageVersion.hs

app/Foliage/CmdBuild.hs

andreabedini · 2023-04-03T05:46:26Z

@yvan-sraka I am aware it's a draft but I left some comments to put you in the right direction. Ping me if you have questions!

yvan-sraka · 2023-04-11T08:02:44Z

@yvan-sraka I am aware it's a draft but I left some comments to put you in the right direction. Ping me if you have questions!

Thank you for your early review and comments, it definitely helped me! I just pushed my last week's work, it seems to works as expected against my local manual experiments. Are there some test scenarios I can run this PR against to be confident it's fully working as intended, if not, I could try to add them in another PR!

I left TODO code comments about some parts I'm not fully comfortable with, thank you again for your review :)

andreabedini

I reviewed the core changes, we can check the webpages when the core logic is working.

app/Foliage/Meta.hs

app/Foliage/PreparePackageVersion.hs

app/Foliage/CmdBuild.hs

michaelpj · 2023-04-13T15:02:26Z

Also perhaps this could be the PR that finally introduces some tests? :D

yvan-sraka · 2023-04-15T23:24:24Z

I reworked the core logic thanks to your feedbacks :)

yvan-sraka · 2023-04-16T10:04:46Z

Also perhaps this could be the PR that finally introduces some tests? :D

I started something simple here #50, the idea would be to write something End-2-End by spawning some HTTP server and then run Cabal against it! Is it the kind of tests you had in mind?

michaelpj · 2023-04-16T10:50:15Z

the idea would be to write something End-2-End by spawning some HTTP server and then run Cabal against it

Cabal can run against a file repository, so no need for the server. We could do this, but we could also do simpler things like just making some assertions about the content of the index tarball. Or we could try and use some logic from cabal the library to test if the repository looks right without having to run cabal separately.

andreabedini

The logic is still not correct. I had a go myself and, to be fair, the end result looks a bit convoluted:

We start from packagesVersions which is roughtly [(PackageId, [(Timestamp, Bool)])]

The deprecated versions are per package so we need to group per package name:

~ [(PackageName, [(PackageVersion, [(Timestamp, Bool)])])]

Then, for each package, we need to take all version deprecation changes, sort them by the timestamp, accumulate them into version ranges:

Flatten on the deprecation changes ~ [(PackageName, [(Timestamp, PackageVersion, Bool)])]
Accumulate into version ranges ~ [(PackageName, [(Timestamp, VersionRange)])]

Then, we flatten on the timestamp to spit out each index entry.

Flatten on timestamp ~ [(PackageName, Timestamp, VersionRange)]

I'll paste my code soon, just need to double check.

PS: There's a fair bit of whitespace change. I don't mind it di-per-se but I'd avoid changing whitespace back and forth. FWIW I use the default formatting with hls (which I think is ormolu but I am not sure).

app/Foliage/CmdBuild.hs

app/Foliage/PreparePackageVersion.hs

app/Foliage/Meta.hs

app/Foliage/PreparePackageVersion.hs

app/Foliage/CmdBuild.hs

andreabedini · 2023-04-18T06:42:31Z

Something like this

    let extraEntries =
          foldMap
            ( \packageGroup ->
                let pn = pkgName $ pkgId $ NE.head packageGroup
                    deprecationChanges =
                      sortOn fst $
                        foldMap
                          ( \PreparedPackageVersion {pkgId = PackageIdentifier {pkgVersion}, pkgVersionDeprecationChanges} ->
                              map
                                ( second $
                                    \deprecated ->
                                      if deprecated
                                        then intersectVersionRanges (notThisVersion pkgVersion)
                                        else unionVersionRanges (thisVersion pkgVersion)
                                )
                                pkgVersionDeprecationChanges
                          )
                          packageGroup
                    effectiveRanges =
                      NE.tail $
                        NE.scanl
                          (\(_, range) (ts, change) -> (ts, simplifyVersionRange $ change range))
                          (0, anyVersion)
                          deprecationChanges
                 in map
                      ( \(ts, effectiveRange) ->
                          mkTarEntry (BL.pack $ show effectiveRange) (IndexPkgPrefs pn) ts
                      )
                      effectiveRanges
            )
            (NE.groupWith (pkgName . pkgId) packageVersions)

It's only left ot make it readable (and to test it) 😂

Co-authored-by: Andrea Bedini <[email protected]>

yvan-sraka · 2023-04-20T14:48:26Z

PS: There's a fair bit of whitespace change. I don't mind it di-per-se but I'd avoid changing whitespace back and forth. FWIW I use the default formatting with hls (which I think is ormolu but I am not sure).

Weird, I use ormolu, but I can revert the whitespace changes :)

(Apparently)

andreabedini · 2023-05-03T07:36:17Z

@michaelpj I think this is now in a decent shape. Would you have time to give it another look and a test?

michaelpj · 2023-05-03T07:56:34Z

app/Foliage/CmdBuild.hs

+getExtraEntries :: [PreparedPackageVersion] -> [Tar.Entry]
+getExtraEntries packageVersions =
+  let groupedPackageVersions = NE.groupWith (pkgName . pkgId) packageVersions
+      generateEntriesForGroup packageGroup = map createTarEntry effectiveRanges


I generally appreciate type signatures for more complex functions like this!

I added the signatures and more comments.

app/Foliage/CmdBuild.hs

michaelpj · 2023-05-03T07:59:06Z

app/Foliage/CmdBuild.hs

+
+-- Apply a given change (`VersionRange -> VersionRange`) to a `VersionRange` and
+-- return the simplified the result with a new timestamp.
+applyChangeToRange :: (UTCTime, VersionRange) -> (UTCTime, VersionRange -> VersionRange) -> (UTCTime, VersionRange)


almost the applicative instance for Timestamped 😂

I guess it's

newtype Timestamped a = Timestamped (Maybe (Last UTCTime), a) deriving (Show, Eq, Ord) deriving (Functor, Applicative) via Ap ((,) (Maybe (Last UTCTime)))

app/Foliage/CmdBuild.hs

app/Foliage/Meta.hs

michaelpj · 2023-05-03T08:09:46Z

app/Foliage/Pages.hs

            -- original cabal file
            AllPackageVersionsPageEntryPackage
              { allPackageVersionsPageEntryPkgId = pkgId,
                allPackageVersionsPageEntryTimestamp = fromMaybe currentTime pkgTimestamp,
                allPackageVersionsPageEntryTimestampPosix = utcTimeToPOSIXSeconds (fromMaybe currentTime pkgTimestamp),
-                allPackageVersionsPageEntrySource = pkgVersionSource
+                allPackageVersionsPageEntrySource = pkgVersionSource,
+                -- FIXME: this weirdly seems to not work (display a `Deprecated` badge on all package version page) ...


going to fix it now or not? It potentially suggests a bug in how we're deciding if things are deprecated or not, which seems serious...

if only we had tests :)

Right, I forgot about that note. It was put there before lots of changes so I wonder it's still true.

There was a missing </td>, it's fixed now.

app/Foliage/PreparePackageVersion.hs

yvan-sraka · 2023-05-03T08:47:17Z

Should I perhaps rebase this PR on top of #51?

andreabedini · 2023-05-03T08:54:00Z

Should I perhaps rebase this PR on top of #51?

I should keep them separate, there should not be a big overlap. Also #51 doesn't seem to be working at the moment.

Likely disabled by accident.

- Add label also to revisions - Resize the label on package versions page

andreabedini · 2023-05-15T07:25:45Z

@yvan-sraka @michaelpj I want to merge this soon. Please have a last look.

michaelpj

Haven't tested (I assume you have), but looks fine!

andreabedini · 2023-05-15T09:01:40Z

I tested what I could. I guess we should just press this green button and see what happens 😂

yvan-sraka marked this pull request as draft April 2, 2023 22:29

andreabedini reviewed Apr 3, 2023

View reviewed changes

app/Foliage/PreparePackageVersion.hs Outdated Show resolved Hide resolved

andreabedini reviewed Apr 3, 2023

View reviewed changes

app/Foliage/CmdBuild.hs Outdated Show resolved Hide resolved

andreabedini reviewed Apr 3, 2023

View reviewed changes

app/Foliage/CmdBuild.hs Show resolved Hide resolved

andreabedini reviewed Apr 3, 2023

View reviewed changes

app/Foliage/CmdBuild.hs Outdated Show resolved Hide resolved

yvan-sraka force-pushed the deprecated-versions branch from 9a86e4c to 6031717 Compare April 11, 2023 07:56

yvan-sraka changed the title ~~[WIP] Fix #18: add support for deprecated-versions~~ Fix #18: add support for deprecated-versions Apr 11, 2023

yvan-sraka marked this pull request as ready for review April 11, 2023 07:57

yvan-sraka requested a review from andreabedini April 11, 2023 08:02

andreabedini requested changes Apr 13, 2023

View reviewed changes

yvan-sraka force-pushed the deprecated-versions branch from 427ee3e to 5b62981 Compare April 13, 2023 08:28

michaelpj reviewed Apr 13, 2023

View reviewed changes

app/Foliage/CmdBuild.hs Outdated Show resolved Hide resolved

yvan-sraka force-pushed the deprecated-versions branch 2 times, most recently from 5ea8ae8 to 4e349c6 Compare April 15, 2023 23:20

yvan-sraka force-pushed the deprecated-versions branch from 4e349c6 to 4974e12 Compare April 16, 2023 10:13

yvan-sraka requested review from michaelpj and andreabedini April 17, 2023 05:17

andreabedini requested changes Apr 18, 2023

View reviewed changes

yvan-sraka mentioned this pull request Apr 18, 2023

Ensure that the revisions numbeEnsure that the revisions numbers rs are ordered regarding timestamp? #52

Open

Fix input-output-hk#18: add support for deprecated-versions

7da6927

Co-authored-by: Andrea Bedini <[email protected]>

yvan-sraka force-pushed the deprecated-versions branch from 2e27d6b to 7da6927 Compare April 20, 2023 14:45

yvan-sraka requested a review from andreabedini April 20, 2023 14:48

andreabedini added 5 commits April 24, 2023 11:52

Put a bandage on cmdImportIndex

33edc77

Rework the metadata validation

16d75ec

Revisions at the same time as publication is allowed

62789a1

(Apparently)

Properly render version ranges

9c142e3

Add comments about revisions timestamps

9948cc7

andreabedini requested review from andreabedini and removed request for andreabedini May 3, 2023 07:35

michaelpj approved these changes May 3, 2023

View reviewed changes

yvan-sraka self-assigned this May 3, 2023

This was linked to issues May 3, 2023

Add support for deprecated-versions #18

Closed

Add support for preferred-versions #8

Closed

Merge branch 'main' into deprecated-versions

73a787e

andreabedini added 6 commits May 15, 2023 14:12

Add more comments

a93eb62

Remove dead code

754a1c1

Comments and formatting

285c8e3

Restore integrity validation for datatables.js

64f1578

Likely disabled by accident.

Fix page formatting issue

4f706f9

Improve deprecation label on the webpages

4cafbe7

- Add label also to revisions - Resize the label on package versions page

andreabedini approved these changes May 15, 2023

View reviewed changes

michaelpj approved these changes May 15, 2023

View reviewed changes

andreabedini merged commit 6c39112 into input-output-hk:main May 15, 2023

yvan-sraka deleted the deprecated-versions branch June 9, 2023 11:48

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix #18: add support for deprecated-versions #49

Fix #18: add support for deprecated-versions #49

yvan-sraka commented Apr 2, 2023 •

edited

Loading

andreabedini commented Apr 3, 2023

yvan-sraka commented Apr 11, 2023

andreabedini left a comment

michaelpj commented Apr 13, 2023

yvan-sraka commented Apr 15, 2023

yvan-sraka commented Apr 16, 2023

michaelpj commented Apr 16, 2023

andreabedini left a comment •

edited

Loading

andreabedini commented Apr 18, 2023

yvan-sraka commented Apr 20, 2023

andreabedini commented May 3, 2023

michaelpj May 3, 2023

andreabedini May 15, 2023

michaelpj May 3, 2023

andreabedini May 15, 2023 •

edited

Loading

michaelpj May 3, 2023

michaelpj May 3, 2023

andreabedini May 3, 2023

andreabedini May 15, 2023

yvan-sraka commented May 3, 2023

andreabedini commented May 3, 2023

andreabedini commented May 15, 2023

michaelpj left a comment

andreabedini commented May 15, 2023

Fix #18: add support for deprecated-versions #49

Fix #18: add support for deprecated-versions #49

Conversation

yvan-sraka commented Apr 2, 2023 • edited Loading

andreabedini commented Apr 3, 2023

yvan-sraka commented Apr 11, 2023

andreabedini left a comment

Choose a reason for hiding this comment

michaelpj commented Apr 13, 2023

yvan-sraka commented Apr 15, 2023

yvan-sraka commented Apr 16, 2023

michaelpj commented Apr 16, 2023

andreabedini left a comment • edited Loading

Choose a reason for hiding this comment

andreabedini commented Apr 18, 2023

yvan-sraka commented Apr 20, 2023

andreabedini commented May 3, 2023

michaelpj May 3, 2023

Choose a reason for hiding this comment

andreabedini May 15, 2023

Choose a reason for hiding this comment

michaelpj May 3, 2023

Choose a reason for hiding this comment

andreabedini May 15, 2023 • edited Loading

Choose a reason for hiding this comment

michaelpj May 3, 2023

Choose a reason for hiding this comment

michaelpj May 3, 2023

Choose a reason for hiding this comment

andreabedini May 3, 2023

Choose a reason for hiding this comment

andreabedini May 15, 2023

Choose a reason for hiding this comment

yvan-sraka commented May 3, 2023

andreabedini commented May 3, 2023

andreabedini commented May 15, 2023

michaelpj left a comment

Choose a reason for hiding this comment

andreabedini commented May 15, 2023

yvan-sraka commented Apr 2, 2023 •

edited

Loading

andreabedini left a comment •

edited

Loading

andreabedini May 15, 2023 •

edited

Loading