llgen "Can't rename to" *.c if /tmp is different filesystem

[kernigh]

I get errors from llgen like .../tokenfile.g, line 1: Cannot rename to tokenfile.c. OpenBSD's ktrace showed rename("/tmp/tmp.1.7PnW0LFxn", "tokenfile.c") returning -1 with errno 18 Cross-device link. This is because /tmp and the current directory are in different filesystems. The default BUILDDIR is /tmp/ack-build, but I edited Makefile to use /home/kernigh/park/ack-build, and I have /tmp and /home as separate partitions.

The recent commit 10717cc caused LLgen/src/main.c to put f_pars in /tmp. Before the commit, f_pars was in the current directory. It went to /tmp because OpenBSD's tmpnam(3) uses /tmp.

To unbreak my build, I added the local change shown below. It's awful -- mktemp() isn't portable (64f2fa9) -- but allows to me to build and run the ack for now.

diff --git a/util/LLgen/src/main.c b/util/LLgen/src/main.c
index f26446e51..a6d9c607d 100644
--- a/util/LLgen/src/main.c
+++ b/util/LLgen/src/main.c
@@ -46,7 +46,9 @@ int main(int argc, register string argv[])
    register string arg;

    TMPNAM(f_temp);
-   TMPNAM(f_pars);
+   /* TMPNAM(f_pars); */
+   memcpy(f_pars, "llgen.XXXXXXXXXX", 17);
+   mktemp(f_pars);

    /* Initialize */

[ccodere]

I did not think about this specific issue.

Let me fix this one, but its a real mess now, what do you recommend here? The temporary file is generated then in the same directory, then we need to create our own implementation of tmpnam based maybe on time_t but this could be a problem, tmpnam will not work on windows neither...

I guess maybe is simply to make sure that rename uses the full path of the file if any, do you think that makes sense, or do you foresee any other issue with this solution?

Any ideas, knowing that we need to make it portable as possible ...

[davidgiven]

Rename's only guaranteed to work if the destination file's in the same directory as the source --- otherwise you run the risk of crossing mount points (which won't work).

I think I'd probably be inclined to remove the install() logic completely. What it's doing is testing to see if the result is different from the output file, and only overwriting the output file if something's changed; it's to avoid unnecessarily updating the output timestamp and causing rebuilds. But we can trust the build system to only call LLgen if the LLgen inputs have changed, which means we expect the output to change, so I don't think this achieves anything any more.

[ccodere]

I was quite worried to change anything in the processing of LLgen so i did not dare to remove install(), i have committed a fix in the ANSI C branch that simply copies the file from source to target, let me know if this fixes the issue.

[kernigh]

Yes, it works to copy the file. Pull request #181 got merged, so I have dropped my local change and now close this issue.

I see 2 reasons for install() in LLgen to exist:

• It avoids touching an old output file if the output didn't change. This caused issue #68; David fixed it by deleting the old output files before running llgen, so now this feature is useless to us.
• It checks for the "LLgen generated code..." comment before overwriting a file. If I wrote something.c, and llgen wanted to write its own something.c, then llgen wouldn't overwrite my file. Our build system doesn't need this, because we don't put outputs in the source directory. I might want this if I would use LLgen in another project.

If I interrupt make(1) or interrupt ninja, then it deletes the output of the current job. If I hit ^C at the wrong time, make|ninja can delete something.c without checking for the comment. So install() might be less useful than it seems.