Ticket #278: cvstrac for Subversion?

I'd like to see CVStrac able to work against Subversion repositories. SVN has some compelling features, and we are considering switching from CVS. But how can we live without CVStrac's fabulous timeline and code browsing features? ;^)
[Append remarks]

Remarks:

2004-Mar-25 14:59:20 by anonymous:
have a look at trac. a very nice "svntrac". it's written in python, which makes it far easier to develop than cvstrac. and it can plug into all those lovely external python packages/bindings (docutils, svn, ClearSilver, ...).

i would mark this ticket closed.

2005-Jul-31 21:47 by anonymous:

Well, I sure as hell wouldn't mark this ticket closed. Trac is a dependency nightmare that has nothing to do with cvstrac's cleanliness and simplicity. You just try installing Trac on a modern Linux distribution and figure out the dependencies - especially those that are now outdated. Python is nicer to develop in, sure, but you can't just "drop in" Trac on any server without spending a couple of days going back and forth through the install and uninstalling Python 2.4 (which, incidentally, will break any modern Fedora install, since all the admin tools depend on it).

I, for one, would like to see a "svntrac" that was as easy to deploy and run as cvstrac, with zero external dependencies.

So please, could we re-open this and consider this as a way forward for cvstrac?

2005-Jul-31 23:13:08 by cpb:
Um... Did I just read a remark that basically says "Trac sucks and CVSTrac developers should do something about it"?

2005-Aug-01 02:22:00 by anonymous:
Funny, I thought I read the same :)

I was just going through the cvstrac sources a few hours ago, trying to figure out how hard it would be to "port" cvstrac to other vcs's (svn in the first place, though monotone could be interesting too). Would it be feasible at all to make some kind of compile time vcs abstraction layer in cvstrac or simple fork would be better?

Either way, I'd like to see some "svntrac" that will keep all the above outlined features of cvstrac!

2005-Aug-01 03:59:15 by cpb:
Would it be feasible at all to make some kind of compile time vcs abstraction layer in cvstrac or simple fork would be better?

Within reason, yes. It'd be a pain to support anything other than a fairly conventional VCS (CVS, SVN, directory oriented, version numbers, etc). Something like monotone... it would probably work, but I don't think a monotone user would feel comfortable with how CVSTrac presents things.

Near as I can see, there's only two places with CVS-specific dependencies. The rcsdiff stuff in timeline.c and the history file processing stuff in history.c. Abstracting the history stuff is the hard part, although it might be easier with SVN. A huge chunk of the logic in update_history() is just spent trying to coalesce discrete directory updates into the same checkin.

2005-Aug-02 01:28:19 by anonymous:
I looked a little deeper into this, and I didn't really like what I saw :(

rcsdiff stuff in timeline.c shouldn't be a problem since same can be done with svn diff command.

But history_update() in history.c could be problematic. Problem here is that svn doesn't have anything similar to CVSROOT/history. Read this.
The right way of dealing with this, I guess, would be to use svn_client API. But that would mean we need to link against svn AND apr, hence 2 more dependecies that we don't want. I guess I won't be wrong if I say that this is not an option for us?!

Only other way I see, might be to interpret output of some svn commands like svn log for example. I'm not sure we can get what we need from it, I'll have to look into it when I find some time.
But since svn assigns revision number to whole tree, I think it might be posible to keep track of last revision we entered into db as oposed to size of CVSROOT/history. Then we'd just need to parse each individual revision between our last revision and HEAD.

Parsing command's output is not a good idea since it makes our code vulnerable to future versions of svn that might alter the output format of svn log, but at the moment I don't see any other way to make it svntrac ; )
If anyone thinks there is a better way to implenet history_update(), please post your ideas here.

I'll post here again when I find out if it's possible to get all the info we need from svn command line client.

2005-Aug-02 11:55:50 by anonymous:
There's a CVSTrac patch floating around out there that's supposed to work over CVS pserver. It may do something similar to "svn log". I'll try to track down the URL.

c.

Ah, here: http://burstproject.org/cvstrac_hack/

2005-Aug-02 23:56:27 by cpb:
All history_update() does is scans for changes in the repository since the last update (last being a bit variable, since it can be reset entirely for a rescan by resetting historysize in the config table, but I digress).

We're not talking about a terribly complicated thing, really:

As a side-effect, it fills the database with stuff. Specifically, it touches the chng and filechng tables and then updates the historysize field in the config table. Some of this could be abstracted to some function calls commonly used by any SCM stub, but it's so simple I wouldn't bother. It might look like there's a lot of other things happening to the database, but that's all temporary tables in order to try to create "atomic" checkins. It doesn't always work, either.

If you don't care about speed or accessing remote subversion repositories (not a good idea, history_update() is called for every hit), I'd think you could start something by parsing svnadmin dump and optimize from there. If I actually used subversion, I'd probably start there.

2005-Aug-06 03:19:11 by anonymous:
I now have a working svntrac :-)
Not all features of cvstrac are working, but it is usable. So I'd like to ask cvstrac developers if there is any intrest in integrating this work with cvstrac HEAD and if so what would be the preferred way to do it.

Needles to say, cvstrac vs svntrac is a compile time decision. My question is how should I organize the code so it can be easily integrated into cvstrac and both versions can be maintained independently?
I was thinking about moving some functions (such as history_update(), for example) into some new file and maintain two separate version of those functions in there with some #ifdef's. I'd also like to leave all WEBPAGE: /page functions where they are now and use #ifdef's there as necessary. I think this should leave the code in maintainable state as there aren't too many of those #ifdef's needed.

Of course, I'm open for ideas and suggestions.

2005-Aug-06 15:07:41 by cpb:
Wow, that's great!

Well, I'd start by attaching a diff so we're all on the same page. But in general, I don't see any reason why not to integrate something like this.

I personally think it would be nice if both CVS and Subversion support was built into the same binary, just to ensure that both will always compile if, say, API calls change, and select the appropriate subsystem based on the executable name (cvstrac vs svntrac) or something.

This would also avoid a potential boatload of #ifdef's (especially if more SCM's are added). If any individual pages are SCM specific, you can just do something like: 

  if( strcmp(g.scm,"cvs") ){
    @ Unsupported by %z(g.scm)
  }else{
    /* do something */
  }

However, this more dynamic approach might be a bit harder to start with. drh might have other preferences, too. Don't take my opinion as the final word on anything.

I agree that it's necessary to move the SCM-specific parts into separate files. cvs.c and svn.c, perhaps, and prefix any of the calls with the appropriate SCM name (i.e. cvs_update_history(), svn_diff(), etc). Pulling out parts of the various functions into common calls wouldn't hurt, either (i.e. history_update_add_chng(), history_update_add_file(), history_reset()).

2005-Aug-06 17:55:10 by anonymous:
I just attached the diff. One note about it, svntrac was not codded against cvstrac HEAD while this diff is, so there are some diffs in there that are actually just updates to cvstrac HEAD and not svntrac. Hopefully this is still useful.
This diff is just plain svntrac, it is no wear near ready to be merger into cvstrac HEAD and I won't start working on that until we agree on how it should be done. This is just for developers to see what has to changed in cvstrac. If you want to test this code, checkut cvstrac HEAD and apply this diff against it and it should compile and run. If not, you can report your problems here but don't expect too much support until svntrac is merged into cvstrac.

One other note, this is very untested code! I was developing and testing this on Windows (my linux box is still awaiting some hardware to arrive :( ) under cygwin. This code had limited testing only in standalone server mode. No chroot, no cgi etc. was tested.

I don't have time right now to discuss this in detail, but your idea about some run time detection of scm sounds interesting but we'll have to see how could it be implemented and of course we still need to hear what other people think about it.

There still are some issues that I'm aware of and that I can resolve, but I'm reluctant to start working on them before we merge.

Got to run now, but if there are any ideas or objections about this integration, lets hear them!

2005-Aug-06 21:53:34 by cpb:
It is a bit rough as patches go, but getting the Subversion support functioning doesn't seem too intrusive.

The chngtype field being added to the filechng table isn't a bad idea, but is there a particular reason why you can't just see if a prior versions exists or not in order to detect adds? chngtype is only checked in one place, after all, so it seems like overkill to add a field to the schema...

2005-Aug-07 03:07:59 by anonymous: 

   > It is a bit rough as patches go, ...

I'm not rally sure I follow you here, can you elaborate on it a bit? 

  > The chngtype field being added to the filechng table isn't a 
  > bad idea, but is there a particular reason why you can't just see  
  > if a prior versions exists or not in order to detect adds?
  > chngtype is only checked in one place, after all, so it seems 
  > like overkill to add a field to the schema...

You're absolutly right, there is no real need for it.

2005-Aug-07 13:47:48 by cpb:
I'm not really sure I follow you here, can you elaborate on it a bit?

Oh, just agreeing when you say that the diff isn't ready for merging. I noticed, for example, all the CVS-specific code that got commented out... ;)

I'll see if I have time today to make some changes so the problem areas are simplified into separate API calls... Actually, while I'm at it, I should probably create a CVS branch for this stuff until it stabilizes...

2005-Aug-07 15:35:07 by anonymous: 

  > I'll see if I have time today to make some changes so the problem areas are simplified into separate API calls...
I guess that means we're going for run time scm detection? Not that I think it's a bad idea, actually quite opposite, just want to make sure we're "in sync" ;)


  > Actually, while I'm at it, I should probably create a CVS branch for this stuff until it stabilizes...
Yeah, that would be good too

One off topic note: While replying to these remarks here I noticed how annoying it is to scroll to top of the page just to click on that "Append remarks" link. I think it would be useful to have that link below the remarks too. Or even just below remarks, but I guess having it in both places is most convenient.

2005-Aug-07 15:44:50 by anonymous:
I removed schema 1.8, made some changes with previous_version() and added that second "Append remarks" link I was talking about.
My current diff is attached.

2005-Aug-07 22:45:40 by cpb:
Put the "append remarks" issue in a separate ticket. It's not a bad idea, but it's best not to mix it up with the svntrac stuff.

I'll go with runtime SCM testing. I like the idea of knowing that at least all parts of the code will build, even if I can't test both code paths.

Unfortunately (for svntrac) I spent most of the day doing plumbing. Might take a stab at it tonight.

2005-Aug-07 23:05:30 by anonymous: 

  > Put the "append remarks" issue in a separate ticket. It's not a 
  > bad idea, but it's best not to mix it up with the svntrac stuff.

Of course, what was I thinking, if I was at all :D
I'll do that right away. 

  > I'll go with runtime SCM testing. I like the idea of knowing that 
  > at least all parts of the code will build, even if I can't test 
  > both code paths.

Yes, that is the most appealing advantage of it I'd say. For now at least.
Hopefully increased binary size won't cause any problems to anyone.

2005-Aug-08 01:53:37 by cpb:
Okay, that should be enough for now. This is all on the svntrac-patches branch, of course.

In a nutshell, the only file that should need any changes to fully support Subversion is svn.c. That is, to get the same functionality as with CVS. If there's anything other than the odd 

  if(!strcmp(g.zSCM,"<blah>")){
  }

needed outside of svn.c besides cosmetic stuff (like talking about CVS everywhere), I'd be surprised.

I did notice that you've got a distinction between file diffs and revision diffs. I think that the revision diff concept is basically the patchset stuff that I did in #357. If that's the case, it might be best to just integrate that functionality by adding a dump_patchset(chng) function to history.c?

2005-Aug-08 02:31:27 by anonymous: 

  > I did notice that you've got a distinction between file diffs and revision diffs.

I had to make that distinction because Subversion is revision centric, as opposed to CVS_'s file centricness. AFAIK _svnlook doesn't do diff on indiviual files, but rather on whole repository. So in order to get file's diff I have to extract it from revision's diff. Not very nice, but that's how it is right now :( I even looked at API's available in libsvn* and there are no funcs to do diff on files, at least not without working copy.

While we're at it, svntrac doesn't need previos version information for diff at all (except to make sure this is not the first revision this file shows in), so I made that ugly hack in cvstrac's code to pass information weather I need to extract file's diff or just dump the whole revision diff. That code has some rough edges and I need to work on it some more so I might be spliting that func into separate functions for file diff and revision diff.

I was thinking about adding some code to those funcs to count number of lines changed in revision/file since Subversion doesn't expose this information anywhere (and won't at least until version 2). This would be pretty simple proces, just count the number of '+' and '-' chars in first column, but I don't know where to do it. It would be most convinient to do it in history_update(), but that would mean I'd have to parse diff's output there so it would be pretty big slowdown for first run of history_update(). Other place I can do it at, is in diff_versions(). That would involve a bit more code, some temp files and some modifications to /chngview to allowe diff_versions() to count those lines before those numbers need to be shown.

2005-Aug-08 02:52:11 by cpb:
Oh... That makes sense, I guess. Although... svn diff won't work?

It'd be better to separate the concept of file and revision diffs. A revision diff is, basically, just a diff done against a chng number, so it could/should be worked in as a basic API call. That's definitely what #357 is about. I'll have a look at it tomorrow.

The only thing I can suggest with the line count thing is that if it's too hard to do efficiently, don't implement it. I highly doubt it's really critical functionality for most people...

2005-Aug-08 03:13:13 by anonymous:
Oh boy, do I miss that "Append remarks" link after the remrks :D 

  > Although... svn diff won't work?

svn, as in command, is a client and works only with local working copy. While svnlook is meant for what we need it for, access information about local repository without working copy. 

  > ...svntrac doesn't need previos version information for diff at all...

Just to clarify my self a bit, svntrac does need previous version information, for reasons stated in my previous remark, but it gets it from db based on new version information. 

  > It'd be better to separate the concept of file and revision diffs. A revision diff is, 
  > basically, just a diff done against a chng number, so it could/should be worked in as a basic API call.

Yes, that would probably be nice, it would make things somewhat more clear and even easier in case of svn. 

  > That's definitely what #357 is about.

I can't really see how's that related? But it might be just that it's really late. 

  > The only thing I can suggest with the line count thing is that if it's too hard to do efficiently, 
  > don't implement it. I highly doubt it's really critical functionality for most people...

It's not implemented at the moment, but I wanted to have it since it's not complicated to do, aids to compatibility with cvstrac and I think it's usefull :D
Only problem is how much would it hurt performance. From performance perspective it would be best to gather that info on the first hit to /chngview since in that case I can avoid the temp file thing. But then that info won't be available for that first hit. Anyway, there's bigger fish to fry at the moment so we'll put it on hold for now.

2005-Aug-08 12:02:35 by anonymous:
Ah, okay, svn diff won't cut it then...

As for the patchset thing... A patchset is basically the diff for the entire chng entry, which in theory is supposed to be a single atomic commit. That would, if I understand things correctly, correspond to a Subversion revision? In the case of CVS, a chng only looks atomic, but in reality it's a merging of a whole bunch of different commits around the same time with the same log message (and only during a single call to update_history()).

Another way to put it is that the collection of file changes you see when you look at a chngview page is basically a patchset. All that #357 tries to do is turn that into something which you can run through patch(1) and not end up with garbage.

2005-Aug-08 13:44:17 by anonymous: 

  >As for the patchset thing... A patchset is basically the diff for the entire 
  >chng entry, which in theory is supposed to be a single atomic commit.

And in svn it is. 

  > That would, if I understand things correctly, correspond to a Subversion revision?

Exactly! And whole revision is all I have to work with, no concept of files here. 

  > In the case of CVS, a chng only looks atomic, but in reality it's a merging 
  > of a whole bunch of different commits around the same time with the same
  > log message (and only during a single call to update_history()).

Yes, I saw that while hacking update_history() 

  > Another way to put it is that the collection of file changes you see
  > when you look at a chngview page is basically a patchset.

I agree here. 

  > All that #357 tries to do is turn that into something which you 
  > can run through patch(1) and not end up with garbage.

Right. Now that we've sorted out concept of revision, let me say what I think #357 is all about.
Basically it's a request to have /chngview output "same" content, but in patch friendly format that can be downloaded and fed to patch command.

I don't really use patch so I don't know how input should be formatted (I'll look that up) but I think #357 could and should be implemented.
Maybe have link "Download check-in as a patch" or something like that

2005-Aug-08 16:10:41 by cpb: 

  Basically it's a request to have /chngview output "same"
  content, but in patch friendly format that can be downloaded
  and fed to patch command.

Pretty much. Essentially, a CVSTrac chng and a patchset (as defined by #357) are all the same underlying information, but presentation is different depending on what URL you hit.

So while the hard part with CVS is making a bunch of file commits look like an atomic chng and then, for something like #357, building a viable patch file, with Subversion the issue is making a single atomic revision look like separate file commits and diffs.

2005-Aug-08 16:59:56 by anonymous: 

  > Pretty much. Essentially, a CVSTrac chng and a patchset (as defined by #357) 
  > are all the same underlying information, but presentation is different
  > depending on what URL you hit.

It's great to know we're on the same page here ; ) 

  > So while the hard part with CVS is making a bunch of file commits look like 
  > an atomic chng and then, for something like #357, building a viable
  > patch file, with Subversion the issue is making a single atomic revision
  > look like separate file commits and diffs.

True, but also a bit ironic, isn't it? : D

2005-Aug-09 03:45:17 by anonymous:
I attached my latest diff. A few problems that I ran into:

2005-Aug-09 03:50:02 by anonymous:
Of course, I forgot something. In svn.c there are some SCM COMMON funcs at top of the file. Those funcs are just copied from cvs.c so maybe it would be good to also have some common_scm.c file for stuff like that?

2005-Aug-09 11:31:11 by cpb: 

  I need to know inside svn_diff_versions() from what page
  it was called (chngview vs. filediff).

Even better, how about instead of iterating through the files in the chng calling diff_versions() we could just change the semantics of diff_chng() to be a pretty-printed (i.e. HTML, changing the current diff_chng() call to, say, patchset_chng()). Then there's just the one call needed in /chngview which you can implement in as optimized a fashion you'd like. 

  That chngtype field that I initially added would be right at
  home here. Any chance of introducing something like that?

If there's no other way to do it, sure. I was also kicking around the idea of adding a prevvers field to make the previous_revision() call considerably nicer. It would also remove the is_dead_revision() call entirely unecessary. 

  That change to browse.c is a fix for general cvstrac bug

Normally whatever generates the URL would handle that, but the custom markup stuff really makes CVSTrac internal URLs more accessible. I'll commit that, but probably tonight (i.e. +10 hours). 

  maybe it would be good to also have some common_scm.c file
  for stuff like that?

Just move things from cvs.c back into history.c... I made svn.c as lean as possible so things were nice and clear about what needed to be done and since I had no solid idea what you needed.

2005-Aug-09 14:11:13 by anonymous: 

  Even better, how about instead of iterating through the files in the 
  chng calling diff_versions() we could just change the semantics of 
  diff_chng() to be a pretty-printed (i.e. HTML, changing the current 
  diff_chng() call to, say, patchset_chng()). Then there's just the one 
  call needed in /chngview which you can implement in as optimized a 
  fashion you'd like.

I did this for svntrac. diff_chng() in cvstrac needs some rework though. 

  If there's no other way to do it, sure. I was also kicking around 
  the idea of adding a prevvers field to make the previous_revision() 
  call considerably nicer. It would also remove the is_dead_revision()
  call entirely unecessary.

svn_previous_version() already operates on db only, but adding prevvers field wouldn't do any harm to svntrac either since it may improve performance for large databases. 

  Just move things from cvs.c back into history.c

Did that too, diff attached.

2005-Aug-10 02:44:22 by cpb: 

  adding prevvers field wouldn't do any harm to svntrac either
  since it may improve performance for large databases.

Okay, over to you. I used integer fields for chngtype since it better follows existing CVSTrac conventions.

2005-Aug-10 15:18:29 by anonymous: 

  > Okay, over to you. I used integer fields for chngtype since it better
  > follows existing CVSTrac conventions.

Speaking about conventions, I see there is alot of white space noise in my diffs. In that regard, is there some cvstrac coding style that would outline the preffered use of spaces vs. tabs, brace positioning, variable naming etc.
If not, would some of current cvstrac developers be willing to write a few lines about that?

2005-Aug-11 00:38:48 by anonymous: 

  > Okay, over to you. I used integer fields for chngtype since it better 
  > follows existing CVSTrac conventions.

I'm attaching my latest diff against current HEAD. Amongst svntrac specific changes, there are some that are related to SQL injection vulnerabilities in cvstrac and have nothing to do with svntrac implementation. Maybe those should be backported to main trunk and 1.1.6 released? After being tested properly, of course.
I'm saying this since there might be some places (in wiki.c for example) where if you try to play with single quotes, or alikes, links that are generated on that page won't work.

Should I open new ticket for this SQL injection stuff?

Hopefully if 1.1.6 is released I'll get my "Append remarks" link below remarks, hah? : )
While we're at it, is this site running stock cvstrack 1.1.5 or some customized version?

Now back to svntrac. AFAICT svntrac is ready for some public testing. Like I said, while developing I was able to test it only on windows under cygwin, in standalone server mode and with relatively small Subversion repository. Any volunteers for testing then? We're looking for any Linux/Unix box that has svnlook binary installed. If you do test it, please report your results here.
I'd be very surprised to hear that svntrac caused some lose of data, but this is alpha code so take precautions and do your backup!

Build procedure is same as for cvstrac except for one thing: call make like this: 

  make install APPNAME='svntrac'

or 

  make install APPNAME='svntrac.exe'

for windows

2005-Aug-11 01:25:24 by cpb: 

  Should I open new ticket for this SQL injection stuff?

Most of them look like they were introduced with changes in this branch, so they can stay here.

The one in attach.c isn't exploitable since zPage is filtered through is_wiki_page(), the one in rss.c isn't exploitable since zCkinPrefix is filtered through sqlite_mprintf("%q"), and in wiki.c, pg is being filtered by is_wiki_page() while the ipaddr stuff requires admin privs.

However, the one in timeline.c is a problem. So yes, I'd open a ticket for that. 

  is this site running stock cvstrack 1.1.5 or some
  customized version?

Stock, I believe. The only public site that tracks HEAD that I know of is http://cvstrac.pfsense.org/.

2005-Aug-11 07:33:06 by anonymous:
ty

2005-Aug-11 12:42:56 by anonymous: 

  The one in attach.c isn't exploitable since zPage is filtered through is_wiki_page(), 
  the one in rss.c isn't exploitable since zCkinPrefix is filtered through sqlite_mprintf("%q"), 
  and in wiki.c, pg is being filtered by is_wiki_page() while the ipaddr stuff requires admin privs.

You're right, of course. I didn't really track back all those variables, I just searched for %s in all db queries and escaped them if they were comming from user input. Anyway, that's plumbed now, so... 

  Stock, I believe.

OK, thanks. I was wondering why there was no option to enter description for attachments and now I looked at timeline to see when was that added to code. It was just then that I realized how long ago 1.1.5 was released : )

2005-Aug-13 01:45:02 by anonymous:
RFC: branching support in svntrac

Short description of problem taken from Subversion book:

In other words, in Subversion there is no sane way of detecting branches.

Recommended way to handle this is to stick to some convention. For example: 

  /path/to/svn_repos/project1/trunk
  /path/to/svn_repos/project1/branches

In this scenario, your main line of development is in trunk subdirectory of your project's root dir. When you need to create a branch you svn copy your project from trunk to subdir of branches, for example branches/my_first_brach. Please visit last link from above for more info.

Only way I see to introduce branch support in svntrac would be to let user define where his/her trunk and branches are. Based on above example, it would look something like this: 

  SVN repository: /path/to/svn_repos/project1
  trunk dir:      trunk
  branches dir:   branches

But this introduces all sorts of problems and ATM I don't really see how those problems are worth solving. Performance would be impacted severely in some cases since everything that queries repository root (diff for most part) would need to extract trunk and branches apart.
It could be done but it would be very fragile since it completely depends on user conventions and assumes that users know exactly what they're doing. In a nutshell, it all comes down to this: 

  ...the resulting directory is only a "branch" because you attach that meaning to it.

So the question is do we support branches in svntrac at all or not? If yes, I'd like it to be done before we release svntrac to public.

I'd really like to hear opinions of some Subversion users about this. How are you managing your branches in Subversion? Do you see branch support in svntrac as natural thing to do?

Now something a bit unrelated to this. svntrac is going to need it's own /setup_cvs, or shall I say /setup_svn, and /setup_user page. Especially if branch support is going to be implemented. /setup_user would need to be changed to allow user to enter path to users file since this can be anywhere in case of svntrac .
How do we go about separating cvs and svn stuff in this case? Just do it in places where it's needed or...?

One other thing. "cvstrac" is seen in lots of places throughout cvstrac. Do we keep it like that or do we make it more SCM aware?

2005-Aug-13 02:50:54 by cpb:
There's no particular reason why CVSTrac needs to know that something is a branch. All you get out of it is an extra bit of text for checkins and maybe a different color for a message. If the branch is obvious from the filename, it may not be worth worrying about telling CVSTrac. Or you can just look and see how Trac handles branches; near as I can see, it doesn't explicitly know about them. 

  Now something a bit unrelated to this. svntrac is going to
  need it's own /setup_cvs, or shall I say /setup_svn,

/setup_repository? 

  How do we go about separating cvs and svn stuff in this case?
  Just do it in places where it's needed or...?

It depends on the page, but it would be preferrable not to build a bunch of SCM-specific pages unless the behaviour is completely different for a particular function. 

  One other thing. "cvstrac" is seen in lots of places
  throughout cvstrac. Do we keep it like that or do we make it
  more SCM aware?

It certainly wouldn't be a bad idea to change references to CVS the SCM, but completely renaming the app isn't as easy. You could certainly replace every instance of: 

  @ blah blah blah CVSTrac blah blah

with, say, 

  @ blah blah blah %s(g.zAppName) blah blah

But it doesn't work so well for wikiinit.c (autogenerated), and you've also got things like links to cvstrac.org. Then there's just the confusion you're going to get when you've got <n> similar names for the exact same binary... (i.e. "I heard that SVNTrac is a fork of CVSTrac?!?"). And SVNTrac was the original name for Trac.

I'd suggest just sticking with the existing name, changing any overly confusing mentions of CVS, and adjusting the /about text to describe the SCM. Oh, and maybe delete some of the CVS specific Wiki pages that come out of wikiinit.c during the database init.

2005-Aug-13 12:41:15 by anonymous: 

  There's no particular reason why CVSTrac needs to know that something is a branch. 
  All you get out of it is an extra bit of text for checkins and maybe a different color for a message.

Exactly. That's why I said it's not worth the effort.
I looked at Trac and they do the same as svntrac does now, nothing. It's just another directory. We'll leave it like that. 

  /setup_repository?
Agreed.


  It depends on the page, but it would be preferrable not to build a 
  bunch of SCM-specific pages unless the behaviour is completely 
  different for a particular function.

Yes, that's what I thought. Since we're not supporting braches, /setup_repository wold be very simillar. No need for separate page here, just a few if's. I'd do the same for /setup_users . I'm not sure I'll even need users to enter path to users file since I can read location of it from some other config files. I'll have to see if this is viable option. 

  I'd suggest just sticking with the existing name, changing any overly 
  confusing mentions of CVS, and adjusting the /about text to describe the SCM. 
  Oh, and maybe delete some of the CVS specific Wiki pages that come out 
  of wikiinit.c during the database init.

In general, I agree.

2005-Aug-13 14:26:30 by cpb: 

  All you get out of it is an extra bit of text for
  checkins and maybe a different color

That could change down the line if we decided to add some sort of tree view to /rlog, but I think we'll never be able to fully support every new feature for every SCM. If we can add new features without completely breaking a particular SCM, I'll be happy enough.

2005-Aug-14 15:15:42 by cpb:
Just out of curiosity, I sat down and ran my (personal) CVS repository through cvs2svn and started up svntrac. In a nutshell: 

  cvs2svn -s /home/cpb/.svntrac /home/cpb/.cvsroot
  svntrac init /home/cpb/.svntrac all
  svntrac server 8009 /home/cpb/.svntrac all

Some comments during a side-by-side comparison:

I'd like to move the discussion of svntrac branches to #445. This ticket is already getting long enough (especially without #378) and, quite frankly, branch handling is a bit more specialized than the basic CVSTrac/Subversion support which, these bugs aside, seems solid.

2005-Aug-14 16:04:06 by anonymous: 

  It Just Works. Not that I didn't expect it, but I'm still impressed at how seamlessly that fell together.

I'm a bit surprised too. I didn't expect it to go so smoothly : D 

  That first history_update()... I strongly suggest making svntrac update <svnroot> <project> 
  the next step after the init. Otherwise that first setup login times out. cvstrac would benefit similarly, I think.

Sounds good to me. 

  ...an admin could just run svntrac update in a post-commit hook if performance becomes a concern.

Yes, that could be very interesting in some cases. 

  Not sure if this is a side-effect of cvs2svn or not,...

I don't have such problems, so for now I'd blame that one on cvs2svn . 

  The tags directory under browse is... well, empty,...

I don't think its a bug in svntrac. I guess cvs2svn is trying to be very helpfull so it trys to copy all your tags from CVS in /trunk dir of Subversion repository, as reccomemded by Subversion book. If it's empty, it probably means you didn't have any tags in your CVS repos, or cvs2svn is not handling them properly.

I'll comment on #445 later, I don't have time for it right now.

btw. I'm working on user import stuff right now and it's progressing pretty good. I'll post a diff when I'm done with it.

2005-Aug-14 18:34:01 by cpb: 

  If it's empty, it probably means you didn't have any tags
  in your CVS repos, or cvs2svn is not handling them properly.

Ah, right. I wasn't clear on that. There is stuff under the /tags directory. There's even module directories under them. There's just nothing in these directories. No files. /tags is just a big empty directory tree. No, wait, it's not.

Actually, I think it's just outright broken.

Neither of the svntrac /branches or /tags are even close to what's in a checked out working copy. Looking at the FILECHNG table directly, it seems that the branches and tags are completely wrong. For example, in a checked out Subversion tree, I've got this tag on a single file: 

  A  work/tags/testing123
  A  work/tags/testing123/palmtactics
  A  work/tags/testing123/palmtactics/ptmath.pl

Which matches what's in my CVS repository exactly.

But in svntrac FILECHNG, I've got: 

  130|tags/testing123/|130||1||
  130|tags/testing123/CVSROOT/|130||2||
  130|tags/testing123/ColdSync/|130||2||
  130|tags/testing123/conduits/|130||2||
  130|tags/testing123/diabetes/|130||2||
  130|tags/testing123/palmtactics/Makefile|130||2||
  130|tags/testing123/palmtactics/gdbmgame.c|130||2||
  130|tags/testing123/palmtactics/gobject.c|130||2||
  130|tags/testing123/palmtactics/gpalmtactics|130||2||
  130|tags/testing123/palmtactics/gpalmtactics.c|130||2||
  130|tags/testing123/palmtactics/palmtactics.c|130||2||
  130|tags/testing123/palmtactics/palmtactics.h|130||2||
  130|tags/testing123/palmtactics/palmtactics_defs.h|130||2||
  130|tags/testing123/sugardaddy/|130||2||

Which is just not right. Even where it's getting the directory, it's missing the one file with that tag. Branches look to be in the same boat.

2005-Aug-14 19:09:43 by cpb:
Found the empty filename thing in the browse, too. It seems that svntrac is putting directories into the FILECHNG table. i.e. 

  1|tags/|1||1||
  1|trunk/|1||1||
  2|trunk/CVSROOT/|2||1||

etc. FILECHNG shouldn't contain directories. Or, at least, the browse interface doesn't expect it to contain directories.

2005-Aug-14 21:27:06 by cpb: 

  Neither of the svntrac /branches or /tags are even close
  to what's in a checked out working copy.

Very strange. It appears that the problem is with svnlook. svntrac is working with what it's been given, but for some reason it's getting junk.

2005-Aug-15 01:09:53 by cpb:
Okay, I've got my head around it. It's partly a problem in how cvs2svn works, but it also indicates that Subversion's tag and branch handling is just nuts. :(

Near as I can tell, what's happening is that the way cvs2svn does tags is to basically: 

  svn copy /repository/trunc /repository/tags/<tagname>

then delete everything that doesn't have the original CVS tag, then commit the whole batch. This results in everything except the tagged files being flagged as being removed, but the files that got the CVS tag aren't even mentioned in the svnlook changed report for that particular tag because, well, they didn't change.

Which is a problem. Because if I just go and do: 

  svn copy file:///home/cpb/.svnroot/trunk
    file:///home/cpb/.svnroot/tags/test_svn_tag
    -m "testing Subversion tagging"

There's nothing under the test_svn_tag directory in the browse view because, as far as Subversion is concerned, nothing has changed. Let's take this over the #445...

2005-Aug-25 01:02:24 by anonymous:
Ouch. I really ought to sign my rants (I'm the one who originally pointed out that Trac was crufty to install - but after blogging about it and coming here, my UMTS connection dropped, and only remembered to come back today as I browsed Michael Tsai's blog.

Just wanted to say thanks for the amazing progress you guys've been doing, and apologise for not leaving a proper contact. :)

Rui Carmo

2005-Aug-25 16:48:54 by cpb: 

  and apologise for not leaving a proper contact. :)

No big problem. It was a prod in a good direction even coming from "anonymous".

On the pure CVS side it's certainly been a Good Thing. Besides the immediately useful features and fixes (#435, #437, #444, among others), the SCM-independent code reorg makes life easier and actually makes CVSTrac faster. 

  Rui Carmo

Ah, Mr #231. Just the person...

c.

2005-Aug-25 20:34:29 by anonymous:
Yep, #231 was mine, submitted after I sent in the RSS feed patch (which I never got around to expanding to include actual RSS item content). I've added a note there concerning the report formatting issue - doing a clean CSS layout for something like CVStrac would be easier if the navigation, code display, footers, etc. and other elements were wrapped in DIV tags...

(I have an old CSS I used to customize CVStrac for my site somewhere, will try to add it to #231)

2005-Sep-09 13:52:45 by chorlya:
My latest diff is attached. SvnTrac should now be able to properly handle svn copy and svn delete on directories.
Small bug that prevented Reconstruct and Rescan to be run on SvnTrac has also been fixed. There are some minor code and comment changes to svn.c .

Performance is still pretty bad, but usable. Still lots of work to be done regarding performance.

2005-Sep-10 00:30:57 by cpb: 

  My latest diff is attached.

Checked in. Took a bit of work... apparently log message lengths are a bit more variable than we'd hope. Also cleaned up the memory handling in the copy and delete tree functions.

However, it now seems to correctly handle my cvs2svn repository.

2005-Sep-10 00:44:16 by chorlya: 

  Checked in. Took a bit of work...

Yeah, I see. Sorry about that. 

  apparently log message lengths are a bit more variable than we'd hope.

Not sure I get what you mean by that. Is that good or bad? : )

2005-Sep-10 01:29:48 by cpb: 

  Not sure I get what you mean by that. Is that good or bad? : )

I call core dumps bad. Seems like an off-by-one in the malloc, just an extra newline or something.

2005-Sep-10 09:23:42 by chorlya: 

  I call core dumps bad. Seems like an off-by-one in the malloc, just an extra newline or something.

Yeah, that is bad. Didn't get those here though. But it makes perfect senese to have a few extra bytes anyway.

[Append remarks]

Properties:

Type: todo           Version:  
Status: defer          Created: 2003-Dec-30 16:32
Severity: 4           Last Change: 2005-Aug-02 18:29
Priority: 4           Subsystem: cvstrac 
Assigned To: drh           Derived From:  
Creator: anonymous 

Derived Tickets:

#435   document CVSTrac coding conventions
#439   No mention of SVN on the Wiki
#444   Search should be able to search on repository filenames
#445   svntrac's branch handling
#446   insert_file() doesn't handle paths without file part properly
#448   import of svnserve users
#459   Fix documentation/UI to make more sense with SVN

Related Check-ins:

2006-Apr-24 01:04   Check-in [751]: (#278) try to abstract the ScmTrac stuff a bit more. Basically, we create a structure under struct Global which contains SCM "capabilities", callbacks, etc. The inidividual SCM's export a single init_<scm> function to fill the structure. Then everything interacts solely with that structure rather [...] (By cpb)
2005-Sep-14 01:07   Check-in [505]: (#278 and #170) replace a bunch of code with a couple common functions + output_pipe_as_html() takes an input stream and a "force" flag and outputs it as HTML. If force is used or the input is plain text, it just gets wrapped with <pre> tags. If it's detected to be HTML, it'll be passed through unchanged. [...] (By cpb)
2005-Sep-14 00:38   Check-in [504]: (#278 #170) filter/diff tweaks + add the %RP substitution that things like svn and git require + unify how all three SCM's take parameters + allow either HTML or plain text output from the file filter to make the diff behaviour. (By cpb)
2005-Sep-10 01:40   Check-in [487]: (#278 and #476) both GitTrac and SvnTrac shared a copy of is_file_available(). Merge them into /cvstrac/history.c and just have CVS with it's own implementation. (By cpb)
2005-Sep-10 01:06   Check-in [485]: (#278 #476 #357) replace the extra patchset_chng() method with just a flag for diff_chng(). This cuts down on duplication and is one less SCM entry point. Also, don't display Patchset or Inspection if the user doesn't have the permissions to use them. (By cpb)
2005-Sep-10 00:22   Check-in [484]: (#278) merge chorlya's 20050909 diff, with some fixes + log messages now variable length. This works nicely for Subversion because it actually indicates length before the message + tree copies and deletes now handled sanely + fixed some memory management issues in the patch + skip svn_did_repository_change() [...] (By cpb)
2005-Sep-10 00:18   Check-in [483]: (#278 and #476) only CVS knows about modules. That might change later. (By cpb)
2005-Sep-04 02:35   Check-in [466]: (#474) can't use CHNG.prevvers directly in CVS because we don't actually build the entire history until previous_version() is called. (By cpb)
2005-Sep-04 02:23   Check-in [465]: (#473) document available *.mk files. (#278) describe how to build Subversion support (By cpb)
2005-Aug-31 00:49   Check-in [460]: (#278) make the added/removed messages relative to version numbers so they make a little more sense. (By cpb)
2005-Aug-31 00:30   Check-in [458]: (#278) use FILECHNG.chngtype to generate labels + we were treating any 1.1 version as a new file. This obviously only works for CVS + flag added and removed files appropriately and always mention the file version. (By cpb)
2005-Aug-28 01:31   Check-in [444]: (#278) merge svntrac-patches into HEAD. This change includes: + Subversion support via svn.c + upgrades the database schema to 1.8 + an assortment of little fixes and tweaks to make all of the above actually work (By cpb)
2005-Aug-28 00:54   Check-in [443] on branch svntrac-patches: (#278) "svnlook" assumes that $CWD is the repository. Which means that because CVSTrac first does a chdir() into the repository directory (as given by the command line), the first login by "setup" will cause a repository import. It's usually a good idea if the admin has a chance to set things up first... (By cpb)
2005-Aug-27 14:49   Check-in [441] on branch svntrac-patches: (#278) remove a free() call on a NULL pointer. (By cpb)
2005-Aug-14 00:14   Check-in [430] on branch svntrac-patches: (#278) last cut at the schema 1.8 upgrade thing. There doesn't appear to be a reasonable way to get performance on 50000 filechng records to be acceptable, so just do the 1.8 upgrade and then do the prevvers and chngtype determination on demand, when previous_version() or is_dead_revision() [...] (By cpb)
2005-Aug-12 16:04   Check-in [427] on branch svntrac-patches: (#278) fix the ordering, change the schema init to make testing easier. 50000 filechng updates still takes a while, but it's a lot faster and it appears to be complete. (By cpb)
2005-Aug-12 16:03   Check-in [426] on branch svntrac-patches: (#278) move the schema stuff into a separate check_schema() function and have the "update" server command call it. This allows for somewhat less painful updates. (By cpb)
2005-Aug-12 16:01   Check-in [425] on branch svntrac-patches: (#278) sanely handle "broken" version numbers (without periods). (By cpb)
2005-Aug-12 01:02   Check-in [424] on branch svntrac-patches: (#278) try to get some better performance from the schema v1.8 update algorithm. 15-20 minutes to churn through 48000 filechng entries seems a bit too much... (By cpb)
2005-Aug-11 02:07   Check-in [423] on branch svntrac-patches: (#278) clean up a bunch of database queries. Use db_short_query() in more places and actually free some results and stuff. This appears to speed up the 1.8 schema update considerably. (By cpb)
2005-Aug-11 01:47   Check-in [422] on branch svntrac-patches: (#278) don't need a svntrac target. Just go make all APPNAME='svntrac' (By cpb)
2005-Aug-11 01:43   Check-in [421] on branch svntrac-patches: (#278) Subversion update (By cpb)
2005-Aug-11 01:37   Check-in [420] on branch svntrac-patches: (#278) quote version in previous_history() query. (By cpb)
2005-Aug-11 01:36   Check-in [419] on branch svntrac-patches: (#278) quote revision in query (By cpb)
2005-Aug-11 01:36   Check-in [418] on branch svntrac-patches: (#278) don't do prevvers chaining for anything other than CVS. Also quote revision. (By cpb)
2005-Aug-11 01:00   Check-in [415] on branch svntrac-patches: (#278) comments to clarify what happens when cvs_previous_version() fails to work because <cough> someone messed with CVS revision numbers. In a nutshell, it breaks diffing but is otherwise currently benign. That may change if we make heavier use of the prevvers chaining and the chngtype "add" [...] (By cpb)
2005-Aug-11 00:58   Check-in [414] on branch svntrac-patches: (#278) is_dead_revision() only needs the database so it doesn't need to be buried in cvs.c. (By cpb)
2005-Aug-10 11:31   Check-in [413] on branch svntrac-patches: (#278) note to self. This works as long as people don't mess with CVS revision numbers, but woe to those like myself... (By cpb)
2005-Aug-10 03:27   Check-in [412] on branch svntrac-patches: (#278) need to set chngtype and prevvers during cvs_update_history() too. This isn't as nice as I'd like, but it seems to work okay. (By cpb)
2005-Aug-10 03:03   Check-in [411] on branch svntrac-patches: (#278) with prevvers in the filechng table, we don't actually need SCM-specific versions of previous_version(). (By cpb)
2005-Aug-10 02:40   Check-in [410] on branch svntrac-patches: (#278) is_dead_revision() is much, much less horrible when you don't need to run external programs... (By cpb)
2005-Aug-10 02:35   Check-in [409] on branch svntrac-patches: (#278) changes to the filechng table. WARNING this is hairy stuff. + This updates the schema to 1.8, adding the prevvers and chngtype fields to the filechng table. [...] (By cpb)
2005-Aug-09 23:04   Check-in [408] on branch svntrac-patches: (#278) add core Subversion support + s/diff_chng/patchset_chng/ + diff_chng() now defined to dump a HTML-ized diff + svn.c now fleshed out with something which supposedly works ;) (By cpb)
2005-Aug-09 22:42   Check-in [407] on branch svntrac-patches: (#278) Make sure we always have '/' in zFile, otherwise link to parent directory won't work for file in repository root. (By cpb)
2005-Aug-08 23:31   Check-in [405] on branch svntrac-patches: (#278, #357) add a chng_diff() function and enable the /patchset page. I'm using the term Patchset because Diff in CvsTrac always seems to imply an HTML page, not a raw diff. That might not be the best choice of words... [...] (By cpb)
2005-Aug-08 17:17   Check-in [404] on branch svntrac-patches: (#278) oops... that built at home. Weird. (By cpb)
2005-Aug-08 02:18   Check-in [403] on branch svntrac-patches: (#278) svn needs a filename for the previous version and, quite frankly, cvs should be using one as well because right now, there's a major problem if someone goes manually mucking with version numbers (via "cvs commit -r <n>", for example) since cvs_previous_version() doesn't even check the [...] (By cpb)
2005-Aug-08 01:39   Check-in [401] on branch svntrac-patches: (#278) add skeleton for Subversion support. Currently doesn't do anything, of course. Update makefile so it should just be a matter of make svntrac (By cpb)
2005-Aug-08 01:28   Check-in [400] on branch svntrac-patches: (#278) add support for dynamic SCM detection from the executable name. In a nutshell, we look for <blah>trac in argv[0] and set the zSCM variable in the Global structure to <blah>. Modified a few places to actually check the g.zSCM value before blindly doing something CVS-like. (By cpb)
2005-Aug-08 00:59   Check-in [399] on branch svntrac-patches: (#278) move all the CVS-specific code into cvs.c except the user import stuff. Note that this is all in the svntrac-patches branch. [...] (By cpb)
2005-Aug-07 23:39   Check-in [398]: (#278) s/history_update/cvs_history_update/ (By cpb)
2005-Aug-07 23:38   Check-in [397]: (#278) update main.mk and makemake.tcl for cvs.c (By cpb)
2005-Aug-07 23:36   Check-in [396]: (#278) keep history.c, actually. That's where the "SCM dispatch" logic can go. (By cpb)
2005-Aug-07 23:23   Check-in [395]: (#278) rename history.c to cvs.c. We'll push more of the CVS-specific stuff into there. (By cpb)

Attachments: