Streamline + consolidate the hardlink handling logic

This is one tricksy piece of code, in particular wrt interactions with
skipped files. Streamline the logic to make it easier to follow, and
consolidate as much as possible inside fsmMkfile() so there's only
one place opening, closing and writing to files. Add bunch of tests
for partially skipped sets.

This actually also fixes a case where metadata does not get properly
set on a partially skipped hardlink file set (the --excludepath=/foo/zzzz
case would fail prior to this)

lib/fsm.c

@@ -297,56 +297,45 @@ static int fsmOpen(FD_t *wfdp, const char *dest)
     return rc;
 }
 
-/** \ingroup payload
- * Create file from payload stream.
- * @return		0 on success
- */
-static int expandRegular(rpmfi fi, const char *dest, rpmpsm psm, int nodigest)
-{
-    FD_t wfd = NULL;
-    int rc;
-
-    rc = fsmOpen(&wfd, dest);
-    if (rc != 0)
-        goto exit;
-
-    rc = rpmfiArchiveReadToFilePsm(fi, wfd, nodigest, psm);
-    fsmClose(&wfd);
-exit:
-    return rc;
-}
-
 static int fsmMkfile(rpmfi fi, struct filedata_s *fp, rpmfiles files,
 		     rpmpsm psm, int nodigest,
 		     struct filedata_s ** firstlink, FD_t *firstlinkfile)
 {
     int rc = 0;
-    int numHardlinks = rpmfiFNlink(fi);
+    FD_t fd = NULL;
 
-    if (numHardlinks > 1) {
-	/* Create first hardlinked file empty */
-	if (*firstlink == NULL) {
+    if (*firstlink == NULL) {
+	/* First encounter, open file for writing */
+	rc = fsmOpen(&fd, fp->fpath);
+	/* If it's a part of a hardlinked set, the content may come later */
+	if (fp->sb.st_nlink > 1) {
 	    *firstlink = fp;
-	    rc = fsmOpen(firstlinkfile, fp->fpath);
-	} else {
-	    /* Create hard links for others */
+	    *firstlinkfile = fd;
+	}
+    } else {
+	/* Create hard links for others and avoid redundant metadata setting */
+	if (*firstlink != fp) {
 	    rc = fsmLink((*firstlink)->fpath, fp->fpath);
+	    fp->setmeta = 0;
 	}
+	fd = *firstlinkfile;
     }
-    /* Write normal files or fill the last hardlinked (already
-       existing) file with content */
-    if (numHardlinks<=1) {
-	if (!rc)
-	    rc = expandRegular(fi, fp->fpath, psm, nodigest);
-    } else if (rpmfiArchiveHasContent(fi)) {
+
+    /* If the file has content, unpack it */
+    if (rpmfiArchiveHasContent(fi)) {
 	if (!rc)
-	    rc = rpmfiArchiveReadToFilePsm(fi, *firstlinkfile, nodigest, psm);
-	fsmClose(firstlinkfile);
-	*firstlink = NULL;
-    } else {
-	fp->setmeta = 0;
+	    rc = rpmfiArchiveReadToFilePsm(fi, fd, nodigest, psm);
+	/* Last file of hardlink set, ensure metadata gets set */
+	if (*firstlink) {
+	    (*firstlink)->setmeta = 1;
+	    *firstlink = NULL;
+	    *firstlinkfile = NULL;
+	}
     }
 
+    if (fd != *firstlinkfile)
+	fsmClose(&fd);
+
     return rc;
 }
 
@@ -994,14 +983,15 @@ int rpmPackageFilesInstall(rpmts ts, rpmte te, rpmfiles files,
                 if (!IS_DEV_LOG(fp->fpath))
                     rc = RPMERR_UNKNOWN_FILETYPE;
             }
-
-        } else if (firstlink && rpmfiArchiveHasContent(fi)) {
-	    /* On FA_TOUCH no hardlinks are created thus this is skipped. */
-	    /* we skip the hard linked file containing the content */
-	    /* write the content to the first used instead */
-	    rc = rpmfiArchiveReadToFilePsm(fi, firstlinkfile, nodigest, psm);
-	    fsmClose(&firstlinkfile);
-	    firstlink = NULL;
+	} else if (firstlink && rpmfiArchiveHasContent(fi)) {
+	    /*
+	     * Tricksy case: this file is a being skipped, but it's part of
+	     * a hardlinked set and has the actual content linked with it.
+	     * Write the content to the first non-skipped file of the set
+	     * instead.
+	     */
+	    rc = fsmMkfile(fi, firstlink, files, psm, nodigest,
+			   &firstlink, &firstlinkfile);
 	}
 
 	/* Notify on success. */

tests/rpmi.at

@@ -812,5 +812,76 @@ runroot rpm -e hlinktest
 1
 ],
 [])
-AT_CLEANUP
 
+AT_CHECK([
+RPMDB_INIT
+runroot rpm -i --excludepath=/foo/zzzz /build/RPMS/noarch/hlinktest-1.0-1.noarch.rpm
+runroot rpm -Vv --nogroup --nouser hlinktest
+runroot rpm -e hlinktest
+],
+[0],
+[.........    /foo
+.........    /foo/aaaa
+.........    /foo/copyllo
+.........    /foo/hello
+.........    /foo/hello-bar
+.........    /foo/hello-foo
+.........    /foo/hello-world
+.........    /foo/zzzz (not installed)
+],
+[])
+
+AT_CHECK([
+RPMDB_INIT
+runroot rpm -i --excludepath=/foo/aaaa /build/RPMS/noarch/hlinktest-1.0-1.noarch.rpm
+runroot rpm -Vv --nogroup --nouser hlinktest
+runroot rpm -e hlinktest
+],
+[0],
+[.........    /foo
+.........    /foo/aaaa (not installed)
+.........    /foo/copyllo
+.........    /foo/hello
+.........    /foo/hello-bar
+.........    /foo/hello-foo
+.........    /foo/hello-world
+.........    /foo/zzzz
+],
+[])
+
+AT_CHECK([
+RPMDB_INIT
+runroot rpm -i --excludepath=/foo/aaaa --excludepath=/foo/zzzz /build/RPMS/noarch/hlinktest-1.0-1.noarch.rpm
+runroot rpm -Vv --nogroup --nouser hlinktest
+runroot rpm -e hlinktest
+],
+[0],
+[.........    /foo
+.........    /foo/aaaa (not installed)
+.........    /foo/copyllo
+.........    /foo/hello
+.........    /foo/hello-bar
+.........    /foo/hello-foo
+.........    /foo/hello-world
+.........    /foo/zzzz (not installed)
+],
+[])
+
+AT_CHECK([
+RPMDB_INIT
+runroot rpm -i --excludepath=/foo/hello-foo /build/RPMS/noarch/hlinktest-1.0-1.noarch.rpm
+runroot rpm -Vv --nogroup --nouser hlinktest
+runroot rpm -e hlinktest
+],
+[0],
+[.........    /foo
+.........    /foo/aaaa
+.........    /foo/copyllo
+.........    /foo/hello
+.........    /foo/hello-bar
+.........    /foo/hello-foo (not installed)
+.........    /foo/hello-world
+.........    /foo/zzzz
+],
+[])
+AT_CLEANUP