104600 - template/custom gets pruned on cvs update -dP

Status: RESOLVED FIXED

(Bugzilla :: User Interface, defect)

Description

[Matthew Tuck [:CodeMachine]]

The template/custom directory shouldn't be in CVS, it should be created
automatically by checksetup.pl.

Comment 1

[Dave Miller [:justdave]]

I think it should be in CVS, but each bottom-level directory should contain a
README file explaining what the directory is for.

If checksetup.pl creates it, cvs update -dP will still prune it.  The only way
to stop that is to actually have files in those directories.  (well, we could
add it to .cvsignore, but I still like the README idea better)

Comment 2

[Zach Lipton [:zach]]

I agree -> user interface and myk

Comment 3

[Matthew Tuck [:CodeMachine]]

I think asking people to remember to create the custom directories is a bit
much.  What if the top level directory had a README, and the bottom level
directories were created by checksetup.pl?

Otherwise we're just asking to have work done that will be forgotten (although I
suppose we could add a test), for little benefit that I can see.

Comment 4

[Dave Miller [:justdave]]

uhhh...  that's exactly what we're trying to do, Matty, is prevent people from
having to remember to create them.

If we have README files in the bottom-level folders, then cvs will create them
at checkout, and nobody has to worry about having to create them.

If checksetup.pl does it, and the user doesn't put anything in them, a properly
structured cvs update will remove them.

Comment 5

[Matthew Tuck [:CodeMachine]]

I was talking about trying to prevent requiring developers from creating them.

Comment 6

[Dave Miller [:justdave]]

ah, but it's too late for that.  They're already in cvs, they're just empty.

Comment 7

[Matthew Tuck [:CodeMachine]]

I was asking for them to be removed.  It's not like we're not going to be adding
more.

Comment 8

[Myk Melez [:myk] [@mykmelez]]

Unfortunately there is no such thing as removing directories in cvs.  Once they
are there, they are in the repository forever.

Comment 9

[Matthew Tuck [:CodeMachine]]

Well that makes sense in a way, but can we prevent them being pulled by new
users?  Is that what this is saying?  If so, I'm happy with it as is.

Comment 10

[Dave Miller [:justdave]]

Why do we want to prevent them from getting pulled by new users?  That's exactly
the opposite of what this bug is about.  We want to make sure they get them. 
Those directories should be pulled on a checkout, and they should have something
in them (probably just a readme file) so that they don't get nuked on a cvs update.

Comment 11

[Matthew Tuck [:CodeMachine]]

Well given I filed this bug, I figure I have first hand knowledge of what the
bug _was_ about anyway. =)

What I'm say is that getting checksetup.pl to create directories based on what
is in template/default is a more reliable and cleaner solution than having to
put them in CVS.

Comment 12

[David D. Kilzer (ddk)]

Easy fix:  Create *blank* .cvsignore files in each template/custom
directory.

cvs update -d template/custom
touch template/custom/.cvsignore
touch template/custom/attachment/.cvsignore
touch template/custom/attachstatus/.cvsignore
touch template/custom/entry/.cvsignore
touch template/custom/global/.cvsignore
touch template/custom/request/.cvsignore
find template/custom -name .cvsignore -exec cvs add {} \;
cvs commit template/custom

Comment 13

[Dave Miller [:justdave]]

We are currently trying to wrap up Bugzilla 2.16.  We are now close enough to
release time that anything that wasn't already ranked at P1 isn't going to make
the cut.  Thus this is being retargetted at 2.18.  If you strongly disagree with
this retargetting, please comment, however, be aware that we only have about 2
weeks left to review and test anything at this point, and we intend to devote
this time to the remaining bugs that were designated as release blockers.

Comment 14

[Andreas Franke (gone)]

This is too easy to fix to be pushed out to 2.18. Just before the release, for
all the directories that have files in them in template/default, cvs add the
corresponding directory in template custom, touch .cvsignore there, and cvs add
the .cvsignore file and commit it. Should take about 10 minutes. On the other
hand, this will make life easier for all of us.

Of course, if this fix is controversial, I understand pushing it out. But I
don't see the problem.

Comment 15

[Jacob Steenhagen]

We need to figure out what we're gonna do w/this, and soon :)

Are we going to have a complete directory structure that mirrors what's in
'default'?  If so, we should add a test that makes sure any directory that
exists in template/default also exists in template/custom (and also make sure
our tests are pruning :).

Comment 16

[Dave Miller [:justdave]]

*** Bug 126280 has been marked as a duplicate of this bug. ***

Comment 17

[Dave Miller [:justdave]]

OK, how about this:

We have endico or leaf go in and manually delete everything under and including
custom in the cvs repository...

drop a .cvsignore file in template itself, and put "custom" in it.

checksetup.pl, when run, creates custom to mirror default.

We probably need to coordinate this with the i18n reorg, since we're probably
going to mutilate that whole hierarchy anyway to make room for a directory level
with language codes...

Comment 18

[Myk Melez [:myk] [@mykmelez]]

Dave: your plan sounds good to me.

endico, leaf: can you do this?

Comment 19

[Myk Melez [:myk] [@mykmelez]]

Attachment: patch: adds custom to .cvsignore file

I manually deleted the "custom" directory from the repository.	This patch adds
it to the pre-existing .cvsignore file in the template directory.

Comment 20

[Dave Miller [:justdave]]

Comment on attachment 77571 [details] [diff] [review]
patch: adds custom to .cvsignore file

r= justdave x2

Comment 21

[Myk Melez [:myk] [@mykmelez]]

Checking in template/.cvsignore;
/cvsroot/mozilla/webtools/bugzilla/template/.cvsignore,v  <--  .cvsignore
new revision: 1.2; previous revision: 1.1
done

Partial fix checked in.  Dropping severity and pushing out to next milestone.

Comment 22

[Dave Miller [:justdave]]

should we move this back to 2.16 and close it, and make a new bug for the rest?
 This now shows up as "ready to check in" under 2.18 because it has a patch with
second-review on it.

Comment 23

[Dave Miller [:justdave]]

This is fixed to my satisfaction.