Tags and deps #551

gbaz · 2016-10-23T07:38:03Z

This is a cleanup of #514 that should supersede it. Changed revdeps in the display to give more useful numbers (after nub applied), made maintainers actually maintainers and not metadata uploaders, made display of already made votes work, improved table styling, added deprecation info to table, and other small UI improvements and cleanups. I'm pretty happy with the result.

I think there's more work to be done on the deploy branch with regards to improving the frontpage and some other docs, but I'm relatively happy here.

Cateories are parsed into tags. Helper functions to allow for tags to be proposed and accepted set up Cleaned up the interface

Added Author and Maintainer fields to PackageItem jQuery DataTables used for the page Added votes as a queryable parameter UI changes to propose/accept tags

A clickable UI for accepting/rejecting tags Added get method to /package/:pkgname/tags shifted forms to a separate template

When a trustee aliases Tag abcd -> Tag abc, all packages that were earlier tagged `abcd` get tagged to `abc` and any new packages tagged `abcd` get retagged `abc` on upload

Added backup functions for tag alias Fixed tag aliasing bug on startup

Add link-to-search on "Package not found" page display the names of the executables that a package can build

Limit revdeps to keep track of latest 5 versions Represent all the dep stats as ReverseCount HTML views for Revdeps by PackageID Popularity Metric based on RevDeps/Votes/Downloads Tag pages with packages by popularity

Add maintainers link to dataTables interface

Fixes category parsing error. "a,b," now parsed to ["a","b"] as opposed to ["a","b",""] Minor UI fixes Fixes tag-edit.html error (caused by rebase)

multiple maintainers (if present) show up in the search tables as opposed to just the latest uploader

adaptive doubling for rev deps graph

reverse dependencies and voting don't redirect spacing and UI fixes as pointed

A simple related packages view (currently based on bm25f results)

A visualization of the package database

Edited the output of /packages/search to use the datatables interface

It doesn't need to be in the interface for the core

We were writing changes into a channel but never reading from it.

Looks like dead code. It was being updated but never read.

It's fully constructing everything.

These days we pretty much have to have all GETS provide some sane cache control headers.

dcoutts

I've pushed a few changes directly. This will take a bit more review yet.

dcoutts · 2016-11-25T05:15:09Z

Distribution/Server/Features/ReverseDependencies.hs

-                let revs = constructReverseIndex index
-                update $ ReplaceReverseIndex revs
-          }
+    initReverseIndex :: IO ()


This is being run every time, so we're not actually benefiting from this as a persistent DB. We could just as well use a MemState and not have the complexities of persistent state.

So we keep this and get rid of the persistent component, right?

dcoutts · 2016-11-25T05:15:59Z

Distribution/Server/Features/ReverseDependencies.hs

+            index <- queryGetPackageIndex
+            let ReverseIndex _ revdeps nodemap = constructReverseIndex index
+                assoc = takeWhile (\(a,_) -> a < Bimap.size nodemap) $ Arr.assocs . Gr.transposeG $ revdeps
+            forM_ (map (\(a,b) -> (nodemap Bimap.!> a, map (nodemap Bimap.!>) b)) assoc) $ uncurry setReverse


This is almost certainly quadratic since it's doing each package one by one rather than in bulk.

dcoutts · 2016-11-25T05:16:36Z

Distribution/Server/Features/ReverseDependencies/State.hs

+        packageNodeIdMap = nodemap
+    }
+
+changePackage :: PackageId -> [PackageId] -> [PackageId]


This is unused. Should it be used?

dcoutts · 2016-11-25T05:18:27Z

Distribution/Server/Features/ReverseDependencies/State.hs

+
+arrDouble :: RevDeps -> RevDeps
+arrDouble rev =
+    Gr.buildG (0, 2*revSize rev) (Gr.edges rev)


This is not how immutable graphs work. Doubling the size helps nothing, since updating the graph has to copy anyway. That is, insEdges is already O(n), and if needs to add more nodes then that can be done in one pass at the same time, using accumArray with the new graph bounds.

dcoutts · 2016-11-25T05:19:47Z

Distribution/Server/Features/ReverseDependencies/State.hs

+    let index = PackageIndex.insert pkgid pindex
+        rd' | revSize revs < Bimap.size nodemap = arrDouble revs | otherwise = revs
+        npm = Bimap.tryInsert pkgid (Bimap.size nodemap) nodemap
+        rd = insEdges (map (\d -> (npm ! d, [npm ! pkgid])) deps) rd'


This graph enlarging ought to be done differently. Array size doubling is bogus for pure immutable arrays.

dcoutts · 2016-11-25T05:31:55Z

Distribution/Server/Features/Html.hs

+            resourceGet = [("html", serveReverseList)]
+          }
+      ]
+


Similarly these all need cacheControl

Ditto as above -- not quite sure what to do for these, though happy to consult.

dcoutts · 2016-11-25T05:34:02Z

Distribution/Server/Features/Html.hs

+    putAliasEdit :: DynamicPath -> ServerPartE Response
+    putAliasEdit dpath = do
+        tagname <- tagInPath dpath
+        mergeTags (Tag tagname)


This mergeTags isn't at the right abstraction level. It includes front-end things like looking up query params and then performing some state update. Instead all the front-end bits should live here and the code in the underlying feature should just deal with validation (if any) and the state/cache updates.

We can fix this by moving parts of the mergeTags code out to here.

dcoutts · 2016-11-25T05:37:47Z

Distribution/Server/Features/Tags.hs

+      addns <- optional $ look "addns"
+      delns <- optional $ look "delns"
+      raddns <- optional $ look "raddns"
+      rdelns <- optional $ look "rdelns"


These front-end bits don't really belong here. putTags should accept all these things as function args and the html layer should get them from the POST params and do the parsing.

dcoutts · 2016-11-25T05:38:38Z

Distribution/Server/Features/Tags.hs

@@ -195,20 +230,78 @@ tagsFeature CoreFeature{ queryGetPackageIndex
        pkgMap <- liftM packageTags $ queryState tagsState GetPackageTags
        return $ Map.fromDistinctAscList . map (\pkg -> (pkg, Map.findWithDefault Set.empty pkg pkgMap)) $ Set.toList pkgs

+    mergeTags :: Tag -> ServerPartE ()
+    mergeTags deprTag = do
+        tags <- optional $ look "tags"


These front-end bits don't really belong here. mergeTags should accept the tags as function args and the html layer should do the lookup and parsing.

dcoutts · 2016-11-25T05:39:09Z

Distribution/Server/Features/Html.hs

@@ -1519,26 +1599,50 @@ mkHtmlTags HtmlUtilities{..}
      pkgname <- packageInPath dpath
      _       <- lookupPackageName pkgname -- TODO: necessary?
      putTags pkgname
-      return $ toResponse $ Resource.XHtml $ hackagePage "Set tags"
-          [toHtml "Put tags for ", packageNameLink pkgname]
+      currTags <- queryTagsForPackage pkgname


Same point about mergeTags applies to putTags here.

dcoutts · 2016-11-25T05:50:36Z

Distribution/Server/Features/ReverseDependencies/State.hs

-    -- The main reverse dependencies map
-    reverseDependencies :: RevDeps,
+data ReverseIndex = ReverseIndex {
+    duplicatedIndex     :: !(PackageIndex PackageId),


It looks like this genuinely is redundant. It is used during lookups, but these are cases where we could grab the PackageIndex from the core feature and use that in combination. This will be a tad easier if we switch to using a MemState. Then the query functions will just need to take the ReverseIndex and the core PackageIndex to do their stuff.

It's not small so it'd be better if we can eliminate it.

dcoutts · 2016-11-25T05:55:28Z

Distribution/Server/Features/ReverseDependencies/State.hs

-  where forComponent (_, iftree, elsetree) = harvestDependencies iftree ++ maybe [] harvestDependencies elsetree
+constructRevDeps :: PackageIndex PkgInfo -> Bimap PackageId NodeId -> RevDeps
+constructRevDeps index nodemap =
+    let allPackages = concatMap (take 5) $ PackageIndex.allPackagesByName index


Any idea what this take 5 is about? Is it trying to take the 5 latest versions? Are we sure it's not actually getting the 5 oldest?

Also this is not consistent. This is only used on startup, but once we're in a steady state then we get all new versions not just the last 5.

dcoutts · 2016-11-25T06:22:38Z

Distribution/Server/Features/Html.hs

+          addns = toStr $ fst  revTags
+          delns = toStr $ snd  revTags
+      trustainer <- authorisedAsMaintainerOrTrustee pkgname
+      user <- authorisedAsAnyUser


I don't likad adding these new ad-hoc auth functions (and particular the new ones they require in turn), and it's better not to add new auth functions to reduce possible confusion. We can just fail immediately if they're not authorised as any user, so overall we can use:

uid <- guardAuthorised [AnyKnownUser] trustainer <- checkPriviledged uid [InGroup (maintainersGroup pkgname), InGroup trusteesGroup]

now ok having said lets not add new auth functions there's a reasonable argument that we should provide a wrapper for checkPriviledged in the Users feature that does what the underlying checkPriviledged does but supplies the Users db appropriately (which is what I'm implicitly using above). This at least isn't ad-hoc since it follows the existing pattern.

dcoutts · 2016-11-25T06:27:52Z

Distribution/Server/Features/Upload.hs

+
+    authorisedAsAnyUser :: ServerPartE Bool
+    authorisedAsAnyUser =
+        guardAuthorised' [AnyKnownUser]


I don't want to add these new auth functions, as mentioned above, but adding a wrapper here for checkPriviledged here is ok. e.g.

checkPriviledged :: UserId -> [PrivilegeCondition] -> ServerPartE Bool checkPriviledged uid privconds = do users <- queryGetUserDb Auth.checkPriviledged users uid privconds

dcoutts · 2016-11-25T06:30:17Z

Distribution/Server/Features/Users.hs

@@ -412,6 +421,11 @@ userFeature templates usersState adminsState
          overrideResponse <- msum <$> runHook authFailHook err
          throwError (fromMaybe defaultResponse overrideResponse)

+    -- Check if there is an authenticated userid, and return info, if so.
+    getAuthentication :: ServerPartE (Maybe (UserId, UserInfo))


This should be called checkAuthenticated since that's what it wraps and that's the pattern here.

dcoutts · 2016-11-25T06:33:15Z

Distribution/Server/Framework/MemSize.hs

 instance MemSize a => MemSize (IntMap a) where
  memSize m = sum [ 8 + memSize v | v <- IntMap.elems m ]

+instance MemSize Graph where
+  memSize m = sum [ 6 + memSize v | v <- Gr.edges m ]


We should just have an instance for ordinary boxed Arrays then this would be automatic. We've got one for unboxed arrays, boxed should be easier.

I don't know the right instance offhand, but feel free to go for it... (shouldn't be a blocker either way)

…ime in mem, avoid spurious doubling, don't dup index

…s for it

gbaz · 2016-11-26T05:55:37Z

Ok, so this should fix everything except for the cache control stuff, which I'll leave to @dcoutts (though I'm happy to consult). Everything with the revdep index is now created directly at startup with no persistence, but it should still be relatively fast I think. Worth testing with a full hackage index just to be sure. Also redid the auth stuff as requested, using getAuthorised' from the users feature, which already existed and was good enough...

gbaz · 2016-12-05T18:12:05Z

Any chance of reviewing this soon?

gbaz · 2016-12-19T16:11:23Z

?

gbaz · 2018-03-22T02:02:36Z

@hvr I think that with the new revdep branch I made there's nothing left on here that hasn't found its way elsewhere?

gbaz · 2023-01-01T18:37:37Z

everything has now been merged in pieces i believe.

Soorya Narayan and others added 20 commits October 9, 2016 21:53

Tags for review

65c956c

Cateories are parsed into tags. Helper functions to allow for tags to be proposed and accepted set up Cleaned up the interface

Browsable Package Index

87db09e

Added Author and Maintainer fields to PackageItem jQuery DataTables used for the page Added votes as a queryable parameter UI changes to propose/accept tags

Smoothened rough edges in tag proposals

6efcaa3

A clickable UI for accepting/rejecting tags Added get method to /package/:pkgname/tags shifted forms to a separate template

Tag Aliasing

138731c

When a trustee aliases Tag abcd -> Tag abc, all packages that were earlier tagged `abcd` get tagged to `abc` and any new packages tagged `abcd` get retagged `abc` on upload

Cleaning up earlier code

5c132eb

Added backup functions for tag alias Fixed tag aliasing bug on startup

Addresses #440 and #511

aa5138d

Add link-to-search on "Package not found" page display the names of the executables that a package can build

Polished voting interface

e5a512d

Miscalleneous

465f158

Limit revdeps to keep track of latest 5 versions Represent all the dep stats as ReverseCount HTML views for Revdeps by PackageID Popularity Metric based on RevDeps/Votes/Downloads Tag pages with packages by popularity

Maintainers link

7c65c71

Add maintainers link to dataTables interface

Minor Fixes

329ca61

Fixes category parsing error. "a,b," now parsed to ["a","b"] as opposed to ["a","b",""] Minor UI fixes Fixes tag-edit.html error (caused by rebase)

Multiple Maintaners

0ac3ce2

multiple maintainers (if present) show up in the search tables as opposed to just the latest uploader

Array Doubling

694f7cb

adaptive doubling for rev deps graph

Ajax(ed?) package page redirects

098f35b

reverse dependencies and voting don't redirect spacing and UI fixes as pointed

HLinted

078dd71

Related packages

e2e0138

A simple related packages view (currently based on bm25f results)

WebGL graph of packages and dependencies

879a1c7

A visualization of the package database

Tabular search interface

a7494a1

Edited the output of /packages/search to use the datatables interface

Cleaned up HTML Templates

4cc6547

whole lotta cleanup for summer of haskell project

2a8a049

cleanup default index

1ca833f

This was referenced Oct 23, 2016

HSoC: Tag Features #514

Closed

Hackage does not list executables in a package anymore #511

Closed

Add link-to-search on "Package not found" page #440

Closed

Gershom added 4 commits October 23, 2016 13:32

change a remaining 'votes' to 'ratings'

270d3b4

add missing datafiles

abcfbe8

fix silly sdist datafiles error

190c0a3

viewport fix

2c3e31c

gbaz mentioned this pull request Oct 23, 2016

Missing viewport meta tag #510

Closed

one more datafiles tweak

b4c4f35

gbaz mentioned this pull request Oct 23, 2016

Collapsable categories #99

Closed

dcoutts added 7 commits November 25, 2016 02:59

Move tagInPath from Core to where it's used

6fae681

It doesn't need to be in the interface for the core

Drop redundant results in revdep update transactions

fd6087d

Remove unused Chan of package id changes in revdep feature

7a07d3b

We were writing changes into a channel but never reading from it.

Remove unused MemState ReverseIndex in revdeps feature

f0313de

Looks like dead code. It was being updated but never read.

Make the ReverseIndex fields strict

613a160

constructReverseIndex doesn't need to use emptyReverseIndex

70b24cf

It's fully constructing everything.

Add a few cacheControls to the revdep feature GETs

0a8d0ea

These days we pretty much have to have all GETS provide some sane cache control headers.

dcoutts suggested changes Nov 25, 2016

View reviewed changes

dcoutts reviewed Nov 25, 2016

View reviewed changes

gbaz added 3 commits November 25, 2016 23:24

major reverse cleanup -- remove persistent state, do it all at init t…

7bc6340

…ime in mem, avoid spurious doubling, don't dup index

refactor auth handling and related for tags -- don't add new function…

8cea2c8

…s for it

drop ghc 7.6.3. from build matrix for travis

9471d1f

gbaz added 2 commits November 26, 2016 01:17

Wall cleanup, add functor constraints for ghc 7.8

aaf0267

further wall cleanup and 7.8 compat

923fd6b

This was referenced Jul 14, 2017

Reverse dependencies #40

Closed

Keep sets of tag aliases #24

Closed

Allow tags to be bulk edited #23

Closed

gbaz mentioned this pull request Mar 22, 2018

Reverse Dependencies #723

Closed

phadej mentioned this pull request Jul 1, 2018

suggestion: list dependent packages? #767

Closed

gbaz closed this Jan 1, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Tags and deps #551

Tags and deps #551

gbaz commented Oct 23, 2016

dcoutts left a comment

dcoutts Nov 25, 2016

gbaz Nov 25, 2016

gbaz Nov 26, 2016

dcoutts Nov 25, 2016

dcoutts Nov 25, 2016

dcoutts Nov 25, 2016

dcoutts Nov 25, 2016

dcoutts Nov 25, 2016

gbaz Nov 26, 2016

dcoutts Nov 25, 2016

dcoutts Nov 25, 2016

dcoutts Nov 25, 2016

dcoutts Nov 25, 2016

dcoutts Nov 25, 2016

dcoutts Nov 25, 2016 •

edited

Loading

dcoutts Nov 25, 2016

dcoutts Nov 25, 2016

dcoutts Nov 25, 2016

dcoutts Nov 25, 2016 •

edited

Loading

gbaz Nov 26, 2016

gbaz commented Nov 26, 2016

gbaz commented Dec 5, 2016

gbaz commented Dec 19, 2016

gbaz commented Mar 22, 2018

gbaz commented Jan 1, 2023

Tags and deps #551

Tags and deps #551

Conversation

gbaz commented Oct 23, 2016

dcoutts 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

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

Choose a reason for hiding this comment

dcoutts Nov 25, 2016 • 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

dcoutts Nov 25, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gbaz commented Nov 26, 2016

gbaz commented Dec 5, 2016

gbaz commented Dec 19, 2016

gbaz commented Mar 22, 2018

gbaz commented Jan 1, 2023

dcoutts Nov 25, 2016 •

edited

Loading

dcoutts Nov 25, 2016 •

edited

Loading