- Download Diff

Summary:

5321 Convenience scripts to build/clean smbsrv, idmap, zfs

Review Request #23 — Created April 10, 2015 and submitted April 12, 2015, 5:16 p.m.

Information
Owner:	gwr
Repository:	illumos-gate
Branch:
Bugs:	5321
Depends On:
Reviewers
Groups:	general
People:

Description

These are some convenience scripts we use for developer "partial builds".
It makes my life a little easier upstreaming the rest of our SMB work
if I can integrate these, because later changes add to them.

Testing Done

In use for quite a while at Nexenta.

Issues

0
0
6
2
All Issues: 8

Description	From	Last Updated
whitespace nit?	risto3	April 10, 2015, 7:54 p.m.
at delphix, we're also building: $SRC/uts/intel/kmdb $SRC/uts/intel/dev in our list of kernel directories. should those two be added to the ...	prakashsurya	April 10, 2015, 7:54 p.m.
I think we may have accumulated some cruft here, but we're also building these libs: libc libnvpair libsec libdevinfo libbrand ...	prakashsurya	April 10, 2015, 7:55 p.m.
ditto	risto3	April 10, 2015, 7:52 p.m.
Our script at delphix is also building the cmds: isaexec platexec syseventd zinject terminfo mdb does it make sense to ...	prakashsurya	April 10, 2015, 7:56 p.m.
should this be moved into the 'for cmd in' list above? is it split out here for a special reason?	prakashsurya	April 10, 2015, 7:56 p.m.
similar to mdb, is this "better" than doing 'cd $SRC/cmd/syseventd && $make $1' ? just curious..	prakashsurya	April 10, 2015, 7:56 p.m.
how about adding the directories for zdb, zinject and mdb too?	prakashsurya	April 10, 2015, 3:54 p.m.

usr/src/tools/quick/make-smbsrv (Diff revision 1)
Show all issues
```
whitespace nit?
```
usr/src/tools/quick/make-zfs (Diff revision 1)
Show all issues
```
ditto
```

Ship it!

I mostly just looked at the make-zfs script, but it looks good to me. I think we'll try replace our internal script and instead use the 'make-zfs' included here after this lands.

The only thing I don't like is the fact that there's so much code duplication between the scripts. At some point (in a later patch) it might be good to move the common code to another file, and then "source" it into the actual scripts.

gwr April 10, 2015, 3:54 p.m.

Some of the differences you point our are things where I agree that make-zfs should do a little more.
I can take a stab a those and update this review.

Several of the others sound like a difference in goals and requirements.
The goal of these make-* scripts is to provide a fast and easy way to
re-run those parts of the build needed to support the "make-* tar" target.
The implementation was intentionally "minimalist", rebuilding only what is
actually required in the tar, not all dependents etc.

The developer is expected to setup the needed dependents by either:
(a) building on a machine where the native libraries etc. are sufficient,
or (b) running a full build at least once before using quick/make-* ...

It sounds like the script you're using a Delphix might be implementing
a different goal, perhaps described as: Build everthing used directly or
indirectly, including all libraries etc. with no assmption that a full
build was ever run, and no assumption that the build system provides
compabible libraries.

Those would both be quite reasonable goals; however, I find the
"quick" aspect the more valuable, because I typically use these
in an "edit, compile, (load), debug" cycle and want that fast.

Am I understanding both our goals correctly?

Perhaps we could make one script serve both goals by adding a
new target to build the additional dependent libraries that
your script builds. The developer would only need to run that
(at least) once per work space.

Thoughts on that?

prakashsurya April 10, 2015, 4:06 p.m.

Yup, I think that sums it up perfectly. We didn't want to have the full build requirement (that takes much too long ;), so it sounds like that's where the descrepancy is coming from.

I'm all for only rebuilding as little as possible, to speed up the cycle.

It'd be great if you added another target to fit our goal of not requiring a full build! That'd save me some work, since I'd have to do that regardless if we wanted to start using this.

usr/src/tools/quick/make-zfs (Diff revision 1)

Show all issues

at delphix, we're also building:

$SRC/uts/intel/kmdb
$SRC/uts/intel/dev

in our list of kernel directories. should those two be added to the list here? (probably with s/intel/$x/)

gwr April 10, 2015, 3:54 p.m.
```
Agreed, should build those too.
```

gwr April 10, 2015, 4:13 p.m.

Actually, I just looked, and those build kmdb and the devfs support,
both of which are not currently delivered by the tar.
Should they be? And if so, why?

I think we just want to build the ZFS mdb modules (for both
mdb and kmdb) but not the rest of mdb or kmdb.

usr/src/tools/quick/make-zfs (Diff revision 1)

Show all issues

I think we may have accumulated some cruft here, but we're also building these libs:

libc
libnvpair
libsec
libdevinfo
libbrand
libzonecfg
libinetutil
libladm
libdlpi
libdiskmgt
libumem
libdisasm
libidmap
libdevid

should any of these be listed here too?

I see there's some overlap between my list and the list in 'do_hdrs' so maybe we were just being lazy and doing a 'make install' when we really just needed a 'make install_h'...

gwr April 10, 2015, 3:54 p.m.
```
See my comments about goals above.
```

gwr April 10, 2015, 7:56 p.m.

I believe the "deplibs" target covers these now.

usr/src/tools/quick/make-zfs (Diff revision 1)

Show all issues

Our script at delphix is also building the cmds:

isaexec
platexec
syseventd
zinject
terminfo
mdb

does it make sense to pull these directories in here too? I can't remember why we build all of these, but I think we'd definitely need to add zinject and mdb if we were to move over to using this tool instead our internal one.

gwr April 10, 2015, 3:54 p.m.

Agreed about zinject.

Not sure we need the others, depending on which goal we're trying to achieve.
As for mdb, I want to build the ZFS mdb module, but not much else.

gwr April 10, 2015, 7:56 p.m.

I added isaexec (which is needed when installing zdb etc)
also zinject. Don't think the others are needed.

usr/src/tools/quick/make-zfs (Diff revision 1)

Show all issues

should this be moved into the 'for cmd in' list above? is it split out here for a special reason?

gwr April 10, 2015, 3:54 p.m.

Hm. I'm not sure ZFS needs this. I'll check.
It might have been left over from copying...

gwr April 10, 2015, 4:13 p.m.

Yeah, we need to build (at least) SUNW_zfs_link.o

This was separated because I was trying to pass:

  LINK_OBJS_CMN=zfs_link.o

but that didn't work (then I gave up on that:)

gwr April 10, 2015, 7:56 p.m.

I figured out how to build just the ZFS mod.

usr/src/tools/quick/make-zfs (Diff revision 1)

Show all issues

similar to mdb, is this "better" than doing 'cd $SRC/cmd/syseventd && $make $1' ? just curious..

gwr April 10, 2015, 3:54 p.m.

Is there a ZFS component of syseventd that the tar should deliver?

usr/src/tools/quick/make-zfs (Diff revision 1)

is this "better" than just 'cd $SRC/cmd/mdb && $make $1' ? that's what we're doing at delphix, so I'm just curious what the different is..

gwr April 10, 2015, 3:54 p.m.

Again about goals: If you try to build all of mdb, there are a ton more dependencies you need.
I wanted to build just the mdb modules each make-* script needs to deliver in its tar.
In this case, I think it's just the ZFS mdb module.
(Are there more mdb modules this needs to deliver?)