[VCS] Merging IAtomic* into IBasic*

[Matthew Woehlke]

Andreas Pakulat wrote:
> On 10.05.07 15:29:54, Matthew Woehlke wrote:
>> Andreas Pakulat wrote:
>>> On 10.05.07 13:00:13, Matthew Woehlke wrote:
>>>> Andreas Pakulat wrote:
>>>>> On 10.05.07 10:10:22, Matthew Woehlke wrote:
>> [snip] In any case, we agree that it should move to 
>> IBrowsableVC, right?
> 
> Hmm, that leaves us with just 1 method in an interface. That looks just
> wrong. I can think of only 1 solution: Move change() either to the Basic
> or Browsable interface and define that either the return value of
> change() may be invalid (for CVS for example) or that functions like ls
> don't have to work allways.

Honestly I can't say I'm against a one-method interface, especially if 
it is something many (but not all) VCS's implement. Isn't having a 
method that *always* fails sort-of counter to the purpose of having the 
interfaces anyway? :-)

>>> if( rev1 < rev2 ) then do something fancy, like rm -rf /home
>> Um... no? ;-) I think doing update() would be a better idea :-).
> 
> Well, replace it ... [snip]

I didn't say it wasn't useful (it is), just that 'rm -rf /home' didn't 
seem like a good idea (you BOFH, you ;-)). Of course unless you are root 
it will mostly fail, but still won't be good.

>> Any thoughts on what this function should look like? I'm not fond of 
>> operators because a comparison might mean consulting the repo (think of 
>> comparing a Date to a GlobalNumber) and might fail (comparing a 
>> FileNumber to anything else, or a Range to anything). I guess comparing 
>> two FileNumbers should assume that they represent the same file; the 
>> conversion would be painful otherwise.
>  
> Hmm, right. We could define that only two revision of the same type can
> be compared. As all our methods take a revision type that the user can
> request this should be enough.

That's fine, although while it is reasonable to compare e.g. a Date to a 
GlobalNumber, I think specifying that you can't do this means that the 
method can be offline and synchronous. In that case I guess I would be 
OK with actual operators if we can say that <, == and > are /all/  (is 
'.' a safe separator?allowed to be false for any given revision pair.

> About FileNumber: Those are actually meaningless without an associated
> path - IMHO - so maybe the revision should carry this information (the
> repo path) internally. Not sure if this should be made accessible, but
> it might make sense..

Ok... if I pay attention and read that correctly, then yes, that makes 
sense. Maybe we should note that this should be done, but it is really 
an implementation detail.

>>>> This brings up two other questions... Should we have a repositoryRoot() 
>>>> function?
>>> What for?
>> log(repositoryRoot(), VcsRevision::Head, limit) to get a list of all 
>> changes that have happened anywhere.
> 
> Hmm, I still can't see how this is useful information, especially in CVS
> (In svn it might be interesting, not sure about the usefulness though)

It would be useful if log(repoRoot) works on CVS. but I don't know if 
that's the case or not.

> Also the above doesn't work that way, there's no Repository class
> anywhere so repositoryRoot() would at least need a repoPath to start
> with.

Step back a second. Perforce is useless without P4USER, P4CLIENT and 
P4PORT, which are environment variables that do not appear 'as part of 
the file system' and are not defined in a repository path. So, either 
the instance of the VCS plugin needs to know this information, or 
repositoryRoot should take a /local/ path that would contain this 
information (in the latter case, using the perforce plugin would mean it 
creates .p4 files containing the server information).

Additionally, if the plugin is to not have state knowledge, then /every/ 
method must take a local path or repository path.

Incidentally, what happens if the plugin needs to ask you for a 
password? Use KWallet I guess?

>> Also it's pretty important for a remote repository browser to know :-)
>> (although I suppose it could require you to specify this).
> 
> Uhm, the vcs plugins don't have any state so they can't know the
> repositoryRoot() without supplying them some information for what
> file/repopath you want to know the root.

See above. This is a problem that needs to be resolved; if the plugin 
has no state information then you will have to do special things for the 
perforce plugin to be usable. Did I mention perforce in 3.4 is 
*completely, 100% broken/unusable* for exactly this reason?

So I guess in order to use a VCS Browser GUI (think 'websvn') you would 
have to tell it the server+root first? Maybe we need to rethink repo 
paths as containing three data items; user, server, path, and additional 
information as required by the VCS. You could still store this as a 
string, but that means the perforce backend would have to parse these 
critters all the time (not a big deal I guess, 'svn' is doing it 
anyway). So a perforce repo path now looks like 
'p4://mwoehlke_kde_trunk.mwoehlke at perforce.kde.org:1666/path/to/foo.c' 
(if it has to look /almost/ like a URL, I prefer making it look all the 
way like a URL :-)).

-- 
Matthew
"Ah, yes. Control the media and you control the world."
   -- from a story by Raven Blackmane