214700 - move string into mozilla/xpcom

Status: RESOLVED FIXED

(Core :: XPCOM, defect)

Description

[cls]

The string & libreg libraries are only used by xpcom.  For a standalone xpcom
autoconf-based build system that reuses the existing build infrastructure to
work (see reorg prototype postmortem), these directories will need to live under
the xpcom heirarchy.  To save cvs history, they will need to be rsync-copied on
the cvs server.

Comment 1

[Doug Turner (:dougt)]

strings sure... lets talk to jag.


libreg shouldn't be moved into xpcom.  we should first kill xpcom/obsolete.  it
is the only thing that requires libreg.  in fact, core Gecko doesn't require libreg.

Comment 2

[cls]

lxr shows that there are a handful of places outside of mailnews which still
have dependencies upon xpcom_obsolete.  I believe bsmedberg or someone has
started converting the mailnews uses as well.  We could convert the few
non-default gecko uses and just move xpcom/obsolete into mailnews as suggested
in bug 113711.

Comment 3

[Doug Turner (:dougt)]

xpinstall was/is a heavy users of the obsolete lib.

Comment 4

[jag (Peter Annema)]

I'm fine with moving strings into xpcom... Would that be xpcom/string or
xpcom/ds/string?

Comment 5

[Doug Turner (:dougt)]

it is key to our xpcom implementation and should have its own top level directory.

Comment 6

[cls]

I added a patch to bug 214889 to remove xpinstall's dependency upon
xpcom_obsolete.  However, xpinstall still has a ton of references to libreg. 
What's the non-deprecated replacement for the nsIRegistry functionality?  If we
can phase out nsIRegistry, then we could just move libreg into xpinstall.

Comment 7

[Doug Turner (:dougt)]

There is no general purpose non-deprecated replacement for the nsIRegistry
functionality.  

Comment 8

[cls]

Umm.  In that case, why is nsIRegistry in xpcom_obsolete?  It's still used by
most of the test apps (mfcembed, qatestembed, powerplant), libprofile and even
some of the plugin code.  If we cannot get rid of nsIRegistry, then we're stuck
with libreg as well.

Comment 9

[Doug Turner (:dougt)]

who says?  client just need to use something else.  

Comment 10

[cls]

Well based upon comment #7, "something else" doesn't exist atm.  So, in that
case, the clients (and profile/plugin code), need to individually rewrite that
functionality from scratch.  I guess I don't see the point of that since we
already have something that works.  Is the only problem with nsIRegistry that it
uses the deprecated nsFileSpec APIs?

Comment 11

[Doug Turner (:dougt)]

libreg and its nsIRegistry wrapper is not a good data structure for most of the
users.  The only client, that probabaly should remain using it, is xpinstall,
for which dveditz invented it for.

Comment 12

[cls]

Ugh. All of this "cleanup" is really beginning to look orthogonal to the
original goal of making xpcom a standalone project.  Can we just move
xpcom/obsolete to mozilla/xpcom_obsolete and be done with it?  

Comment 13

[Doug Turner (:dougt)]

fine by me.  I do not want to support anything in that lib.

Comment 14

[cls]

Attachment: reorg xpcom v1.0

Patch to be applied after directories are copied & renamed on the server.

Comment 15

[Doug Turner (:dougt)]

Comment on attachment 129179 [details] [diff] [review]
reorg xpcom v1.0

looks fine to me.  great work.

Comment 16

[Daniel (Leaf) Nunes]

what, if any, directories need copying? I'm about to be out of the country, and
i'm not sure who else is around to do directory/file copies, so be quick, but be
specific, and be sure.

Comment 17

[cls]

cd /cvsroot/mozilla
rsync -auv string xpcom/
rsync -auv xpcom/obsolete .
mv obsolete xpcom_obsolete

Comment 18

[cls]

Oh, how annoying.  I just realized that mozilla/xpcom_obsolete will not be
pulled by the default SeaMonkey tag.  Now, I can modify client.mk to add it to
the SeaMonkey pull definition, but that will still break anyone overriding
MOZ_CO_MODULE.

Comment 19

[cls]

Attachment: Pull xpcom_obsolete via client.mk

Comment 20

[Daniel (Leaf) Nunes]

Comment on attachment 129187 [details] [diff] [review]
Pull xpcom_obsolete via client.mk

r=leaf

Comment 21

[Daniel (Leaf) Nunes]

Was obsolete ever built for any previous release? If so, no moving (i'd have to
rsync/cp it).

Comment 22

[cls]

No, no.  Afaik, there was never a mozilla/obsolete.  We need to rsync/cp
obsolete from mozilla/xpcom to mozilla/ and then rename it to xpcom_obsolete.

Comment 23

[Alec Flett]

I think the point is that xpinstall is NOT the only user of libreg - libprofile
is also a user. in weaning people off xpcom_obsolete, you at least have to give
people an exit path.

obviously we want to support as little code as possible (As a general rule) but
until someone switches code like libprofile over to some new system (which would
break backwards compatibility with previous installs - i.e. without libreg, we
can't find the profile directory)

Maybe the switch to firebird/thunderbird is a good time to write a new
libprofile that uses the windows registry and/or other platform-specific
registries in order to store the list of profile directories. We'd just need
firebird/thunderbird to somehow be able to magically locate profile directories
when a profile is migrated from seamonkey to firebird/thunderbird. I'm not sure
how we'd find profile directories though, other than just doing a depth-first
search in the default root profile directory.

Comment 24

[Doug Turner (:dougt)]

>you at least have to give people an exit path.

I have: roll your own.  the registry is an over kill for existing clients.  I
spoke to most of the stakeholders and all agree that migrating to something
else, whatever it is, probably is a good idea.

Comment 25

[Daniel (Leaf) Nunes]

Comment on attachment 129179 [details] [diff] [review]
reorg xpcom v1.0

sr=leaf, copies done.

Comment 26

[Brendan Eich [:brendan]]

New top-level directories should be run by staff, in particular, by me.  Who
will own mozilla/xpcom_obsolete?  Why should we have it lying around unowned at
the top level?

I really don't think this is the right thing.  Doug, alecf: are either of you on
the hook for replacing xpcom/obsolete?  I don't think we should spend a lot of
time reinventing this wheel if it doesn't buy us something big.  What does it
buy us, anyway?

/be

Comment 27

[Brendan Eich [:brendan]]

Doug: migration is extinction if the birds fly off to nowhere.  Right now there
is nowhere to fly.  Why rename the place they birds want to migrate from, if it
does not help anything, sets bad precedent, and clutters up the countryside?

/be

Comment 28

[Doug Turner (:dougt)]

that was very colorful. :-)  

I don't really care where the obsolete directory ends up.  It is stuff that
hasn't been supported for years.  

I think cls wanted to make the stuff in mozilla/xpcom not depend on anything
other than NSPR.  Since libreg is only required by the obsolete lib, he wanted
to move it out.

Comment 29

[Brendan Eich [:brendan]]

Doug: I understand the motivation; it's also clear you don't care where the crap
falls, but I do: it should not fall unowned on the top level of mozilla/, and it
should be owned soon, or I'm going to flush it (how's that for color? ;-).

Alecf, anyone else: who will own this "obsolete" (deprecated is the right word,
if it's still in use) stuff?

/be

Comment 30

[Doug Turner (:dougt)]

on a roll with the visuals.  :-)

i would be a great service if you would pull the handle.  People are working
updating their code to use other stuff.  It is very possible that this lib could
just be moved into mailnews and part of it into xpinstall.  This would be ideal.

Comment 31

[cls]

Brendan:  The deprecated stuff would remain unowned as it is now.  We originally
took the flushing path (via bug 38122).  However, the lack of a direct
replacement for nsIRegistry threw up a pretty sizable roadblock.

But that's really a secondary issue wrt this bug, IMO.  Regardless of whether
the code gets moved to the top-level, under modules/ or some new obsolete/
directory, it clearly doesn't belong in xpcom/.  xpcom_obsolete has its own
MODULE namespace,  it doesn't get built as part of the xpcom/ build and it's not
part of the xpcom library.  Where should the directory be moved to if not the
top-level?

Comment 32

[Brendan Eich [:brendan]]

cls: best to part it out, *now*, as dougt suggested in comment #30.  Failing
that, if there must be mozilla/deprecated euthanasia ward as was suggested by
several earlier comments.  But a plan with no owner for a new top-level dumping
ground is worse than a plan where dougt has to suffer with the dead hand of the
past for a bit longer.  Why was it so important to relieve xpcom of this
dependency *now*?

Again, I see work being done in the wrong order, in a way that will enshrine
lack of ownership by putting it where no one has to see it.  Not in my back yard....

/be

Comment 33

[cls]

xpcom has *no* dependency upon xpcom/obsolete.  Since the desire was to make
xpcom/ self-contained, short of the build directories, the original plan was to
move libreg into xpcom/obsolete.  However, since dougt has stated that
xpcom/obsolete is dead and *no one* should be using it, it doesn't make much
sense for a standalone xpcom package to contain those files and risk confusing
users of the standalone xpcom.  Hence, the slight change of focus to just move
xpcom/obsolete somewhere else.  

And just to make sure that I'm not missing something subtle after my trip, are
you saying that xpcom/obsolete is owned by dougt atm because of the fact that it
lives under xpcom/ despite his comments (#13 & #28) to the contrary?  If
xpcom/obsolete is already unowned, then why does it matter where it lives when
cvs history is being preserved?  How is moving it to another non-toplevel
subdirectory going to make it any "less visible", especially considering that
the new xpcom_obsolete directory would be directly referenced by the toplevel
Makefile as it is now?

Your back yard definitely needs to be mowed and I don't see the problem with
moving the rusty old swing set to a different side of the back yard so that we
can tend to this section of grass.  You're advocating waiting until some
undetermined person shows up at some unscheduled time with a pickup to take away
the old swing set and replace it with a new one...which would live in a
different portion of the back yard anyway.  What's the point of that delay?  You
just took a 2 man/5 min project and turned it into a 5 man/2 week project.  What
happened to taking the incremental approach and not attempting to do everything
in one fell swoop?

Comment 34

[Brendan Eich [:brendan]]

cls: do not put words in my mouth.  I didn't take on xpcom ownership, dougt did.
 To then say "not in my backyard" and move the rusty swingset to the middle of
the village commons is bad ownership.  It's not *my* rusty swingset.  You seem
to like holding me to account for others' acts of disownership.  You are barking
up the wrong tree.

I never said anything about "visibility" being the issue.  The issue is who owns
something that you are bothering to move to the wrong place (but that's a nit),
and bothering to keep building without any owner.  Let's fix the ownership
problem, then move things around into neat little piles, please.  I know you
personally can't fix the ownership problem, and I'm not asking you to fix it.

/be

Comment 35

[Brendan Eich [:brendan]]

"xpcom has *no* dependency upon xpcom/obsolete."

I know that -- why was it necessary to say it?

No dependency might mean move to top level, but not without an owner, and not
with an _obsolete name mangling.  Again, if it were obsolete, we could cvs
remove it.

Cc'ing some mail and xpinstall guys who might be able to help, per comment 30.

/be

Comment 36

[cls]

Brendan: Apparently, I misinterpretted your comment about "not in my back yard"
as in you didn't want the reorganization of "obsolete" code to occur in the tree
that you steward, not that you didn't want that code "dumped" in your area of
ownership (mozilla/).  However, my point still stands.  The code would be
"owned" in the same fashion as it is now regardless of where it lives in the
tree.  Whether we like it or not, for code that has no *explicit* owner, we
almost always fall back to cvsblame.  That old "you touched it, you own it" adage.  

> You seem to like holding me to account for others' acts of disownership.

No, what I'm challenging is the idea that by moving this code, we're going to
make it "less owned" than it is now (which is not at all).  There's nothing to
be gained by leaving the code where it is and by moving it, we get the slight
gain that I mentioned in comment #33.  Since the location of the code was just a
nitpick, what do you suggest as a better location?

> I never said anything about "visibility" being the issue.

Actually, what you said in comment #32 was:

> in a way that will enshrine lack of ownership by putting it where no one has
to see it

Which, to me, sounds like a visibility issue.  And is not the case.  By moving
xpcom/obsolete to xpcom_obsolete, we make it clear that this code is *not* part
of the xpcom module and never has been since that code was factored out.

> Let's fix the ownership problem, then move things around into neat little piles

I can understand the need for a logical grouping of the code but why would
ownership matter in where the code lived in the tree?  Did I miss the memo that
said that everything under xpcom/ must be owned by dougt & alecf and everything
under xpinstall/ must be owned by dveditz & ssu?  Also, the modules which
comprise gecko & mailnews are the only users of the code which would make the
code effectively a part of gecko and not part of xpcom.  So, there shouldn't be
a logistical problem with moving that directory anywhere else in the open gecko
space.

> "xpcom has *no* dependency upon xpcom/obsolete."
> I know that -- why was it necessary to say it?

That would be in reference to your comment #32:

> Why was it so important to relieve xpcom of this dependency *now*?

I'm merely making it crystal clear that there's no build/code dependencies of
any sort.  Afaiac, this has turned into a managerial issue of what happens to
deprecated code and who is responsible for taking the code from deprecated to
obsolete status.  An issue, I will add, which has nothing to do with the
original purpose of this bug and is only tangentially related.  The issues with
xpcom/obsolete are not new (see bug 38122 & bug 113711 for starters), are not
affected by the proposed fixes in this bug and are *not* on the critical path to
fix this bug.  So let's find a new place to rsync/cp that directory to so that
we can move on and let someone who cares worry about the managerial issue.

Comment 37

[cls]

Attachment: reorg xpcom v1.1

This patch just contains the string changes from the previous patch.

Comment 38

[cls]

Comment on attachment 129681 [details] [diff] [review]
reorg xpcom v1.1

Transferring review & super-review.  I'd like to get this change in soon before
our two copies of string/ become out of sync.

Comment 39

[cls]

Comment on attachment 129681 [details] [diff] [review]
reorg xpcom v1.1

Marking a= based upon IRC conv with mkaply.

> mkaply: so, regarding bug 214700, do drivers have issues with the string changes or are we just waiting for the other issues in that bug to resolve themselves before progressing?

<mkaply> cls: We were just trying to understand what the bug was asking for.
All you want to do is cause the other strings to be build so you can remove
mozilla/string, correct?

> mkaply: right. leaf already copied the source in the repo to xpcom/string. It's already been pulled by default.  I just want to hook up xpcom/string and
remove mozilla/string

<mkaply> cls: go for it.
<mkaply> a=mkapl
<mkaply> y
<mkaply> Everyone agreed that was where string belonged, correct?

> dougt & jag agreed. I don't recall any dissenting comments but I'll double check

Comment 40

[Axel Hecht]

modules.mk needs to know about this, too. Anything involving
BUILD_MODULES=xpcom
still pulls string and tries to build it. Including my baby, waaaah. ;-)

Comment 41

[Mike Pinkerton (not reading bugmail)]

yay, this broke camino.

Comment 42

[cls]

Moving on, moving on.