diff --git a/anatomy b/anatomy index 03bbd10..6b1f417 100644 --- a/anatomy +++ b/anatomy @@ -375,8 +375,6 @@ install: all $(ROOTPROG) clean: -lint: lint_PROG - include ../Makefile.targ ``` @@ -402,11 +400,6 @@ that `clean` only gets rid of intermediate files such as object files, where as of the generated programs. Because we have just one C file here, we can go directly to and from the generated binary without intermediary files. -The last target we have is `lint`. It's reason for existence is to run `lint` on -our program and describing how that works for us. In this case, we can simply -use the `lint_PROG` target to take care of doing all of this for us. That is -defined in Makefile.targ to run lint on our program for us. - Now, we can go ahead and build it and test it out. ``` @@ -443,7 +436,7 @@ $ And that's it. Now we've done and added a simple command. To finish up, we'd want to go and [add a manual page](#manual-pages) and make sure that our code is -`cstyle`, `lint`, and `pbchk` are all clean. +`cstyle` and `pbchk` are all clean. #### Multi-Architecture commands @@ -515,7 +508,6 @@ $ cat Makefile.com PROG = gethrtime OBJS = gethrtime.o -SRCS = $(OBJS:%.o=../%.c) FILEMODE = 0555 @@ -538,23 +530,16 @@ $(PROG): $(OBJS) clean: -$(RM) $(CLEANFILES) -lint: lint_PROG - include ../../Makefile.targ ``` To start it off, we have a familiar definition, `PROG`. Again, this is the name of the resulting command, mainly, `gethrtime`. Next we have to declare the -`OBJS` and `SRCS`. We use both throughout the build and to help drive things -like `lint`. In `OBJS` we place a list of our object files. In this case we only +`OBJS`. We use both throughout the build and to help drive +other targets. In `OBJS` we place a list of our object files. In this case we only have one source file and thus one object file, `gethrtime.c` and `gethrtime.o` respectively. If we were to have multiple object files, we would list them all -in that section. The next definition `SRCS` is a make pattern substitution. What -might surprise you here is the mapping of a `.o` to `../%.c`. When we do the -actual build, the current directory of any makefiles will be inside the -architecture subdirectory, in other words `cmd/gethrtime/i386`. Because all of -our source files are located in `cmd/gethrtime`, we must refer to it via -`../%.c` +in that section. The next bit we, setting `FILEMODE`, is used as part of the install target. You'll notice that the install target isn't actually defined here. It is instead @@ -585,10 +570,10 @@ files. Here we again use another macro, `COMPILE.c` which knows several things such as the compiler, the cflags, and the cppflags. You should never directly invoke the compiler, only through the macros. -From here on out, things stay relatively straightforward. Previously we didn't -worry about a `clean` target, but now that we've generated intermeidate object -files we have to go through and clean them up. The `lint` target is the same as -before. Finally, we end things with an include of `Makefile.targ` from inside of +From here on out, things stay relatively straightforward. Previously we +didn't worry about a `clean` target, but now that we've generated +intermediate object files we have to go through and clean them up. +Finally, we end things with an include of `Makefile.targ` from inside of the `cmd` directory. ##### The Architecture Makefiles @@ -708,9 +693,8 @@ all := TARGET = all install := TARGET = install clean := TARGET = clean clobber := TARGET = clobber -lint := TARGET = lint -all clean clobber lint: $(SUBDIRS) +all clean clobber: $(SUBDIRS) install: $(SUBDIRS) -$(RM) $(ROOTPROG) @@ -757,20 +741,14 @@ same target that the original makefile was invoked with. We follow this up with the specific definition for FRC. Because this does not actually generate anything, we can remain confident that it will always cause the `SUBDIRS` target to be out of date and thus cause us to run again. To wrap everything up, we do -one last include for `Makefile.targ`. This includes some of the various -definitions that we use such as `lint_PROG`. +one last include for `Makefile.targ`. #### Adding Multiple Source Files Now that we're this far, adopting this setup to have multiple source files is -actually quite simple. There are two changes that we need to make to our -`Makefile.com` file. The first is to append the new object file to the `OBJS` -variable. The second is to change how we're handling the `lint` target. -Currently the `lint` target is set to `lint_PROG`. We need to change that to -`lint_SRCS`. While the `SRCS` assignment we made in `Makefile.com` earlier may -have been mysterious, this is one of the places where it gets used. If we make -those two changes and say we have a new file called `foo.c` our Makefile.com -looks like: +actually quite simple. There is only one change that we need to make to our +`Makefile.com` file at this point: the new object file to the `OBJS` +variable. If we had a new file called `foo.c` our Makefile.com looks like: ``` $ cat cmd/gethrtime/Makefile.com @@ -791,7 +769,6 @@ $ cat cmd/gethrtime/Makefile.com PROG = gethrtime OBJS = gethrtime.o foo.o -SRCS = $(OBJS:%.o=../%.c) FILEMODE = 0555 @@ -814,8 +791,6 @@ $(PROG): $(OBJS) clean: -$(RM) $(CLEANFILES) -lint: lint_SRCS - include ../../Makefile.targ $ @@ -868,7 +843,6 @@ $ cat Makefile.com PROG = gethrtime OBJS = gethrtime.o -SRCS = $(OBJS:%.o=../%.c) FILEMODE = 0555 @@ -893,8 +867,6 @@ $(PROG): $(OBJS) clean: -$(RM) $(CLEANFILES) -lint: lint_PROG - include ../../Makefile.targ $ dmake clobber ... @@ -949,8 +921,8 @@ sparcv9/ At this top level we can see that we have two different makefiles which control different parts of the compilation process and then there are five directories. -The `common` directory contains all of the code, headers, mapfiles, and lint -libraries that are common to all architectures. For most libraries, this will +The `common` directory contains all of the code, headers, and mapfiles +that are common to all architectures. For most libraries, this will contain all of the code and headers. Next, each library should have an architecture specific directory for each supported architecture. That's why you see one for both 32-bit and 64-bit x86 and SPARC. The top level Makefile @@ -1089,11 +1061,10 @@ all := TARGET = all clean := TARGET = clean clobber := TARGET = clobber install := TARGET = install -lint := TARGET = lint .KEEP_STATE: -all clean clobber lint: $(SUBDIRS) +all clean clobber: $(SUBDIRS) install: install_h $(SUBDIRS) @@ -1138,9 +1109,9 @@ targets. The next important part is where we define all of the targets which are ISA-dependent and have them depend on `$(SUBDIRS)`. At a minimum there will -always be what you see above: specifically `all`, `clean`, `clobber`, and -`lint`. By depending on `$(SUBDIRS)` and using the conditional assignment, we -know for sure that we will recurse into each of the subdirectories and invoke +always be what you see above: specifically `all`, `clean`, and `clobber`. +By depending on `$(SUBDIRS)` and using the conditional assignment, +we know for sure that we will recurse into each of the subdirectories and invoke make with our desired target. Next we have the `install` target. If there are no header files, then the @@ -1215,14 +1186,10 @@ SRCDIR = ../common LDLIBS += -lc -$(LINTLIB) := SRCS = $(SRCDIR)/$(LINTSRC) - .KEEP_STATE: all: $(LIBS) -lint: lintcheck - include ../../Makefile.targ $ ``` @@ -1253,9 +1220,8 @@ therefore our paths need to be relative to that. Following that we declare `LIBS`. This should always be set to `$(DYNLIB) $(LINTLIB)` which takes care of making sure that whenever we're invoked we're -taking care of both the actual shared object and the corresponding lint library. -We also specify where to find the actual source code which will almost always be -in `../common`. +taking care of both the actual shared object. We also specify where to find the +actual source code which will almost always be in `../common`. All of the various flags that one would want to pass to the compiler, pre-processor, and linker need to be specified at this point, including the set @@ -1268,12 +1234,12 @@ position independent code or anything else that you might commonly pass for a library, the illumos makefiles take care of that for you. There are a few targets that all libraries will end up defining in -`Makefile.com`, specifically `lint` and `all`. `all` should simply depend on -`$(LIBS)` and `lint` on `lintcheck`. If your library requires specific targets, -for example one of your object files will come from something like `lex(1)` or -`yacc(1)`, then you'll need to add additional rules. However, if you do not have -anything special, you should not be adding your own rules to build various -aspects of your library excepting very specific and rare situations. +`Makefile.com`, specifically `all`. `all` should simply depend on `$(LIBS)`. If +your library requires specific targets, for example one of your object files +will come from something like `lex(1)` or `yacc(1)`, then you'll need to add +additional rules. However, if you do not have anything special, you should not +be adding your own rules to build various aspects of your library excepting very +specific and rare situations. To finish off the makefile, you should include `Makefile.targ` and again remember to watch out for the fact that you're including this a subdirectory @@ -1312,10 +1278,9 @@ important to note that we never define any recipes for `install` itself. We rely on the fact that each of these takes care of one aspect or another of the build. The `all` target takes care of building the library itself. `$(ROOTLIBS)` installs the library into the proto area. `$(ROOTLINKS)` takes care of -installing the compilation symlinks for the library into the proto area and -finally `$(ROOTLINT)` takes care of installing the lint library. The 64-bit -versions of these macros do the exact same thing, but ensure that everything is -set for the 64-bit version of them. +installing the compilation symlinks for the library into the proto area. The +64-bit versions of these macros do the exact same thing, but ensure that +everything is set for the 64-bit version of them. Now these makefiles can occasionally get a bit more complicated. Let's take as an example, `libproc`'s amd64 library. Here's what it looks like: @@ -1536,33 +1501,6 @@ EOF $ ``` -Now the last portion that we need is the lint library. Here's what that will -look like for our library: - -``` -$ cat > common/llib-lsolo < -EOF -``` - Great, now we've set up all of our source, so let's go onto the makefiles that we need. @@ -1600,14 +1538,10 @@ SRCDIR = ../common LDLIBS += -lc -$(LINTLIB) := SRCS = $(SRCDIR)/$(LINTSRC) - .KEEP_STATE: all: $(LIBS) -lint: lintcheck - include ../../Makefile.targ ``` @@ -1642,11 +1576,10 @@ all := TARGET = all clean := TARGET = clean clobber := TARGET = clobber install := TARGET = install -lint := TARGET = lint .KEEP_STATE: -all clean clobber install lint: $(SUBDIRS) +all clean clobber install: $(SUBDIRS) install: install_h $(SUBDIRS) @@ -1833,7 +1766,6 @@ UTSBASE = ../.. # MODULE = vnic OBJECTS = $(VNIC_OBJS:%=$(OBJS_DIR)/%) -LINTS = $(VNIC_OBJS:%.o=$(LINTS_DIR)/%.ln) ROOTMODULE = $(ROOT_DRV_DIR)/$(MODULE) CONF_SRCDIR = $(UTSBASE)/common/io/vnic @@ -1846,14 +1778,13 @@ include $(UTSBASE)/intel/Makefile.intel # Define targets # ALL_TARGET = $(BINARY) $(SRC_CONFILE) -LINT_TARGET = $(MODULE).lint INSTALL_TARGET = $(BINARY) $(ROOTMODULE) $(ROOT_CONFFILE) # # Overrides # CFLAGS += $(CCVERBOSE) -LDFLAGS += -dy -Ndrv/dld -Nmisc/mac -Nmisc/dls +LDFLAGS += -Ndrv/dld -Nmisc/mac -Nmisc/dls CERRWARN += -_gcc=-Wno-switch CERRWARN += -_gcc=-Wno-uninitialized @@ -1871,12 +1802,6 @@ clean: $(CLEAN_DEPS) clobber: $(CLOBBER_DEPS) -lint: $(LINT_DEPS) - -modlintlib: $(MODLINTLIB_DEPS) - -clean.lint: $(CLEAN_LINT_DEPS) - install: $(INSTALL_DEPS) # @@ -1899,9 +1824,8 @@ in a moment. The value of `$(OBJS_DIR)` will vary based on whether we're doing a debug or non-debug build and whether or not we're doing a 32-bit or 64-bit build. If you do a standard debug and non-debug build you'd see four directories: `obj32`, `obj64`, `debug32`, `debug64`. Each of these directories -would contain the compiled object files and the generated module. `LINTS` does a -similar transformation on the `VNIC_OBJS`, except this time for running lint. -The `ROOTMODULE` line, describes where we'll install the driver. In this case, +would contain the compiled object files and the generated module. The +`ROOTMODULE` line, describes where we'll install the driver. In this case, `ROOT_DRV_DIR` expands to `/usr/kernel/drv`. Just like with libraries, we have common makefiles that describe how to build @@ -1909,17 +1833,15 @@ targets and common definitions. With these definitions in place, we go ahead and include the first of the two common files. The following section describes what we're going to be building as a part of -each phase. Specifically, what does `make all`, `make lint`, and `make install` -responsible for putting into place. What we see here matches what we expect for -the vast majority of kernel mdoules. Specifically that the `ALL_TARGET` needs to -build the binary module as defined by `BINARY` and that it's responsible for -making sure that the configuration file is there as well. If a module doesn't -have a configuration file for some reason, then the `SRC_CONFFILE` line -would be dropped. `LINT_TARGET` defines what we do for `dmake lint`, which is to -run lint for this kernel module. Finally, the `INSTALL_TARGET` takes care of -making sure that we actually install everything into the proto area. If there -was no configuration file for the driver, then the `ROOT_CONFFILE` would be left -out. +each phase. Specifically, what is `make all` and `make install` responsible for +putting into place. What we see here matches what we expect for the vast +majority of kernel mdoules. Specifically that the `ALL_TARGET` needs to build +the binary module as defined by `BINARY` and that it's responsible for making +sure that the configuration file is there as well. If a module doesn't have a +configuration file for some reason, then the `SRC_CONFFILE` line would be +dropped. Finally, the `INSTALL_TARGET` takes care of making sure that we +actually install everything into the proto area. If there was no configuration +file for the driver, then the `ROOT_CONFFILE` would be left out. The build has default sets of flags for the compiler and link-editor. In the overrides section modules may add additional values for `CPPFLAGS`, @@ -1936,8 +1858,7 @@ differences between compilers. The `LDFLAGS` is generally the most important part for any kernel module. As you can see, ther are two different sets of flags. While these are all in `ld(1)`, -it's worth talking about them. The first `-dy`, tells the linker it should be -producing a dynamic object. Next, the `-N` arguments specify the dependencies +it's worth talking about them. The `-N` arguments specify the dependencies that this module has on other modules. The kernel run-time link-editor (krtld) ensures that all of the dependent modules are loaded before loading this module. @@ -1975,15 +1896,13 @@ driver: $(OBJS_DIR)/%.o: $(UTSBASE)/common/io/vnic/%.c $(COMPILE.c) -o $@ $< $(CTFCONVERT_O) - -$(LINTS_DIR)/%.ln: $(UTSBASE)/common/io/vnic/%.c - @($(LHEAD) $(LINT.c) $< $(LTAIL)) ``` -The first one describes the rules for building C files, the second is for -building lint. These rules are describing the directories to look in for -building. That should be the only difference between the set of rules, the -directories to look in, otherwise, the rules should look identical. +These rules are describing the directories to look in for building. That should +be the only difference between the set of rules, the directories to look in, +otherwise, the rules should look identical. The main difference you'll see +occasionally is that some of them will refer to assembly files (`.S` or `.s` +files). ### Adding a new Kernel Module @@ -2201,10 +2120,6 @@ to do is to add the following logic to the file `uts/common/Makefile.rules`: $(OBJS_DIR)/%.o: $(UTSBASE)/common/io/nr/%.c $(COMPILE.c) -o $@ $< $(CTFCONVERT_O) - - -$(LINTS_DIR)/%.ln: $(UTSBASE)/common/io/%.c - @($(LHEAD) $(LINT.c) $< $(LTAIL)) ``` The first rule describes how to build the module. With rare exception, the build @@ -2212,8 +2127,7 @@ rule should look exactly like this. This file will be included when building every kernel module, and this drives the means of building components and mapping a given file name like, `nr.o` to the file `uts/common/io/nr/nr.c`. A consequence of this design is that it means every file that is used to build a -given kernel component must have a unique name. The second rule does a similar -thing, except for building lint. +given kernel component must have a unique name. Next, we add a list of objects that are required for this module. To do that we add the following line to the file `uts/common/Makefile.files`: @@ -2263,18 +2177,14 @@ UTSBASE = ../.. MODULE = nr OBJECTS = $(NR_OBJS:%=$(OBJS_DIR)/%) -LINTS = $(NR_OBJS:%.o=$(LINTS_DIR)/%.ln) ROOTMODULE = $(ROOT_DRV_DIR)/$(MODULE) CONF_SRCDIR = $(UTSBASE)/common/io/nr include $(UTSBASE)/intel/Makefile.intel ALL_TARGET = $(BINARY) $(SRC_CONFILE) -LINT_TARGET = $(MODULE).lint INSTALL_TARGET = $(BINARY) $(ROOTMODULE) $(ROOT_CONFFILE) -LDFLAGS += -dy - .KEEP_STATE: def: $(DEF_DEPS) @@ -2285,12 +2195,6 @@ clean: $(CLEAN_DEPS) clobber: $(CLOBBER_DEPS) -lint: $(LINT_DEPS) - -modlintlib: $(MODLINTLIB_DEPS) - -clean.lint: $(CLEAN_LINT_DEPS) - install: $(INSTALL_DEPS) include $(UTSBASE)/intel/Makefile.targ diff --git a/debugging b/debugging index 24b9e46..f5be101 100644 --- a/debugging +++ b/debugging @@ -694,7 +694,7 @@ Note Section: .note(phdr) While developing and working on illumos, the build may fail for a number of reasons. It could be because of a typo in the code or you've hit an issue with a -change due to lint or packaging. +change due to smatch or packaging. If you're running something inside of `bldenv` you will see the exact line that failed as well as the command that caused it. When compiling a piece of C code, diff --git a/glossary b/glossary index c35d119..2db8a23 100644 --- a/glossary +++ b/glossary @@ -17,7 +17,7 @@ Fault management architecture. Gate is old Sun Microsystems terminology for a software source-code repository. **Lint; linting** -Using the `lint` tool to check for common programming errors. +Using the `smatch` tool to check for common programming errors. **ON** OS/Net (Operating system/Network) consolidation. In historical version of SunOS, @@ -30,7 +30,8 @@ part of the illumos gate. See [Integrating Changes](./integrating.html). **Sun Studio** A compiler suite historically required to build illumos. As of 2016, dependency -on Sun Studio has been eliminated with the exception of the `lint` tool. +on Sun Studio has been eliminated and the use of its old `lint` tool was +replaced with smatch. **UTS** Unix Time Sharing. This is a historical name for UNIX, and lives on in the diff --git a/help b/help index 4f3aed6..041de1d 100644 --- a/help +++ b/help @@ -71,8 +71,9 @@ distribution specific IRC channel which are generally also found on In any case, please remember that people can only help you if you provide them enough information. If you have a program dumping core or the operating system is panicking, then please find some way to make that available. If you -have questions about code that you have written consider making a webrev of your -changes available. +have questions about code that you have written consider making your +changes available on a git branch or with +[gerrit](https://illumos.org/docs/contributing/gerrit/). If you are hitting a bug that you do not understand in the compiler, then please include the full compilation line. In some build systems you may be required to diff --git a/integrating b/integrating index 74ef8be..e72a036 100644 --- a/integrating +++ b/integrating @@ -11,29 +11,27 @@ tips and advice to prove a smooth integration process. When you want to submit a change into illumos, we call that a request to integrate, or RTI. illumos has no single person that serves as a gatekeeper. An -RTI is sent to the advocates. An advocate who is familiar with the subject -matter will review the materials and either accept or reject the RTI. If it is -accepted, the advocate will push those change into illumos. If it is rejected, -the advocate will explain why, for example inadequate testing, and you can -return when you have corrected those issues. The advocates is made up of members -of the community and while they have commit access to the gate, they are also -required to go through the same process. +RTI is sent to the core team. A core team member who is familiar with the +subject matter will review the materials and either accept it or not. If the +change isn't accepted for whatever reason, then the core team member will +explain why. That team member will work with you to get things fixed up. Most of +the time, the core team members are just looking for some small amount of +additional explanation or confirmation about a change. The core team members go +through this same process when integrating their own changes. ### The Requirements -When you come to the advocates, you are required to have a few different pieces -of information. We'll start off by listing them now, and go into more detail a -bit later about each of them. +When you want to integrate a change, it should include the following information: - * List of Reviewers + * A list of reviewers - * Explanation of Testing + * An explanation of testing in the corresponding tickets * The output of `git pbchk` * Nightly build `mail_msg` - * The change itself (output of `git format-patch`) + * The change itself (usually a link to a review on [gerrit](https://illumos.org/docs/contributing/gerrit/) ### Shrink to Fit @@ -44,10 +42,10 @@ fixing a typo in a manual page, then you do not need to do a full nightly build and the only testing you need to do is to make sure that you didn't add any rendering pages to the new manual page. -At the end of the day, it is the advocates who are responsible for determining +At the end of the day, it is the core team who are responsible for determining whether or not something is sufficient. If you have opted to pass on one of the -requirements believing that it is not necessary, but the advocate feels that it -does, then you must address that. +requirements believing that it is not necessary, but the core team member feels +that it does, then you must address that. ### Review @@ -64,49 +62,47 @@ work in ZFS before, then that may be a warning sign to your advocate. If a reviewer is not satisfied, you may not list them as a reviewer when submitting your RTI. Furthermore, if there are issues or disagreements about -things between you and your reviewers, that is something that you need to tell -the advocates. +things between you and your reviewers, that is something that you should +mention to the core team. It's okay to still submit the change. -When providing materials for review, the best form of this is a `webrev`. For -information on generating it, please read [the workflow section on -webrev](./workflow.html#webrev). If you have written custom testing software, -you should make that available. This will not only help reviewers better -understand how your changes work, but they will also be able to provide feedback -on the testing and may be able to find cracks in the test plan. +When providing materials for review, the best thing to use here is +[gerrit](https://illumos.org/docs/contributing/gerrit/). If you have written +custom testing software, you should make that available. This will not only +help reviewers better understand how your changes work, but they will also be +able to provide feedback on the testing and may be able to find cracks in the +test plan. #### Finding Reviewers Sometimes it can be challenging to find reviewers. Often times there may be no -subject matter expert in the community. If you're uncertain of who should review -your work, you can send mail to the illumos developer list +subject matter expert in the community. If you're uncertain of who should +review your work, you can send mail to the illumos developer list `developer@lists.illumos.org`. There exist a large number of people on the list who are willing to help review work. For complicated or large changes, you'll want to give your reviewers plenty of time to do their work. Thorough reviews -are complicated. - -illumos also has sublists for different aspects of the system. If your change -deals with that area, you should consider sending it to that list. While there -are less people on these lists, they generally have more familiarity and -expertise with that specific area of code. These lists include: - - * `zfs@lists.illumos.org` for ZFS related changes - * `dtrace-discuss@lists.dtrace.org` for DTrace related changes - * `networking@lists.illumos.org` for networking related changes. +are complicated. If you're having trouble finding reviewers, you can always +reach out to the core team at `advocates@lists.illumos.org` or just reach out +in IRC. There is no requirement for sending mail to a public list for review if you already feel that you have adequate reviewers. Keep in mind that in addition to ensuring your changes are correct, part of the point of soliciting review and -having reviewers who are familiar with the area is to aid in showing your -advocate that your changes are correct. +having reviewers who are familiar with the area take a look. ### Testing You are required to do enough testing such that not only you, but others can be -confident in your changes. Testing is one place where it's really shrink to fit. -For example, the amount of testing involved for adding a new argument to the -command `head` is much less than making a change to ZFS. As part of the RTI +confident in your changes. Testing is one place where it's really shrink to +fit. For example, the amount of testing involved for adding a new argument to +the command `head` is much less than making a change to ZFS. As part of the RTI process, you will need to have a description of *how* you tested it. The how -aspect is what your advocate cares about, it's assumed that your tests pass. +aspect is what the core team members cares about, it's assumed that your tests +pass. + +This information should be recorded in the tickets. This is a useful record +when someone next is working in this area and wants to consider what to test. +It also helps in case we discover issues in the future and want to understand +what was and wasn't working in the past. #### Test Suites @@ -134,7 +130,7 @@ that there are no regressions as a side effect of your change. Part of the material that you need to submit includes the output of `git pbchk`. As discussed in the [workflow section](./workflow.html#pbchk), you should be -`git pbchk` clean. +`git pbchk` clean; however, copyright comments are always optional.. ### nightly `mail_msg` @@ -145,51 +141,45 @@ targets. Similarly, if you have touched anything to do with pacakges, you must ensure that your nightly flags build the packages and that you run a `protocmp` which checks everything that is in the proto area. -### Delta - -The advocates are going to apply your changes to the gate. As we are using -`git`, the best way to do this is by attaching the output of `git format-patch`. -While `webrev` generates a patch, it isn't always the most useful, as it leads -to people trying to manually add missing files which can be an error prone -process. +### The Change Itself -If isn't required for you to have merged your changes into the head of the tree, -but if your advocate is doing so and runs into substantial trouble with that, -they may come back to you and ask you to sync up your changes and provide an -updated patch. +To integrate a change, we're going to need the source code change itself. The +simplest way to do this is to link directly to a gerrit review. Otherwise, +attach the output of `git format-patch`. ## Submitting Once you have prepared all of the different components, it's time to submit your -change to the advocates list. You should prepare an e-mail and address it to +change to the core team list. You should prepare an e-mail and address it to `advocates@lists.illumos.org` with a subject line that describes the changes that you wish to integrate, for example, use a list of the bug ids or a subject which describes the work you're doing such as 'libproc enhancements'. You should include the following in the e-mail: - * The list of reviewers (commonly folks use the commit message for this) + * The list of reviewers (commonly folks use the commit message for this or link to the review on gerrit) * A description of the testing and links to the tests if applicable * The output of `git pbchk` You should attach the following in your e-mail: - * The `git format-patch` for your change + * The `git format-patch` for your change, if not linking directly to gerrit * The nightly `mail_msg` -You should expect the advocates to get back to you promptly, in general give +You should expect the core team to get back to you promptly, in general give them a day before you nag them. -### When Advocates Drop the Ball +### When the Core Team Drop the Ball -Like the rest of us, the advocates are human, and occasionally a given change -might fall through the cracks. It could be that the advocate most familiar with -your area. If an advocate needs time to review the contents of the change or has -questions regarding the nature of it and requests more time, that advocate -should tell you that. The advocates do not want to have changes feel like they -are sitting in limbo. +Like the rest of us, members of the core team are human, and occasionally a +given change might fall through the cracks. It could be that the core team +member most familiar with your area is not available and someone is waiting for +them to get back. If a core team member needs time to review the contents of +the change or has questions regarding the nature of it and requests more time, +that core team member should tell you that. The core team does not want to have +changes feel like they are sitting in limbo. If you don't hear from them within a day or two, send a friendly reminder as a -reply to that e-mail. If an advocate is free in `#illumos` on IRC, politely ask -them. Please remember to send a given change to all of the advocates. If you -send a given change to just one of them, that is a recipe for having your change -get dropped. +reply to that e-mail. If a core team member is free in `#illumos` on IRC, +politely ask them. Please remember to send a given change to the core team +alias (`advocates@lists.illumos.org`). If you send a given change to just one +of them, that is a recipe for having your change get dropped. diff --git a/layout b/layout index fb2de93..93f3dde 100644 --- a/layout +++ b/layout @@ -378,13 +378,6 @@ subdirectory. It is aware of the specific targets that need to be built in each subdirectory in order to perform a complete build, and itself knows how to create a skeleton proto area for later use by `install` and `install_h` targets. - * `usr/src/Makefile.lint` - -All linting from the top level is driven by this makefile. It contains long -lists of directories known to be lint-clean and contains simple recursive rules -for rebuilding each subdirectory's lint target. The actual linting is driven by -the lower-level makefiles. - * `usr/src/Makefile.master` * `usr/src/Makefile.master.64` diff --git a/workflow b/workflow index b1d6ed6..4edb5e3 100644 --- a/workflow +++ b/workflow @@ -7,148 +7,9 @@ experience building programs written in the C programming language. It will take you from obtaining a copy of the illumos source tree all the way through to getting it ready for putback. - -## Obtaining the source - -illumos uses [git](http://git-scm.com/) for its source control management. The -simplest way to obtain the illumos source is to clone the git tree. You should -first make sure that git is installed. You can do this with the following -command: - -``` -$ which git -/usr/bin/git -``` - -The location of git may be different, but if it says git is not there, you -should consult your distribution's documentation for installing packages. - -illumos can be built inside of a zone or in the global zone. It can also be -built as a user or as the super user. In most environments you will be doing -your work as a normal user. The rest of this guide will assume you are a normal -user. There should be no change if you are building as the super user. - -The primary git repository is on -[github](http://github.com/illumos/illumos-gate) under the illumos organization. -To get started you should pick a file system location to do your work. Once -there, you can use git to obtain a copy of the source tree: - -``` -$ whoami -rm -$ cd /ws/rm -$ git clone git://github.com/illumos/illumos-gate.git -Cloning into illumos-gate... -remote: Counting objects: 331190, done. -remote: Compressing objects: 100% (73583/73583), done. -remote: Total 331190 (delta 238180), reused 322706 (delta 232512) -Receiving objects: 100% (331190/331190), 211.34 MiB | 1.32 MiB/s, done. -Resolving deltas: 100% (238180/238180), done. -$ cd illumos-gate -$ ls -README exception_lists usr -``` - -Obtaining a copy of the illumos source tree may take several minutes depending -on the speed of your network connection. Feel free to place your workspace -wherever you want on the file system. The rest of this guide will assume it is -`/ws/rm/illumos-gate`. Once you've verified that you have it, we can prepare to -build the gate. - - -## Preparing to Build - -While you may only want to work on a single component of illumos, the simplest -way to do that is to do a full build of the stock gate and then start making -your changes. This makes it so you don't have to worry as much about what your -component depends on. - -You'll want to make sure that you have all the build dependencies necessary -already installed. Distributions have different ways of getting these packages. -Each distribution has their own way of getting this set up. You should consult -your illumos distribution on the best way to take care of getting this set up. - -### Setting up illumos.sh - -The easiest way to do the full build is to run nightly(1ONBLD). This takes care -of building everything that you might care about and in the right order. -`nightly` works off of a file that contains environment variables that describe -how and what to build. A set up and highly commented copy of this file already -exists for you to use. To get started we're going to copy the stock environment -variable, modify it ever so slightly, and then do the build. - -``` -$ cd /ws/rm/illumos-gate -$ cp /opt/onbld/env/illumos illumos.sh -$ vi illumos.sh -``` - -To get going there are a few important changes that you have to make this file, -specifically to tell `nightly` where your workspace is located. Following that, -we'll ensure that we're set up to use the correct compiler. - -As you're looking at the environment file, you'll find that the first thing you -encounter is the `NIGHTLY_OPTIONS` variable. This controls what's get built by -`nightly`. Sticking with the default settings will build all of the different -components for illumos in a debug-only build. If you need something else, for -example a non-debug build for building a release or performance testing, please -see the nightly(1ONBLD) manual page. - -#### Setting the path to our workspace - -Next, we need to make sure that `nightly` and other tools can find where to find -our illumos workspace. The variables that we want to modify are `GATE` and -`CODEMGR_WS`. We're going to set `GATE` to the name of the directory that we're -in and `CODEMGR_WS` to the path to it. So in our case the lines in `illumos.sh` -will look like: - -``` -export GATE="illumos-gate" - -... - -export CODEMGR_WS=$(git rev-parse --show-toplevel) -``` - -#### Setting the Compiler - -illumos uses [gcc](http://gcc.gnu.org/) as its primary compiler. gcc 7.3.0 is -the primary compiler right now and gcc 10.3.0 is the shadow compiler. Building -with both compilers is required. In addition, we leverage the tool smatch as -another shadow compiler for catching lint errors. The default illumos -environment file is configured to use gcc7 as the primary and 10 as the shadow. -You may need to tweak their paths. - -### Obtaining Closed Binaries - -While Sun made almost all of the operating system open source, there are still a -few closed components that the illumos community is replacing. You can obtain a -copy of these closed binaries from a mirror provided by Joyent. You'll want to -perform the following: - -``` -$ cd /ws/rm/illumos-gate -$ curl -O https://download.joyent.com/pub/build/illumos/on-closed-bins.i386.tar.bz2 -$ curl -O https://download.joyent.com/pub/build/illumos/on-closed-bins-nd.i386.tar.bz2 -$ tar xjvpf on-closed-bins.i386.tar.bz2 -$ tar xjvpf on-closed-bins-nd.i386.tar.bz2 -``` - -## Build illumos - -Now we can go ahead and build illumos. I suggest that you do the build inside of -a screen session or nohup, making it easier to run your build unattended. - -``` -$ cd /ws/rm/illumos-gate -$ /opt/onbld/bin/nightly ./illumos.sh -$ echo $? -0 -``` - -The amount of time that it takes `nightly` to complete varies. It can take -anywhere from forty minutes at the low end to several hours. The more CPUs -available to the build, the faster it will go. +To set up and initialize the build, see the [Building illumos +instructions](https://illumos.org/docs/developers/build/) that are part of the +normal docs. ## Diagnosing Build Failures @@ -535,52 +396,28 @@ changes down into one commit and have the chance to fix that up. You've built your changes, made some local commits, and tested them. The hard part is done! Before you can get the change integrated there are a few steps that need to be done. These steps insure that the gate has consistent style and -catches a common errors. - -### Lint and make check +helps with review. -In addition to compiler warnings, illumos-gate uses a tool called `lint` which -checks for various programming errors. Historically, compilers did not always -issue warnings about various constructs and issues which led to programmer -error. Because of that, the tool `lint` was adapted from the `pcc` compiler. -Today, the `lint` that illumos-gate uses comes from the Sun Studio Compiler -Collection. - -In addition to the `lint` checks, there is a Makefile target called `make check` -that validates aspects of header files and style. - -The easiest way to do check all of this is to simply run `nightly` again and -look at the contents of the `mail_msg`. If you see errors from there, you will -need to go back and make the appropriate fixes. - -### webrev +### Gerrit There are lots of ways that you can share your changes with other people. The -preferred format by the illumos community is that of the webrev(1ONBLD). A -`webrev` is a series of html pages that show the differences and changes in your -code. `webrev` allows people to select various kinds of `diff` formats to use to -look and review your changes. Unlike looking simply at a `diff` or `patch` file, -a `webrev` breaks down the changes on a per-file basis and allows for the reader -to get much more context than normally is possible. - -Creating a webrev is easy. All you need to do is make sure that you have your -changes committed locally. It doesn't matter how many commits you have. By -default, all of your uncommitted changes are compared to the head of the tree. -The `webrev` tool has many options for comparing your changes against different -revisions. See the `OPTIONS` section of webrev(1ONBLD) if you need something -other than the default. Otherwise, you can generate a webrev simply by doing the -following while in `bldenv`. - -``` -$ webrev -$ find /ws/rm/illumos-gate/webrev -$ +preferred format by the illumos community is using the online tool gerrit at +[code.illumos.org](https://code.illumos.org/). This tool allows folks to +provide comments and makes it easy to see what changes between releases. For +more information on getting started with gerrit see [Code Review with +Gerrit](https://illumos.org/docs/contributing/gerrit/). It will use your ssh key +that is in the bug tracker for authentication. Briefly, you will push your code +to a specially named branch, which will create the review. + +``` +git push ssh://your-username@code.illumos.org/illumos-gate your-branch:refs/for/master ``` -You'll want to share your webrev with your reviewers. If you don't have a place -to upload it or share with them, the illumos community provides some common -places for that which is `cr.illumos.org`. See XXX for more information on -using `cr.illumos.org`. +Will create a review for _your-branch_, and output its URL. Each commit in your +branch will be a separate review, though each will be grouped together, and each +needs a `Change-Id` line, as described in the Gerrit documentation (the error +message should you not do this will tell you how to enable a hook to create them +automatically, should you want to). ### Reviewers