Re: The Trunk: Monticello-eem.512.mcz

This change is unsustainable and has broken the API contract with
other respository types.  The only required MCRepository API that
should be used by outside clients such as MCRepositoryInspector is:

  #allPackageNames - answer a list of package names in this repository.
  #basicStoreVersion: - add a Version to this repository.
  #includesVersionNamed: - does a version with this name exist in this
repository?
  #versionNamed: - answer the first Version object with the given name.
  #versionNamesForPackageNamed: - answer the version names for the
given package name.
  #versionWithInfo:ifAbsent: - answer the Version object with the
given unique VersionInfo

A detailed explanation is here:

  http://lists.squeak.org/pipermail/squeak-dev/2011-March/157825.html

I use other repository types besides FileBased which are now broken by this.

Eliot, is there some way to solve your issue without enumerating all
filenames of a repository?

On Thu, Jun 7, 2012 at 12:19 PM,  <[hidden email]> wrote:
> Eliot Miranda uploaded a new version of Monticello to project The Trunk:
> http://source.squeak.org/trunk/Monticello-eem.512.mcz
>
> ==================== Summary ====================
>
> Name: Monticello-eem.512
> Author: eem
> Time: 7 June 2012, 10:17:48.386 am
> UUID: a2addfac-e45d-4770-8f36-250da0d46b0b
> Ancestors: Monticello-cmm.511
>
> Provide a preference (Browse branched package versions separately)
> that makes the repository inspector show branches as
> separate groups.  e.g. makes it easier to find the newest
> VMMaker.oscog amongst the trunk versions.
>
> =============== Diff against Monticello-cmm.511 ===============
>
> Item was added:
> + ----- Method: MCFileBasedRepository>>allPackageAndBranchNames (in category 'overriding') -----
> + allPackageAndBranchNames
> +       | answer |
> +       answer := Set new.
> +       self allFileNamesOrCache do:
> +               [ : each | | versionName |
> +               versionName := each asMCVersionName.
> +               versionName isValid ifTrue: [ answer add: versionName packageAndBranchName ] ].
> +       ^ answer!
>
> Item was changed:
>   ----- Method: MCFileRepositoryInspector>>versionNamesForSelectedPackage (in category 'private') -----
>   versionNamesForSelectedPackage
>         ^ self allVersionNames select:
> +               (self class browseBranchedVersionsSeparately
> +                       ifTrue: [[:each| each packageAndBranchName = selectedPackage]]
> +                       ifFalse: [[:each| each packageName = selectedPackage]])!
> -               [ : each | each packageName = selectedPackage ]!
>
> Item was changed:
> + ----- Method: MCGOODSRepository>>allPackageNames (in category 'packages') -----
> - ----- Method: MCGOODSRepository>>allPackageNames (in category 'as yet unclassified') -----
>   allPackageNames
>         ^ self root collect:
>                 [ : ea | ea package name ]!
>
> Item was changed:
>   MCVersionInspector subclass: #MCRepositoryInspector
>         instanceVariableNames: 'repository packageNames versionNames selectedPackage selectedVersion order versionInfo loaded newer inherited'
> +       classVariableNames: 'BrowseBranchedVersionsSeparately Order'
> -       classVariableNames: 'Order'
>         poolDictionaries: ''
>         category: 'Monticello-UI'!
>
> Item was added:
> + ----- Method: MCRepositoryInspector class>>browseBranchedVersionsSeparately (in category 'class initialization') -----
> + browseBranchedVersionsSeparately
> +
> +       <preference: 'Browse branched package versions separately'
> +       category: 'Monticello'
> +       description: 'If true, versions of packages on branches will be shown in a separate section from trunk package versions'
> +       type: #Boolean>
> +       ^BrowseBranchedVersionsSeparately ifNil: [false]!
>
> Item was added:
> + ----- Method: MCRepositoryInspector class>>browseBranchedVersionsSeparately: (in category 'class initialization') -----
> + browseBranchedVersionsSeparately: aBoolean
> +       BrowseBranchedVersionsSeparately := aBoolean!
>
> Item was changed:
> + ----- Method: MCRepositoryInspector class>>order (in category 'class initialization') -----
> - ----- Method: MCRepositoryInspector class>>order (in category 'as yet unclassified') -----
>   order
>         Order isNil
>                 ifTrue: [ Order := 5 ].
>         ^Order!
>
> Item was changed:
> + ----- Method: MCRepositoryInspector class>>order: (in category 'class initialization') -----
> - ----- Method: MCRepositoryInspector class>>order: (in category 'as yet unclassified') -----
>   order: anInteger
>         Order := anInteger!
>
> Item was changed:
> + ----- Method: MCRepositoryInspector class>>repository:workingCopy: (in category 'instance creation') -----
> - ----- Method: MCRepositoryInspector class>>repository:workingCopy: (in category 'as yet unclassified') -----
>   repository: aFileBasedRepository workingCopy: aWorkingCopy
>         ^self new
>                 setRepository: aFileBasedRepository workingCopy: aWorkingCopy;
>                 yourself!
>
> Item was changed:
>   ----- Method: MCRepositoryInspector>>packageList (in category 'morphic ui') -----
>   packageList
>         | result loadedPackages |
>         packageNames ifNotNil: [ ^ packageNames ].
>         repository cacheAllFileNamesDuring:
>                 [ "Enjoy emphasis side-effects of populating my versionNames."
>                 self versionNames.
> +               result := self class browseBranchedVersionsSeparately
> +                                       ifTrue: [ repository allPackageAndBranchNames ]
> +                                       ifFalse: [ repository allPackageNames ] ].
> -               result := repository allPackageNames ].
>         "sort loaded packages first, then alphabetically"
>         loadedPackages := Set new: loaded size.
>         loaded do:
>                 [ : each | loadedPackages add: each packageName ].
>         result := result asArray sort:
>                 [ : a : b | | loadedA loadedB |
>                 loadedA := loadedPackages includes: a.
>                 loadedB := loadedPackages includes: b.
>                 loadedA = loadedB
>                         ifTrue: [ a < b ]
>                         ifFalse: [ loadedA ] ].
>         ^ packageNames := result collect:
>                 [ : each | self packageHighlight: each ]!
>

>> Eliot, is there some way to solve your issue without enumerating all
>> filenames of a repository?
>
> I'm out of my depth here.  Bert is the authority.  But I do know it is
> absolutely necessary to take account of the branch name.  Here's why:
>
> a) finding the head of a branch is extremely difficult if it is lumped in
> with the trunk.  For example in VMMaker the head of VMMaker is 284 while the
> head of VMMaker.oscog is 196.  So VMMaker.oscog is buried far down the list.
> This is sort-of sufferable by me, but not by newbies who likely won't find
> it.
>
> b) the "is the package modified" highlighting (emboldening and underlining)
> is entirely misleading if branches and trunks are lumped together.
>
> So please reconsider.  Monticello used to separate trunks and branches. The
> change that lumped them together make the system difficult to use for some
> packages (and I hope you agree VMMaker is kind of important).

I just want to understand better, not make anyone's life harder.  The
breakage can be "solved" by adding

   MCRepository>>allPackageAndBranchNames
       ^ self allPackageNames

but the reason I brought this up is because I put a lot of work into
getting MC's treatment of repository's cleaned up and under control so
that they all can work uniformly.  This change puts us back to
supporting only FileBased repositories, which are slow because they
can only do their work by enumerating all their files.

..I can't see that Monticello has ever had any notion of branches --
just its ancestry structure.  There's no "Branch" class of any kind
and no methods (other than the ones recently added).  Not even a
comment about it anywhere.

We already have at least 3 fields that make up a MCVersionName
(Package, author, version) and now this puts in a 4th.  It increases
complexity of the code and even adds a new Preference all just for
sorting items in one list widget -- this responsibility would seem
fall more on MCRepositoryInspector.

If there is no other way, fine, but currently it's mysterious and
breaks the system, so I thought I'd at least ask..

Thanks.

Ok, thanks for considering it.

Eliot, I already need to make a patch for fixing non-FileBased
repositories so rather than collide with two versions I can go ahead
and remove the preference.

I'll update the comment in MCVersionName as well... might need help on
that but I'll tap on Berts shoulder if I do..

On Thu, Aug 9, 2012 at 1:22 PM, Bert Freudenberg <[hidden email]> wrote:
> On 09.08.2012, at 19:53, Chris Muller wrote:
>
>> Before you spend time -- what think you (and Bert) of the suggestion
>> to have branches simply be in their own repository (its own directory
>> of the name of the branch)?  I know it's not how you do it today, but
>> please just imagine it in terms of the UI -- would it work?  Ok, there
>> might be issues diffing versions *between* branches -- is that a
>> frequent requirement?  Actually, they might both be in your
>> package-cache so may be could be diff'd there?
>
>
> It's always interesting to come up with alternatives. But I have given it a couple minutes of thought and come to the conclusion that no, branches cannot be meaningfully emulated by having multiple repositories. Encoding the branch in the version name is simple and works well.
>
> - Bert -
>
>
>