Closed Bug 506601 Opened 15 years ago Closed 14 years ago

Transition LDAP SDKs to Mercurial (from cvs)

Categories

(MailNews Core :: Build Config, defect)

defect
Not set
minor

Tracking

(thunderbird3.1 .8-fixed)

VERIFIED FIXED
Thunderbird 3.3a2
Tracking Status
thunderbird3.1 --- .8-fixed

People

(Reporter: sgautherie, Assigned: standard8)

References

(Depends on 1 open bug)

Details

Attachments

(2 files, 2 obsolete files)

+++ This bug was initially created as a clone of Bug #437064 +++ Or at least a mirror (at http://hg.mozilla.org/cvs-trunk-mirror/file/default/ or elsewhere) Fwiw, from a SeaMonkey p-o-v, DOMi and Venkman are now in hg, CZ stayed in cvs only.
This is something I've been meaning to bring up for discussion for a while - on mozilla.dev.tech.ldap not here (bugzilla isn't a discussion forum, and this should have discussion first). I'll try and post something there later today.
The message thread is here: http://groups.google.com/group/mozilla.dev.tech.ldap/msg/07608796e5e37cc4 Please post any discussion to that thread.
I'm currently working through some of this with the LDAP guys. Should have more to report soon.
Assignee: nobody → bugzilla
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Summary: Create an own hg repository for directory (= ldap) → Create an own hg/Mercurial repository for directory (= ldap)
Attached patch Draft Patch (obsolete) (deleted) — Splinter Review
I've been discussing this offline with the ldap folks. There is a test repository here that has been set up which has imported the code and a relevant branch from cvs: http://hg.mozilla.org/users/bugzilla_standard8.plus.com/ldap-sdks/ The main location has been suggested as: http://hg.mozilla.org/ldap-sdks/ Note that this includes more that just the ldap c-sdk, hence the change in name. I'm waiting for a couple more days for final confirmation, but once I get that I'll file a bug to get the repository created for us. The patch attached does the work to get the ldap-sdks repo included in comm-central. I've also moved from using /directory/ to /ldap/ as it is a better description of what is actually in that folder. I've not removed the old cvs directory, just in case anyone has changes in it. The url in client.py is currently pointing to the test repository which, of course, will need updating when we get the new one set up.
Comment on attachment 491503 [details] [diff] [review] Draft Patch >-# "COMM", "MOZILLA", "CHATZILLA", "INSPECTOR" or "VENKMAN" >+# "COMM", "MOZILLA", "CHATZILLA", "INSPECTOR", "VENKMAN", "LDAPCSDK" nit: LDAPSDK or LDAPSDKS since the hg repo will have more than CSDK (same for following vars)
Attachment #491503 - Flags: feedback+
Blocks: 613846
Depends on: 613921
(In reply to comment #4) > The main location has been suggested as: > > http://hg.mozilla.org/ldap-sdks/ Following discussion on bug 613921 the revised suggestion is to use: http://hg.mozilla.org/projects/ldap-sdks/ as this probably doesn't need to be a top-level directory. If there's comments against this (or for a better location) please comment within the next 24 hours.
Attached patch The fix (deleted) — Splinter Review
Updated patch to point to the now created and filled-out repository. Also takes account of the latest changes to client.py. I'll take whoever reviews it first. Note that I'm planning on landing this in comm-central first for a couple of days, then porting to comm-1.9.2 and comm-1.9.1 after that. I'm aiming to get this into comm-1.9.* in time for the builds of the security releases.
Attachment #491503 - Attachment is obsolete: true
Attachment #492596 - Flags: review?(kairo)
Attachment #492596 - Flags: review?(bugspam.Callek)
Comment on attachment 492596 [details] [diff] [review] The fix >diff --git a/client.py b/client.py >-def do_cvs_checkout(modules, tag, cvsroot, cvs, checkoutdir): yay for killing cvs! :) >+def fixup_ldap_repo_options(options): Hmm, why exactly do we need fixup_ldap_repo_options here? It sounds superfluous to me... >diff --git a/configure.in b/configure.in >--- a/configure.in >+++ b/configure.in >@@ -7281,14 +7281,17 @@ MOZ_BUILD_APP="$MOZ_BUILD_APP_CACHED" > if test -n "$COMPILE_ENVIRONMENT" -a -n "$MOZ_LDAP_XPCOM"; then > # these subdirs may not yet have been created in the build tree. > # don't use the "-p" switch to mkdir, since not all platforms have it >- if test ! -d "directory/c-sdk/ldap"; then >- if test ! -d "directory/c-sdk"; then >- if test ! -d "directory"; then >- mkdir "directory" >+ if test ! -d "ldap/sdks/c-sdk/ldap"; then >+ if test ! -d "ldap/sdks/c-sdk"; then >+ if test ! -d "ldap/sdks"; then >+ if test ! -d "ldap"; then >+ mkdir "ldap" >+ fi >+ mkdir "ldap/sdks" > fi >- mkdir "directory/c-sdk" >+ mkdir "ldap/sdks/c-sdk" > fi >- mkdir "directory/c-sdk/ldap" >+ mkdir "ldap/sdks/c-sdk/ldap" > fi Why exactly do we need this still? It sounds like a lot of hackery to me...
Attachment #492596 - Flags: review?(kairo) → review+
(In reply to comment #8) > >+def fixup_ldap_repo_options(options): > > Hmm, why exactly do we need fixup_ldap_repo_options here? It sounds superfluous > to me... Well, it follows the style of the other functions in there. We could probably make sorting out all those options easier, but I can't see a way at a quick glance, and it'd be a separate bug anyway. > >diff --git a/configure.in b/configure.in > >+ if test ! -d "ldap/sdks/c-sdk/ldap"; then > >+ if test ! -d "ldap/sdks/c-sdk"; then > >+ if test ! -d "ldap/sdks"; then > >+ if test ! -d "ldap"; then > >+ mkdir "ldap" > >+ fi > >+ mkdir "ldap/sdks" > > fi > >- mkdir "directory/c-sdk" > >+ mkdir "ldap/sdks/c-sdk" > > fi > >- mkdir "directory/c-sdk/ldap" > >+ mkdir "ldap/sdks/c-sdk/ldap" > > fi > > > Why exactly do we need this still? It sounds like a lot of hackery to me... I'm pretty sure its for objdir builds - we need to ensure that the target directory is created for the configure to run properly.
Attachment #492596 - Flags: review?(bugspam.Callek)
Summary: Create an own hg/Mercurial repository for directory (= ldap) → Transition LDAP SDKs to Mercurial (from cvs)
(In reply to comment #9) > (In reply to comment #8) > > >+def fixup_ldap_repo_options(options): > > > > Hmm, why exactly do we need fixup_ldap_repo_options here? It sounds superfluous > > to me... > > Well, it follows the style of the other functions in there. We could probably > make sorting out all those options easier, but I can't see a way at a quick > glance, and it'd be a separate bug anyway. We only have fixup functions where we need to replace an existing dir with something else and move away old dirs for that. This is not the case here, so I don't think we need it. > > Why exactly do we need this still? It sounds like a lot of hackery to me... > > I'm pretty sure its for objdir builds - we need to ensure that the target > directory is created for the configure to run properly. Hmm, I wonder if we could get that into the comment. IMHO, that sounds like it really should happen inside the c-sdk configure though, ideally. Maybe we should create a bug for moving it there.
(In reply to comment #10) > (In reply to comment #9) > > (In reply to comment #8) > > > >+def fixup_ldap_repo_options(options): > > > > > > Hmm, why exactly do we need fixup_ldap_repo_options here? It sounds superfluous > > > to me... > > > > Well, it follows the style of the other functions in there. We could probably > > make sorting out all those options easier, but I can't see a way at a quick > > glance, and it'd be a separate bug anyway. > > We only have fixup functions where we need to replace an existing dir with > something else and move away old dirs for that. This is not the case here, so I > don't think we need it. Well mozilla and inspector both no longer do anything with their dirs, so they are in the same position. Although the functions are really fixing up the options for the repository they apply to, so I don't think they are that far out in naming. > > > Why exactly do we need this still? It sounds like a lot of hackery to me... > > > > I'm pretty sure its for objdir builds - we need to ensure that the target > > directory is created for the configure to run properly. > > Hmm, I wonder if we could get that into the comment. IMHO, that sounds like it > really should happen inside the c-sdk configure though, ideally. Maybe we > should create a bug for moving it there. Not sure, but we do the same thing with mozilla/ if that hasn't been created, its just not as complex.
Blocks: 614294
This is now checked in. I've filed bug 614294 for getting mxr updated to remove the old cvs files. http://hg.mozilla.org/comm-central/rev/da7dab70b9d2
Status: ASSIGNED → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
V.Fixed, per bug 614294 comment 2. At last :-)
Status: RESOLVED → VERIFIED
(In reply to comment #6) > http://hg.mozilla.org/projects/ldap-sdks/ Ftr, wrt c-sdk, *You "merged" .cvsignore files into .hgignore. *You removed ldap/examples/Makefile.
Blocks: 614978
Depends on: 615459
Depends on: 616065
Depends on: 616141
No longer depends on: 616141
Depends on: 577887
No longer depends on: 577887
Attached patch Port to 1.9.2 (obsolete) (deleted) — Splinter Review
I've discussed this with the TB build team and whilst we normally wouldn't back-port a change like this, due to the frequent failures that we're seeing from cvs, especially at release time, we have decided in this instance to back-port the change. This patch is based on the trunk patch but is minus the change of location (i.e. I've not moved directory/ to ldap/).
Attachment #504400 - Flags: review?(gozer)
Attached patch Port to 1.9.2 v2 (deleted) — Splinter Review
Correct version of the patch - forgot to hg qrefresh.
Attachment #504400 - Attachment is obsolete: true
Attachment #504414 - Flags: review?(gozer)
Attachment #504400 - Flags: review?(gozer)
(In reply to comment #15) > Created attachment 504400 [details] [diff] [review] > Port to 1.9.2 > > I've discussed this with the TB build team and whilst we normally wouldn't > back-port a change like this, due to the frequent failures that we're seeing > from cvs, especially at release time, we have decided in this instance to > back-port the change. Assuming gozer r+'s this, any chance you could whip up a version for us all for 1.9.1 for suite's benefit (and/or review a 1.9.1 port too); I'd like to get a+ from KaiRo on a plan for that [even if over IRC] though.
(In reply to comment #17) > (In reply to comment #15) > > Created attachment 504400 [details] [diff] [review] > > Port to 1.9.2 > > > > I've discussed this with the TB build team and whilst we normally wouldn't > > back-port a change like this, due to the frequent failures that we're seeing > > from cvs, especially at release time, we have decided in this instance to > > back-port the change. > > Assuming gozer r+'s this, any chance you could whip up a version for us all for > 1.9.1 for suite's benefit (and/or review a 1.9.1 port too); I'd like to get a+ > from KaiRo on a plan for that [even if over IRC] though. Last I spoke to KaiRo he didn't want to transition 1.9.1. I also don't have any spare time this week, but I expect the patch will largely apply straight-off if you want it.
Attachment #504414 - Flags: review?(gozer) → review+
Attachment #504414 - Flags: approval-thunderbird3.1.8+
Moving this to mailnews core, as the patch was all mailnews and I need the flags that are there ;-)
Component: LDAP C SDK → Build Config
Product: Directory → MailNews Core
QA Contact: csdk → build-config
Target Milestone: --- → Thunderbird 3.3a2
Version: other → Trunk
Flags: in-testsuite-
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: