339298 - umbrella bug for tracking the merge of Sun and Mozilla LDAPCSDK code

Status: RESOLVED FIXED

(Directory :: LDAP C SDK, defect)

Description

[Anton Bobrov]

User-Agent:       Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.8.0.3) Gecko/20060426 Firefox/1.5.0.3
Build Identifier: Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.8.0.3) Gecko/20060426 Firefox/1.5.0.3

this is an umbrella high level bug to track all major changes during 
the merge. each bug opened during the merge process that either 
represents a major feature, bugfix or any major code change otherwise 
has to be linked here for general visibility and tracking.


Reproducible: Always

Steps to Reproduce:
n/a
Actual Results:  
n/a

Expected Results:  
n/a

n/a

Comment 1

[Rich Megginson]

Attachment: merge results

I have completed the initial merge of the sun merge branch (TRUNK_MERGE_POINT_V6_0_0) with the trunk HEAD.  For the most part, the diffs are exactly the same as the diffs we have been reviewing for the sasl, ber, and ipv6 changes made to the sun merge branch.  There were only a couple of places that needed to be merged manually.
1) in request.c:re_encode_request() I needed to use ber types in ber_scanf/ber_printf - please review those to make sure I did the merge correctly.
2) Since the new sasl implementation does not touch liblber, we don't need any sasl include or library files in liblber, so I removed those from the makefile.
3) Changed some printf types in ldapsearch.c

Comment 2

[Rich Megginson]

Attachment: merge results for configure/make related files

I had to split up the merge results - these diffs are the configure/make files.

Comment 3

[Rich Megginson]

Attachment: merge results for prldap

merge results for prldap files

Comment 4

[Rich Megginson]

Attachment: rest of merge results

rest of merge results

Comment 5

[Anton Bobrov]

(In reply to comment #1)

Rich, sorry if i'm going too fast here, maybe i dont understand whats this
for exactly. this is for the new release off the HEAD, right ?!

> I have completed the initial merge of the sun merge branch
> (TRUNK_MERGE_POINT_V6_0_0) with the trunk HEAD.  For the most part, the diffs

libs version is still at "5", is that or purpose ? 

> are exactly the same as the diffs we have been reviewing for the sasl, ber, and
> ipv6 changes made to the sun merge branch.  There were only a couple of places
> that needed to be merged manually.

i see Bug 352519 prldap changes were omitted. while they aint any critical
code in there i would appreciate if you can get them in as well and so we
know its sync. i dont think we have anything else 'pending' right now.

> 1) in request.c:re_encode_request() I needed to use ber types in
> ber_scanf/ber_printf - please review those to make sure I did the merge
> correctly.

looks alright.

> 2) Since the new sasl implementation does not touch liblber, we don't need any
> sasl include or library files in liblber, so I removed those from the makefile.
> 3) Changed some printf types in ldapsearch.c
> 

same here.

on a side note. can you please mark 'patch' for those attachements next time ? 
i dunno what "by the book" in here but i personally found Bugzilla 'Diff' link
real useful, especially when there are many lines of changes involved. if it
not too much trouble and doesnt clash with the rules [pretty] please do that.

Comment 6

[Rich Megginson]

> Rich, sorry if i'm going too fast here, maybe i dont understand whats this
> for exactly. this is for the new release off the HEAD, right ?!

Yes.  This is to merge the sun_merge_branch changes into the HEAD (trunk).

>> I have completed the initial merge of the sun merge branch
>> (TRUNK_MERGE_POINT_V6_0_0) with the trunk HEAD.  For the most part, the diffs

> libs version is still at "5", is that or purpose ? 

Yes.  We will do the version change last, after the code changes.  There are still a couple of bug fixes we want to get into 6.0 as well.

>> are exactly the same as the diffs we have been reviewing for the sasl, ber, and
>> ipv6 changes made to the sun merge branch.  There were only a couple of places
>> that needed to be merged manually.

> i see Bug 352519 prldap changes were omitted. while they aint any critical
> code in there i would appreciate if you can get them in as well and so we
> know its sync. i dont think we have anything else 'pending' right now.

We didn't want to take those latest prldap changes for 6.0.  Can they wait until the next release?

> on a side note. can you please mark 'patch' for those attachements next time ? 
> i dunno what "by the book" in here but i personally found Bugzilla 'Diff' link
> real useful, especially when there are many lines of changes involved. if it
> not too much trouble and doesnt clash with the rules [pretty] please do that.

Sure.  I honestly didn't know it was working.  I've tried to use it in the past unsuccessfully, but I'll try again.

Comment 7

[Rich Megginson]

It looks like these changes are pretty safe for 6.0.  Once we get the other code merged into the trunk, we can merge these diffs too.  We are going to run some tests with this code to make sure it works well with the other code.

Comment 8

[Anton Bobrov]

Rich, i'm not sure what changes your last comment is about so i just
clarify re Bug 352519 prldap changes: if you dont have time to bother 
with them they can wait til next round but those changes are pretty 
minimal and got nowt to do with the core prldap code there so there 
shouldnt be any impact on prldap in general or your recent IPv6 stuff. 
the reason i wanted them in is simply because they add a few new apis 
that can be useful. and thanx for adding those 'diff' links btw!

Comment 9

[Rich Megginson]

We are testing the latest prldap changes you made.  I think they are ok and should make it into the trunk for 6.0 (as well as your other ldapssl_shutdown() fix).

Comment 10

[Anton Bobrov]

(In reply to comment #9)

ok, thanx a bunch!

> We are testing the latest prldap changes you made.  I think they are ok and
> should make it into the trunk for 6.0 (as well as your other ldapssl_shutdown()
> fix).

Comment 11

[Rich Megginson]

Mark, almost all of these diffs have already been reviewed, for fixes committed for the other bugs in the depends on list.  When you get a chance, please review them.

Comment 12

[Mark Smith [:mcs]]

Comment on attachment 239715 [details] [diff] [review]
merge results

OK, except for a few things I noticed:

1) There is a CVS conflict in examples/Makefile (maybe you fixed that in your tree).  Also this in examples.h:

-#define	MY_PORT		LDAP_PORT
+#define	MY_PORT		10200

This line in libldap/request.c:re_encode_request made me smile:

+ ber_int_t along;

Maybe replace the name "along" with "origmsgid" or something similar.

Comment 13

[Rich Megginson]

Attachment: commit log

Merge changes have been committed.

Comment 14

[Rich Megginson]

Attachment: merge changes from bugs 352519 352673

These diffs are to merge in the changes made for the above bugs on the sun merge branch to the trunk.  One minor change not yet reviewed was to remove an unused variable.

Comment 15

[Mark Smith [:mcs]]

Comment on attachment 241141 [details] [diff] [review]
merge changes from bugs 352519 352673

Looks OK at a glance.  Anton should review to make sure nothing was missed.

Comment 16

[Anton Bobrov]

(In reply to comment #15)

looks like everything is in order.

> (From update of attachment 241141 [details] [diff] [review] [edit])
> Looks OK at a glance.  Anton should review to make sure nothing was missed.
> 

Comment 17

[Rich Megginson]

Attachment: commit log for prldap merge

Thanks for the review, guys.

Comment 18

[Mark Smith [:mcs]]

Attachment: API changes (diff from public ldap header files)

I am posting this here for reference.  It is a diff that attempts to show the API changes present in the 6.0 (post Sun merge) LDAP C SDK release.  The diff was taken between these two tags:
  ldapcsdk_5_17_client_branch
  LDAPCSDK_6_O_O_RTM

Comment 19

[James Napolitano]

All dependent bugs here have had checkins long ago, and since there's been no activity.  Has the merging been completed?  If so, can this bug along with its dependencies be closed?

Comment 20

[Rich Megginson]

(In reply to comment #19)
> All dependent bugs here have had checkins long ago, and since there's been no
> activity.  Has the merging been completed?  If so, can this bug along with its
> dependencies be closed?
> 
Yes, let's close this bug.

Comment 21

[Mark Banner (:standard8)]

Comment on attachment 239716 [details] [diff] [review]
merge results for configure/make related files

Given we marked this bug as fixed long ago, I'm assuming that the review requests on these patches are no longer required. If they are, please re-request so that we can get them moving again.