Closed Bug 339298 Opened 16 years ago Closed 14 years ago

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

Categories

(Directory :: LDAP C SDK, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: neuroc0der, Assigned: mcs)

References

Details

Attachments

(8 files)

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
Depends on: 228704
Depends on: 347933
No longer depends on: 228704
Depends on: 228704
Depends on: 349254
Attached patch merge resultsSplinter Review
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
Attachment #239715 - Flags: review?(mcs)
I had to split up the merge results - these diffs are the configure/make files.
Attachment #239716 - Flags: review?(mcs)
merge results for prldap files
Attachment #239717 - Flags: review?(mcs)
rest of merge results
Attachment #239719 - Flags: review?(mcs)
(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.
Depends on: 328791, 351020, 352519
> 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.
Attachment #239715 - Attachment is patch: true
Attachment #239716 - Attachment is patch: true
Attachment #239717 - Attachment is patch: true
Attachment #239719 - Attachment is patch: true
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.
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!
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).
(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).


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.
Depends on: 355244
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.
Attachment #239715 - Flags: review?(mcs) → review+
Attached file commit log
Merge changes have been committed.
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 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.
(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.
> 
Thanks for the review, guys.
Depends on: 355434
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
Depends on: 357668
Depends on: 362619
Depends on: 364812
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?
Status: NEW → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
(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 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.
Attachment #239716 - Flags: review?(mcs)
Attachment #239717 - Flags: review?(mcs)
Attachment #239719 - Flags: review?(mcs)
You need to log in before you can comment on or make changes to this bug.