Hi Sven, Thanks for your reply.
On Sun, 5 Jan 2020 at 14:02, Sven Van Caekenberghe <[email protected]> wrote: > > Hi Guys, > > I would obviously want to avoid branches, as long as they are not really > needed. It's not obvious to me. :-) In fact given that Pharo is prepared to break backward compatibility (over time, and in a controlled manner), having a branch per release seems like a good option (not necessarily the only one, of course). > Alistair, looking again at your proposal, I must disagree even harder ;-) > > Right now, (as of 16 days ago), Zinc builds cleanly into Pharo 7 and 8, that > is absolute requirement for me. Of course, it might/will override some > changes in Pharo 8 that were not yet synced back. That is on the todo list. > If one of those changes is absolutely critical to you, we can add it quickly > (syncing the change back, in small chunks). That does require that it also > works on older Pharo versions, that there are comments, tests. Up to now, I > have always managed to make this work. I don't think the change I was proposing would make Zinc any less likely to build cleanly, it should actually increase the likelihood since internal changes to the core Zinc classes that don't affect the interface will not cause problems (in the current version they will). And in fact, it doesn't even introduce (git) branches (despite what I said above) :-) It would mean that there is only one copy of the core classes, which I expect would reduce your workload. > What is your particular problem ? Pavel did some refactoring in https://github.com/pharo-project/pharo/commit/45b7de2b6c901112be04230faba485a2f69ebb1e that moved some methods from the FileSystem-Core package to the Zinc-Resource-Meta-Core package, and they go missing when loading other Zinc packages, e.g. SSO. Thanks! Alistair > Sven > > > On 5 Jan 2020, at 03:14, Tudor Girba <[email protected]> wrote: > > > > Hi, > > > > Would introducing branches in Zinc to deal with the different versions of > > Pharo not be an option? > > > > Cheers, > > Doru > > > > > >> On Jan 3, 2020, at 2:48 PM, Alistair Grant <[email protected]> wrote: > >> > >> Hi Sven, > >> > >> On Fri, 3 Jan 2020 at 12:36, Sven Van Caekenberghe <[email protected]> wrote: > >>> > >>> Alistair, > >>> > >>> Thanks for the write up, it is useful. > >>> > >>> However, the general situation with the relationship between Zinc HTTP > >>> Components and Pharo 8 (or its most recent version in development) is > >>> often difficult. > >>> > >>> For me, as author and maintainer of Zinc HTTP Components is had been very > >>> hard to follow the changes in the latest Pharo and to keep > >>> syncing/merging them back. > >>> > >>> Especially since some changes are not backwards compatible with Pharo 7 > >>> (the latest stable) and the previous ones (let's say 6 and 5). For me and > >>> most users that is unacceptable: HTTP is way too important to not > >>> maintain it properly. The problems with source code file access were an > >>> example of that, luckily Pablo reacted super fast. > >>> > >>> Another point of friction for me are the many cosmetic changes that were > >>> applied, often using automatic means. They do not contribute much > >>> (functionally), but cause lots of trouble (to merge, check, validate). > >>> > >>> Normally, years ago, Zinc was synced into Pharo trunk regularly (like > >>> every 6 months or so). However, this has not happened for a long time. > >>> Why ? Because I have too much work syncing/merging everything back first. > >>> > >>> So I have to put time in this, but it won't happen soon - too much other > >>> work. > >>> > >>> Some more comments inline. > >>> > >>>> On 3 Jan 2020, at 11:34, Alistair Grant <[email protected]> wrote: > >>>> > >>>> Hi Sven, > >>>> > >>>> Following on from a discussion I started in Discord development... > >>>> > >>>> Loading Zinc SSO in Pharo 8 currently causes problems because the Zinc > >>>> packages in core Pharo 8 are out of sync with your repository > >>>> (specifically > >>>> https://github.com/pharo-project/pharo/commit/45b7de2b6c901112be04230faba485a2f69ebb1e > >>>> caused problems for me). > >>> > >>> Hmm, I guess I missed that one, but I would of course see it when trying > >>> to merge back. > >>> > >>>> I was able to resolve the issue by modifying > >>>> BaselineOfZincHTTPComponents>>baseline: to remove all references to > >>>> the packages that are now part of core Pharo 8: > >>>> > >>>> BaselineOfZincHTTPComponents>>baseline: spec > >>>> <baseline> > >>>> spec for: #common do: [ > >>>> spec baseline: 'NeoJSON' with: [ spec repository: > >>>> 'github://svenvc/NeoJSON:v17/repository' ]. > >>>> spec baseline: 'XMLParser' with: [ spec repository: > >>>> 'github://feenkcom/XMLParser:v0.6.1/src' ]. > >>>> spec baseline: 'XMLWriter' with: [ spec repository: > >>>> 'github://feenkcom/XMLWriter:v0.6.0/src' ]. > >>>> "spec project: 'XML Support' with: [ spec className: > >>>> 'ConfigurationOfXMLSupport'; repository: > >>>> 'http://www.squeaksource.com/MetacelloRepository' ]." > >>>> spec > >>>> package: 'Zinc-AWS'; > >>>> package: 'Zinc-REST' with: [ spec requires: #('NeoJSON') ]; > >>>> package: 'Zinc-WebSocket-Core'; > >>>> package: 'Zinc-WebSocket-Tests' with: [ spec requires: > >>>> 'Zinc-WebSocket-Core' ]; > >>>> package: 'Zinc-SSO-OAuth1-Core' with: [ spec requires: > >>>> #('NeoJSON') ]; > >>>> package: 'Zinc-SSO-OAuth2-Core' with: [ spec requires: > >>>> #('NeoJSON') ]; > >>>> package: 'Zinc-SSO-OpenID-Core'; > >>>> package: 'Zinc-SSO-Demo' with: [ spec requires: > >>>> #('Zinc-SSO-OAuth1-Core' 'Zinc-SSO-OAuth2-Core' > >>>> 'Zinc-SSO-OpenID-Core') ]; > >>>> package: 'Zinc-SSO-OAuth1-Tests' with: [ spec requires: > >>>> #('Zinc-SSO-OAuth1-Core') ]; > >>>> package: 'Zinc-SSO-OpenID-Tests' with: [ spec requires: > >>>> #('Zinc-SSO-OpenID-Core') ]; > >>>> package: 'Zinc-WebDAV'; > >>>> package: 'Zinc-WWS-Server'; > >>>> package: 'Zinc-WWS-Client'. > >>>> spec > >>>> group: 'AWS' with: #('Zinc-AWS'); > >>>> group: 'WebDAV' with: #('Zinc-WebDAV'); > >>>> group: 'WebSocket' with: #('Zinc-WebSocket-Core' > >>>> 'Zinc-WebSocket-Tests'); > >>>> group: 'SSO-OAuth1' with: #('Zinc-SSO-OAuth1-Core' > >>>> 'Zinc-SSO-OAuth1-Tests'); > >>>> group: 'SSO-OAuth2' with: #('Zinc-SSO-OAuth2-Core'); > >>>> group: 'SSO-OpenID' with: #('Zinc-SSO-OpenID-Core' > >>>> 'Zinc-SSO-OpenID-Tests'); > >>>> group: 'SSO-Demo' with: #('Zinc-SSO-OAuth1-Core' > >>>> 'Zinc-SSO-OAuth2-Core' 'Zinc-SSO-OpenID-Core'); > >>>> group: 'SSO' with: #('SSO-OAuth1' 'SSO-OAuth2' > >>>> 'SSO-OpenID' 'SSO-Demo' 'Zinc-SSO-Demo'); > >>>> group: 'WWS' with: #('Zinc-WWS-Server' 'Zinc-WWS-Client'); > >>>> group: 'REST' with: #('Zinc-REST') ] > >>>> > >>>> > >>>> This isn't directly usable by you since it replaces 'XML Support' with > >>>> XMLParser and XMLWriter and it would break Pharo 7. > >>> > >>> I don't think using feenk repos is a solution > >> > >> That's why I wrote that it isn't directly usable - it references > >> feenk, which will obviously have to be removed, and it is Pharo 8 > >> specific, more on that below. > >> > >>> , especially since Torsten did official ports of the XML stuff. I would > >>> then certainly not use a old metacello configuration in squeak source, > >>> but would go to all GitHub/Tonel. > >>> > >>>> But if we modified it to put in checks for Pharo 7 vs Pharo 8 it would > >>>> somewhat future proof the code against changes made in core Pharo 8. > >>> > >>> Like I said, I really want to support two versions back (so today 7 and > >>> 6). > >> > >> I think what I'm proposing would support this. To expand a bit more, > >> the idea is to have: > >> > >> baseline: spec > >> <baseline> > >> > >> SystemVersion current major <= 7 > >> ifTrue: [ self baseline7: spec ] > >> ifFalse: [ self baseline: spec ] > >> > >> > >> baseline7: would be the existing baseline. The new baseline: would be > >> changed to remove all the references to the packages in core Pharo 8 > >> (similar to the example above, but without the feenk references). > >> > >> > >>> I would like that Pharo code developers that make changes to Zinc/Zodiac > >>> packages at the very least acknowledge and be careful of the fact that > >>> Zinc exists outside of one particular Pharo image and has to support some > >>> older versions. > >> > >> This obviously isn't a panacea, but it will at least reduce the impact > >> of some changes made in the pharo-project repository. > >> > >> I'm happy to submit a PR if you agree, > >> > >> Cheers, > >> Alistair > > > > -- > > feenk.com > > > > "Every successful trip needs a suitable vehicle."
