[MPlayer-cvslog] r19341 - in trunk/stream: Makefile asf_mmst_streaming.c asf_streaming.c network.c network.h pnm.c stream_ftp.c stream_netstream.c stream_rtsp.c stream_vstream.c tcp.c tcp.h

[Diego Biurrun]

On Sat, Aug 05, 2006 at 05:07:59PM +0200, Michael Niedermayer wrote:
> 
> On Sat, Aug 05, 2006 at 03:51:45PM +0200, Diego Biurrun wrote:
> > On Sat, Aug 05, 2006 at 03:29:51PM +0200, Michael Niedermayer wrote:
> > > 
> > > On Sat, Aug 05, 2006 at 02:51:52PM +0200, Reimar D?ffinger wrote:
> > > > Hello,
> > > > On Sat, Aug 05, 2006 at 01:44:53PM +0200, Benjamin Zores wrote:
> > > > > hum, i also did some changes using #defines for error codes instead of -1/2/3.
> > > > 
> > > > Great, and how is anyone supposed to review like that?
> > > > I'd be really thankful if more people didn't behave if this was their
> > > > very code with nobody else using, caring or maintaining it.
> > > > Does anyone actually still care about stuff written in patches.txt or
> > > 
> > > seems many do not ...
> > > this is a very seriuos problem, if code cannot be reviewed then the code
> > > will become more and more buggy and or developers will turn away as its
> > > too much work to review messy changes
> > > i dont even want to think what happens if someone would commit malicious 
> > > code hidden in a unreviewable change
> > > 
> > > with cvs we could reverse changes, and try again,
> > 
> > I have said so before, I will say it again now: CVS has never and will
> > most likely never support reversing changes.  Now everybody repeat this
> > after me please.
> 
> cvs does support reversing the n last changes, furthermore you can rebuild
> the RCS files to remove any change in the middle, it of course requires
> an account on the server and we never did this to remove an old change
> but its possible, svn supports that too it just requires everything to
> be done to the whole repository instead of a single individual file so its
> a much bigger mess

It sure is *possible*, but Subversion has a clean way of offering this
functionality by dumping and reloading the repository.

Suppose you wish to remove r12345, just do

  svnadmin dump --incremental -r 0:12344 REPO > dump_file1
  svnadmin dump --incremental -r 12346:HEAD REPO > dump_file2
  svnadmin create new_repo
  svnadmin load new_repo < dump_file1
  svnadmin load new_repo < dump_file2

And this works just as easily if the commit spans 5 directories and 20
files.

All of this requires access to the repository, but this is an advantage,
not everybody should be allowed to fiddle with the repo history just
like that.

> > 'cvs admin -o' functions in a way that resembles reversing commits in
> > corner cases, but nothing more. 
> 
> yes, but these corner cases are mostly what we need(ed)

Not always.  You seem to be forgetting the problems and flames that (the
misuse of) 'cvs admin -o' caused ...

> > It's a dangerous tool that can easily
> > wipe out the wrong parts or all of a file's history.
> 
> fully agree, the solution IMHO would be backups + restrict cvs admin to 
> root at mphq

Unfortunately CVS did not allow this out of the box.  Subversion is a
clear improvement in this regard.

> > > now if trojanized
> > > code got commited then reversal would remove it and a following clean
> > > change could be reviewd ... with svn it currently looks like we will
> > > flame and then go crying in a corner while leaving the change in svn
> > > this is not good at all and IMO overshadows any security advantage
> > > svn has over cvs ...
> > > 
> > > for ffmpeg iam starting to consider to ask the admins to disable
> > > svn accounts on the first such violation without warning, well i certainly
> > > dont like that but if we cant find a reasonable simple way to reverse 
> > > such change then such a meassure would at least reduce the number
> > > of such changes and thus make it more realistic to review them manually
> > 
> > IMO this is a policy problem, not a tool problem.  
> 
> strongly disagree, its like putting a bag with a millon dollars on the street
> in a big western city and then put a sign on top with "please dont take"
> and now wheres the problem? the text on the sign? or the fact that you
> forgot to put a policeman or 2 there?

Let me better explain what I was trying to say here: We're all
relatively new to Subversion and best practices are only starting to
appear.  An example is when and how to use 'svn cp'.  If you split a
file, the best solution appears to be to copy it to preserve the common
history and stuff.  This is probably not obvious to new developers.  In
any case, it's not (yet) explained in DOCS/tech/svn-howto.txt.

> > Some parts of it may need to get clarified or made more explicit,
> > though.  For example, svn copy is new, there are no clear guidelines on
> > when and how to use it.  We will just have to make those up now.
> 
> policy said not to copy or move files by remove and add but to ask arpi/root
> to do it IIRC
> surely it should mention svn cp/mv now but its not like it wasnt clear about
> what wasnt allowed

I think your memory needs a quick freshup:

  8. Renaming/moving files or contents of files:

    svn move <source> <destination>
    svn commit <source> <destination>

    Do not move or rename files before discussing it on the
    mplayer-dev-eng mailing list first!

    Don't do a lot of cut'n'paste from one file to another without a
    very good reason and discuss it on the mplayer-dev-eng mailing
    list first. It will make those changes hard to trace.

    Such actions are useless and treated as cosmetics in 99% of cases,
    so try to avoid them.

So maybe this could have been discussed a bit more, but it surely was
not forbidden.  IIRC Ben informed people of his plans on dev-eng, I'm
not terribly sure about this, though.  However you know that our devs
are lazy to react to such mails, but quick to flame after the fact :(

> please dont missunderstand me, iam not saying cvs is better then svn
> just that svn has problems which cvs did not have

OK, I was under the impression that you were passing through a stint of
CVS nostalgia lately.  Every tool has it's problems and drawbacks, but
with CVS we had come to rely on some bad hacks just because we were
lucky enough to have them not break for us.

Diego