88454 - fast-update.pl: add automatic "last update time" & update whole tree

Status: RESOLVED WONTFIX

(Webtools Graveyard :: Tinderbox, defect)

Description

[kill this account]

fast-update.pl is great.

I'd like to propose we add code to detect the last fast-update.pl time to
reduce the amount of cvs pulling.

Comment 1

[kill this account]

Attachment: patch; add code to determine hours (rounded up) since last fast-update

Comment 2

[kill this account]

Alec asked to assign it to me.

Comment 3

[kill this account]

Attachment: the last patch was the whole file. This is just the cvs diff -u

Comment 4

[Chris McAfee]

r=mcafee

Comment 5

[kill this account]

Attachment: patch; New! and Improved! Now with comments!

Comment 6

[kill this account]

Attachment: patch; to update whole tree: add to client.mk, parse the "cvs co ..." command

Comment 7

[kill this account]

This version takes the "cvs co ..." commands in client.mk and converts them
to fast-update.pl commands.

I developed/debugged/tested this on a 2 week old copy of the tree.

Comment 8

[Alec Flett]

I like the effort that's going into this but I was really hoping that
fast-update.pl could eventually just handle all the standard CVS commands on its
own, instead of having an external script...so basically you'd just replace
"cvs" with "config/fast-update.pl" and run the commands as normal..

Comment 9

[kill this account]

I broke it into 2 script because I see it being used 2 ways:

1) update this dir

2) update the whole tree

The fast-update.pl does the 1st.

I do not know anyway that any script could do the 2nd without knowing what
client.mk does. Different parts of the tree come from different places. There
is no way that I know of for any script to find these places.
Also, the 2nd goal should require the minimal long term maintenance to
keep it working correctly. As such I made it follow the checkout target's 
components.

In my first pass I put it all in fast-update.pl but then it got very complicated
(relative to what #1 requires) so I split the "cvs co ..." parsing into a 
separate command and put in a target in client.mk to know what all the parts
are.

I do not see any advantage to doing a "pull the tree" checkout with
fast-update.pl so we will always need the cvs command. The cvs command will be
the one that always gets attention since it is required. The fast-update.pl
command will always track a tiny bit behind.

Also I doubt that fast-update.pl will ever be able to completely replace the
cvs command. This would presume that bonsai perfectly reported what has
happened with the tree. There will always be the possibility that there is
a small difference hence people will want to do the real command if only to
verify if there is / is not a problem.

Comment 10

[kill this account]

However, that is not to say someone else could not rework this.

That would be quite okay with me.

All I want is a faster way to update my tree.

Comment 11

[kill this account]

be sure to set the execute bit for cvsco-fast-update.pl

Comment 12

[Chris McAfee]

  chmod +x foo.pl

before adding foo.pl to cvs will set this permission
when the file is checked out, this can be changed by
changing the permissions on the repository file.
If this file needs to be changed let me know, it's easy to do.

Comment 13

[Chris McAfee]

re: my last comment, this is a new file!  ok.

Comment 14

[Chris McAfee]

r=mcafee

Comment 15

[Chris McAfee]

I tried the 7/1 patch, this looks like it's working ok.
We should check this in so we can get some coverage on it.

Comment 16

[Simon Fraser [no longer active]]

Looks ok from my limited knowledge of makefiles & perl on other platforms. sr=
sfraser

Comment 17

[kill this account]

checked in

Comment 18

[kill this account]

Attachment: patch; add a "real_fast-update" ala "real_checkout" to "clear" the previous log

Comment 19

[kill this account]

Now that the core work is in I currently see 2 areas for improvment:

1) clear (actually rename) the checkout log as the "checkout" target does.
   see attachment 45919 [details] [diff] [review]

2) I need to debug why bonsai does not report the addition of the
   mozilla/modules/plugin/samples and mozilla/modules/plugin/tools dirs

Comment 20

[kill this account]

Can I get an r= and sr= on attachment 45919 [details] [diff] [review]?

thanks

Comment 21

[kill this account]

a bit more info:

I see the mozilla/modules/plugin dirs not being handled when I do a fast-update
of a 30 day old tree (a tree pulled before these dirs were added). Everything
appears fine in more current trees.

Doing a normal checkout does bring these files in and appears to clear things up
but fast-upate should do this.

Comment 22

[Chris McAfee]

Looks ok to me, r=mcafee

Comment 23

[Simon Fraser [no longer active]]

my makefile fu isn't up to reviewing that, but rs=sfraser if you need it.

Comment 24

[kill this account]

attachment 45919 [details] [diff] [review] checked in

Comment 25

[kill this account]

reopening this bug to address:

> need to debug why bonsai does not report the addition of the
> mozilla/modules/plugin/samples and mozilla/modules/plugin/tools dirs

Comment 26

[kill this account]

in my tree I see that the mozilla/modules/plugin/samples/simple/CVS/Entries file
looks odd:

  /.cvsignore/1.1/Sat Dec  5 09:04:39 1998//
  /Makefile.in/1.17/Fri Aug 10 07:35:51 2001//
  ..
  /makefile.win/1.15/Fri Aug 17 04:51:57 2001//
  D

The last line should look like

  D/samples////

Comment 27

[kill this account]

Attachment: patch; add ability to handle cvs -D <date> to fast-update

Comment 28

[Chris McAfee]

r=mcafee for latest patch.

Comment 29

[Simon Fraser [no longer active]]

sr=sfraser

Comment 30

[kill this account]

I see that fast update also does not pass the branch tag to the cvs up command.

Comment 31

[kill this account]

After the tree opens and I can check in the last change I will add code to
test for the conditions where fast-update is not appropiate; eg:

  Bonsai does not work when files have sticky tags.

  The branch tag changes require all the files in all the subdirs 
  be retagged so the fast update strategy of limiting work to
  single dirs (ie: non-recursive) is not appropiate

  If the date tag is earlier then all the files in all the subdirs need to
  be retagged so fast-update is not appropiate. Note: if the date tag is
  later and fast-update is working correctly then fast-update will work 
  correctly since the untouched files will be the lastes even if their
  tag is not the latest.

  

Comment 32

[John Keiser (jkeiser)]

Attachment: patch; periodically (and the first time we run) do a full checkout when tbox is set up to do fast-update

Comment 33

[John Keiser (jkeiser)]

Comment on attachment 117289 [details] [diff] [review]
patch; periodically (and the first time we run) do a full checkout when tbox is set up to do fast-update

note about that patch: it's untested ATM, but I wanted to give a couple of
people a chance to look at it.	It basically takes functionality I *have*
tested in the tbox3 client and dumps it in tbox1.

Comment 34

[Brad Roberts]

Attachment: diffs against brad-fast tinderbox to add few fixes from bbaetz

Comment 35

[Bradley Baetz (:bbaetz)]

The use of Date::Parse should probably be a |require|

jkeiser's patch looks right, but I'm not really able to test ATM.

Comment 36

[Brad Roberts]

Comment on attachment 117305 [details] [diff] [review]
diffs against brad-fast tinderbox to add few fixes from bbaetz

been running on brad-fast for months.  bbaetz wrote 'em, so I'll be the
reviewer.  jkesiter, cls suggested you as the sr'er.

Comment 37

[John Keiser (jkeiser)]

Comment on attachment 117305 [details] [diff] [review]
diffs against brad-fast tinderbox to add few fixes from bbaetz

eek, I thought I responded to this ... I'm not an sr and I can't think who is
an sr in the realm of Perl and build changes.  The changes seem OK to me
though.

Comment 38

[Alec Flett]

Comment on attachment 117305 [details] [diff] [review]
diffs against brad-fast tinderbox to add few fixes from bbaetz

I'm the original author of fast-update.pl, and a sr, so I'll gladly sr= this.
the approach seems right.. next time please do a diff -u though - these diffs
are pretty hard to read...

one nit:
>   print "checking lat update time in $filename\n";

"last" not "lat"

Comment 39

[Bradley Baetz (:bbaetz)]

Comment on attachment 117289 [details] [diff] [review]
patch; periodically (and the first time we run) do a full checkout when tbox is set up to do fast-update

Is this still a valid patch, given the other one on this bug?

Comment 40

[John Keiser (jkeiser)]

Comment on attachment 117289 [details] [diff] [review]
patch; periodically (and the first time we run) do a full checkout when tbox is set up to do fast-update

bbaetz: the attachment still applies, still should work and is probably still
untested.

Comment 41

[John Keiser (jkeiser)]

FYI, I have a tinderbox client running and once the initial run checks out I'll
test this.

Comment 42

[John Keiser (jkeiser)]

bbaetz, brad: are we missing some files that haven't been checked in? 
fast-update isn't working on my pull (today's pull) because the fast-update.pl
script doesn't accept -D ...

Comment 43

[John Keiser (jkeiser)]

Attachment: patch v1.1; periodically (and the first time we run) do a full checkout when tbox is set up to do fast-update

Posted for transfer; seems to be working but I'll set it to run for a couple
days and make sure.

Comment 44

[Benjamin Smedberg]

Mozilla tinderboxes doesn't use fast-update, and we're switching to mercurial, so WONTFIX.