184517 - support Solaris Packaging in NSPR 4.1.3

Status: RESOLVED FIXED

(NSPR :: NSPR, defect, P1)

Description

[Kirk Erickson]

We need support for Solaris Packaging in NSPR 4.1.3.
The differences between 
NSPRPUB_RELEASE_4_1_2_SUN_PKG_BRANCH and
NSPRPUB_RELEASE_4_1_2 need to checked into
NSPRPUB_RELEASE_4_1_BRANCH.

See http://bugzilla.mozilla.org/show_bug.cgi?id=158704

Comment 1

[Wan-Teh Chang]

You will need to create a
NSPRPUB_RELEASE_4_1_3_SUN_PKG_BRANCH for this.
There is one change on the NSPRPUB_RELEASE_4_1_2_SUN_PKG_BRANCH
that I can't check into the NSPRPUB_RELEASE_4_1_BRANCH: the
renaming of libultrasparc4.so to libnspr_flt4.so.  I can't
make such a change in a patch release.

I can, however, take this change in the trunk of NSPR for the
upcoming NSPR 4.3 release.

We can consider checking in all the other changes on the
NSPRPUB_RELEASE_4_1_BRANCH, and then create a
NSPRPUB_RELEASE_4_1_3_SUN_PKG_BRANCH just for the
libultrasparc4.so-to-libnspr_flt4.so change.

Comment 2

[Ian McGreer]

Would it be possible to copy/rename the file when building the package?  That
will be effectively the same, in that only users of Solaris packages will
encounter libnspr_flt4.so, while avoiding the creation of a new branch.

Comment 3

[Wan-Teh Chang]

This needs more than just copying or renaming the file.
libnspr4.so needs to be built with the -f linker flag
with the name of the "filtee".  Currently the filtee
is libatomic.so.  In the Sun pkg branch the filter is
$ORIGIN/$ISALIST/libnspr_flt4.so.  This string is
recorded in the libnspr4.so binary.

We can add a new make variable BUILD_SUN_PKG
so that NSPR builds libultrasparc4.so by default
and libnspr_flt4.so if BUILD_SUN_PKG is set to 1.
This will require adding some ifdefs.

Comment 4

[Kirk Erickson]

Attachment: patch for NSPRPUB_RELEASE_4_1_BRANCH

Note, someone needs to create
NSPRPUB_RELEASE_4_1_3_SUN_PKG_BRANCH for checkin since it
changes the name of the filter library.
Sonja, can you do this for us?

Before applying the patch, create new directories:

mkdir -p mozilla/nsprpub/pkg/solaris/common_files/
mkdir -p mozilla/nsprpub/pkg/solaris/SUNWprx
mkdir -p mozilla/nsprpub/pkg/solaris/SUNWpr
patch < 2002-1210.patch

Comment 5

[Sonja Mirtitsch]

I do not want an additional BRANCH. I thought we had discussed in email that
Wan-Teh's suggestion:

> We can add a new make variable BUILD_SUN_PKG
> so that NSPR builds libultrasparc4.so by default
> and libnspr_flt4.so if BUILD_SUN_PKG is set to 1.
> This will require adding some ifdefs.

 was good, and I had hoped that you or Ian would be working on it. I assume it
has not even been started?

Comment 6

[Kirk Erickson]

> I do not want an additional BRANCH. I thought we had discussed
> in email that Wan-Teh's suggestion...
> was good, and I had hoped that you or Ian would be working on it. 
> I assume it has not even been started?

Good assumption - this is the first I've heard of your objection.
Why is creating a BRANCH problematic?

Comment 7

[Sonja Mirtitsch]

well, some of the discussion on BRANCH or not was on this bug, and I had assumed
you had followed the email thread too. I'll forward it to you.

Comment 8

[Kirk Erickson]

Attachment: patch for NSPRPUB_RELEASE_4_1_BRANCH

I tested building all combinations with and without:

    BUILD_OPT=1
    BUILD_SUN_PKG=1
    USE_64=1

This alleviates the need to create a new branch;
its targeted for NSPRPUB_RELEASE_4_1_BRANCH.
Following Wan-Teh's suggestion, I changed the end of
./mozilla/nsprpub/config/SunOS5.mk:

ifeq ($(OS_TEST),sun4u)
ifeq ($(BUILD_SUN_PKG),1)
ULTRASPARC_LIBRARY = nspr_flt
ULTRASPARC_FILTER_LIBRARY = libnspr_flt4.so
else
ULTRASPARC_LIBRARY = ultrasparc
ULTRASPARC_FILTER_LIBRARY = libatomic.so
endif
DSO_LDOPTS  += -f $(ULTRASPARC_FILTER_LIBRARY)
endif

Note - there's no need to create new directories before
applying this version.

Comment 9

[Kirk Erickson]

Sorry - you do still to create the new subdirs.
To apply the patch:

mkdir -p mozilla/nsprpub/pkg/solaris/common_files
mkdir -p mozilla/nsprpub/pkg/solaris/SUNWprx
mkdir -p mozilla/nsprpub/pkg/solaris/SUNWpr
patch <XXX.patch

Comment 10

[Sonja Mirtitsch]

 Thanks for getting this done quickly, but I'd still like the patch to be
reviewed by Wan-Teh before checking it in.

Comment 11

[Sonja Mirtitsch]

We also need to generate a prototype file with the solarispackage, this has been
missed in the original implementation of the package for NSPR 4.1.2 the
prototype is not crucial to the package itself, but it seems to be absolutely
necessary to generate a patch to the package. The combination of prototype_com
and prototype_sparc (or prototype_com and prototype_i386) is not accepted by
buildpatch.
sorry about noticing this so late.

Comment 12

[Wan-Teh Chang]

Attachment: patch for NSPRPUB_RELEASE_4_1_BRANCH, v3

I edited Kirk's patch.	I've checked in this patch on
the NSPRPUB_RELEASE_4_1_BRANCH.

Comment 13

[Sonja Mirtitsch]

Attachment: patch to generate the prototype file

Comment 14

[Ian McGreer]

Comment on attachment 109188 [details] [diff] [review]
patch to generate the prototype file

Sonja-

I don't know if you mean any Sun developer :), but I can rubber-stamp the patch
in that Danek's email indicates it will work.  But probably you want someone
who understands the package system to review it.

Comment 15

[Kirk Erickson]

I'm no packaging expert, but I don't see the motivation for the
prototype patch (109188).  Worst case, we can create the single 
file on the fly when we run buildpatch. 

Comment 16

[Wan-Teh Chang]

Comment on attachment 109188 [details] [diff] [review]
patch to generate the prototype file

Please break the long sed command into multiple lines.
Each line should ideally be less than 80 column wide.
Thanks.

Comment 17

[Sonja Mirtitsch]

Thanks Ian, checking it in!

The patch experts in Byron Gutierez team are at the moment too busy to be able
to tell us if the  build time timestamp is needed on the prototype file.

Nobody who is CC'd on this bug seems to know but, and Serge Dusson and Frank
Throckmorton who have done a lot of patches have stated they do not know of any
way to do a buildpatch without a complete prototype file.

Since it is such a hot topic and a lot of emails seem to get lost lately I'll
paste the patch related email conversations and attach the half finished patch
script

=======

Hi Sonja,

Sonja Mirtitsch wrote:

> 

> >> I can't access /net/pip-tools/eng, no ping of pip-tools, ypcat host doesn't
show a machine with a similar name. I searched for buildpatch in /usr/dist and
could not find anything
> >>

> > ok, this was a typo ... have you tried /net/pip-tools.eng/latest ?
> > see also http://wwwspg.eng.sun.com/Patch/
> >

> thanks, that one works, and wants a lot more parameters. It still has the same
problem though.
> 


I've been using it for quite a while without problems, but I always 
provide a complete prototype file in the Util area.


> >> > I think this says that you didn't provide the prototype file for this
package. This
> >> > prototype is expected to be in /home/sonmi/patchdefs/test/114045/SUNWpr
(that is, the
> >> > 3rd arg of our '-P' option to buildpatch).
> >> >

> >> I had understood that it will also accept a prototype_com and
prototype_sparc combination (or prototype_com and prototype_i386) which reside
in this directory. Our package does not have prototype files other than these
> >>

> > I think that buildpatch need a regular and complete prototype file.
prototype_com and prototype_i386 are
> > usually used to build the final prototype file before building the package
(can be seen in the relevant
> > makefile, if I am not mistaken).
> >

> No, in our Makefile.targ the pkgmk is called directly with prototype_$(MACH)
and the prototype_com is never mentioned. I read the packaging guide it says
nothing about a need to generate the  prototype file
http://solaris.eng/benet/Packaging/SW_Pkg_Rqrmts,v4_0.pdf. I also tried to find
info on prototype file in the patches faq.
> Do you think I can just run pkgproto right before buildpatch to generate the
prototype?


you are refering here to packages docs, not patches docs. Please refer to
the URLs I've previously sent to you, including the buildpatch API URL:

http://wwwspg.eng.sun.com/Patch/
http://pst.ireland/FAQ/index.shtml
http://installzone.eng.sun.com/tsk/specs/PatchArch/buildpatch.api.html

HTH,
Serge




> 
> Sonja

Sonja,

Sonja Mirtitsch wrote:

> 
> Hi Serge,
> 

> >> > the complete output (mkpatch.log)
> >> > /net/mcm-ultra1/export/sims3-tools/SUNWpatch/buildpatch -v -f directory
> >> >

> > make sure you use the latest buildpatch (available from
/net/pip-tools/eng/latest
> >

> I can't access /net/pip-tools/eng, no ping of pip-tools, ypcat host doesn't
show a machine with a similar name. I searched for buildpatch in /usr/dist and
could not find anything


ok, this was a typo ... have you tried /net/pip-tools.eng/latest ? 
see also http://wwwspg.eng.sun.com/Patch/

> 

> >> > -p 114045-01 -M /home/sonmi/patchdefs/test/114045/patch_files -d
> >> > /home/sonmi/patchdefs/test/dist/patches -P SUNWpr /tmp/114045/nspr
> >> > /home/sonmi/patchdefs/test/114045/SUNWpr
> >> > Constructing patch 114045-01
> >> > ERROR: Couldn't find
> >> > /home/sonmi/patchdefs/test/dist/patches/114045-01/PROTO_SUNW
> >> > pr/prototype.
> >> >

> > I think this says that you didn't provide the prototype file for this
package. This
> > prototype is expected to be in /home/sonmi/patchdefs/test/114045/SUNWpr
(that is, the
> > 3rd arg of our '-P' option to buildpatch).
> >

> I had understood that it will also accept a prototype_com and prototype_sparc
combination (or prototype_com and prototype_i386) which reside in this
directory. Our package does not have prototype files other than these.
> 



I think that buildpatch need a regular and complete prototype file.
prototype_com and prototype_i386 are
usually used to build the final prototype file before building the package (can
be seen in the relevant
makefile, if I am not mistaken).

HTH,
Serge



> > As an example, this is the command I run to generate a patch for S1DS 5.1 on S9:
> > buildpatch -f directory -v -7 -w -k null -M <mydir>/PatchMetaFiles -p
113859-01 -d <my_destination_dir> -P IPLTadcon / <my_pkgdef_dir>/IPLTadcon/Util
 -P IPLTadmin / <my_pkgdef_dir>/IPLTadmin/Util  -P IPLTcons /
<my_pkgdef_dir>/IPLTcons/Util  -P IPLTdscon / <my_pkgdef_dir>/IPLTdscon/Util  -P
IPLTdsu / <my_pkgdef_dir>/IPLTdsu/Util  -P IPLTnss /
<my_pkgdef_dir>/IPLTnss/Util  -P IPLTjss / <my_pkgdef_dir>/IPLTjss/Util  -P
IPLTnls / <my_pkgdef_dir>/IPLTnls/Util  -P IPLTnspr /
> > <my_pkgdef_dir>/IPLTnspr/Util  -P IPLTpldap / <my_pkgdef_dir>/IPLTpldap/Util
> > but before calling buildpatch, I did make sure that everything I need to
build a package
> > is under <my_pkgdef_dir>/<my_pkg>/Util. This is where you shall put the
prototype
> > file (as well as the pkginfo file, and m
> > aybe some others).
> > HTH,
> > Serge
> >

> >> > Quitting buildpatch. Patch construction directory
> >> > has been created but no patch has been constructed.
> >> > It is right, this file does not exist, but from your sample scripts it
> >> > doen't look l
> >> > ike we need it.
> >> > I had also hoped that you could have a look at the script, and if
> >> > possible do a full  review, or maybe we can review it together. I tried
> >> > to comment everything, it is a little complicated since we deal with 6
> >> > (and in the future 8) platforms, and all have different filelists, and a
> >> > lot of other things are different from your scripts too. A few things
> >> > that were unclear to me have the string ASK in a comment followed by a
> >> > question. Known problems are marked with the string FIXME. I started to
> >> > put some code in for nss and jss, which will be patched with the same
> >> > patch in the future. I noticed you have an SCCS area, which I am not
> >> > clear about. Namly the files s.copyright and s.prototyp
> >> > e get referenced
> >> > in your script, could you please explain what it is for?
> >> > I attach the current version of the script, and the output logs, it has
> >> > a function mkcommon, which gets the files for the common area, and I am
> >> > not 100% certain that I get the right ones (log is mk
> >> > comon.log).
> >> > My work area is in /home/sonmi/patchdefs/test, the common area is
> >> > /home/sonmi/patchdefs/test114045-comon. I put the complete debugging
> >> > output of the script being run 3 log files there
> >> > Thanks
> >> > Sonja
> >> > 

Comment 18

[Sonja Mirtitsch]

Attachment: patchscript

unfinished

Comment 19

[Kirk Erickson]

Comment on attachment 109188 [details] [diff] [review]
patch to generate the prototype file

Sonja, don't you mean to use prototype_$(MACH), rather than hardcoding _sparc?

why can't your patchscript do this?

Comment 20

[Sonja Mirtitsch]

Attachment: corrected patch to generate the prototype file

Comment 21

[Sonja Mirtitsch]

Ian, could you please review once more?

Kirk, the Manager Byron Gutierrez of the Solaris patch group has answered on if
there is a need for the prototype, or the _com _sparc and _i386 are sufficient.
I'll forward you the entire email as well.


Sonja,

I feel your frustration.

In ON our prototype file is seperated into prototype_com, prototype_sparc, and
prototype_x86, when then create a intermediate packaging template from which we
generate the patch.  Am not sure why the patch people are calling this a 'hack',
since this is how we've done things all along.

Anyhow, I have a some free time this afternoon, will take a look then.  It
sounds to me that you need something simular to our process which generates a
intermediate form which build patch is happy with.

Will get back to you on this shortly after lunch.

ByRoN

Comment 22

[Ian McGreer]

Comment on attachment 109243 [details]
corrected patch to generate the prototype file

Again, this is rubber-stamping.  One nit is that while it is ideal to have all
lines less than 80 characters, in this case I think it is better to indent the
last two lines and allow them to spill over.  Sometimes make can be picky about
indentation, though with the line continuations it shouldn't matter.

Comment 23

[Kirk Erickson]

Comment on attachment 109243 [details]
corrected patch to generate the prototype file

> Kirk, the Manager Byron Gutierrez of the Solaris patch group has
> answered on if there is a need for the prototype, or the _com
> _sparc and _i386 are sufficient.  I'll forward you the entire
> email as well.

Its clear you wouldn't want to give buildpatch _com or _sparc.
It needs a single file called 'prototype' which is a cat of both.
You have yet to address my question:

Why not create the prototype file in your patchscript?

I believe this is what Byron and the others are doing.

Comment 24

[Michael Hein]

I'm very disappointed that we have not been able to get this done by now.  I'm 
unfortunately at a position where I can't wait any longer as Sonja is going on 
vacation.  I have asked Sonja to track the changes here locally, and we will be 
proceeding independently for this release.

Michael

Comment 25

[Sonja Mirtitsch]

I did the release build with a tree that differs from the checked in version:

  1) HP-UX 11i -D_USE_BIG_FDS
  2) solaris patch change to generate the prototype file

In both case, unfortunately I could not prove beyond any doubt that the changes
were absolutely necessary.

I will attach the diff. A version of NSS is now building with our nspr 4.1.3, it
differs from the checked in version in exactly the same points. I'll spare
myself the trouble of trying to get the there changes reviewed, since we already
ran out of time, and I do not want to go through this once more. From a QA point
of view I'd like to note that it is a disaster to have manual release builds
that are different from the tree. 

Comment 26

[Sonja Mirtitsch]

Attachment: difference between the tagged beta and the build tree