Transition LDAP SDKs to Mercurial (from cvs)

VERIFIED FIXED in Thunderbird 3.3a2

Status

MailNews Core
Build Config
--
minor
VERIFIED FIXED
8 years ago
6 years ago

People

(Reporter: sgautherie, Assigned: standard8)

Tracking

(Depends on: 1 bug)

Trunk
Thunderbird 3.3a2
Dependency tree / graph
Bug Flags:
in-testsuite -

Thunderbird Tracking Flags

(thunderbird3.1 .8-fixed)

Details

Attachments

(2 attachments, 2 obsolete attachments)

(Reporter)

Description

8 years ago
+++ 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.
(Assignee)

Comment 1

8 years ago
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.
(Assignee)

Comment 2

8 years ago
The message thread is here: http://groups.google.com/group/mozilla.dev.tech.ldap/msg/07608796e5e37cc4

Please post any discussion to that thread.
(Assignee)

Comment 3

7 years ago
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)
(Assignee)

Comment 4

7 years ago
Created attachment 491503 [details] [diff] [review]
Draft Patch

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+
(Assignee)

Updated

7 years ago
Blocks: 613846
(Assignee)

Updated

7 years ago
Depends on: 613921
(Assignee)

Comment 6

7 years ago
(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.
(Assignee)

Comment 7

7 years ago
Created attachment 492596 [details] [diff] [review]
The fix

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 8

7 years ago
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+
(Assignee)

Comment 9

7 years ago
(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.
(Assignee)

Updated

7 years ago
Attachment #492596 - Flags: review?(bugspam.Callek)
(Assignee)

Updated

7 years ago
Summary: Create an own hg/Mercurial repository for directory (= ldap) → Transition LDAP SDKs to Mercurial (from cvs)

Comment 10

7 years ago
(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.
(Assignee)

Comment 11

7 years ago
(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.
(Assignee)

Updated

7 years ago
Blocks: 614294
(Assignee)

Comment 12

7 years ago
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
Last Resolved: 7 years ago
Resolution: --- → FIXED
(Reporter)

Comment 13

7 years ago
V.Fixed, per bug 614294 comment 2.

At last :-)
Status: RESOLVED → VERIFIED
(Reporter)

Comment 14

7 years ago
(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.
(Reporter)

Updated

7 years ago
Blocks: 614978
(Reporter)

Updated

7 years ago
Depends on: 615459
(Reporter)

Updated

7 years ago
Depends on: 616065
(Reporter)

Updated

7 years ago
Depends on: 616141
(Assignee)

Updated

7 years ago
No longer depends on: 616141
(Reporter)

Updated

7 years ago
Depends on: 577887
(Assignee)

Updated

7 years ago
No longer depends on: 577887
(Assignee)

Comment 15

7 years ago
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.

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)
(Assignee)

Comment 16

7 years ago
Created attachment 504414 [details] [diff] [review]
Port to 1.9.2 v2

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.
(Assignee)

Comment 18

6 years ago
(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+
(Assignee)

Updated

6 years ago
Attachment #504414 - Flags: approval-thunderbird3.1.8+
(Assignee)

Comment 19

6 years ago
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
(Assignee)

Comment 20

6 years ago
Checked into 1.9.2: http://hg.mozilla.org/releases/comm-1.9.2/rev/86e548c2dde3
status-thunderbird3.1: --- → .8-fixed
(Assignee)

Updated

6 years ago
Flags: in-testsuite-
You need to log in before you can comment on or make changes to this bug.