#2487 make rebuilds version.o and relinks gnuplot despite no changes

[Tommaso Cucinotta]

When I type 'make' again and again, I'm seeing version.o recompiled and gnuplot binary(ies) relinked every time, albeit I made no changes to any sources. This happens both typing make from the top-level directory, and from the src/ one.
I found out this patch fixes the situation for me
https://sourceforge.net/u/kefren/gnuplot/ci/dd17d1599aeff2b714bb224d62d500c0ceb4c639/
(dropping .PHONY and any dependencies from timestamp.h, as the target is already NOT rebuilt if unchanged, thanks to the last line "if cmp -s ...")
However, I'm not too convinced this is really the right fix, as timestamp.h is a prerequisite for all .o files, but this rebuild problem is only occurring on version.o (note that the build process does not change any timestamp to the version.{c,h} files, AFAICS).
Perhaps someone who knows/understands better the make pipeline might have a say here...

Thanks.

[Tommaso Cucinotta]

forgot: building git master tree, with GNU make 4.3, on Ubuntu 20.10.

[Tatsuro MATSUOKA]

What version source do you use?
I have been make gnuplot binaries on the cygwin 64 using GNU make but have not met phenomena that you mention at least 5.4.2, 5.4.3 and the current git (5.5).

[Tommaso Cucinotta]

repo: https://github.com/gnuplot/gnuplot.git
branch: master
(commit: 7a547280)

~/gnuplot$ make
[...]
make[4]: Entering directory '/home/tommaso/gnuplot/src'
Making timestamp.h
depbase=`echo version.o | sed 's|[^/]*$|.deps/&|;s|\.o$||'`;\
gcc -DHAVE_CONFIG_H -I. -I..  -I../term -I../term -DBINDIR=\"/usr/local/gnuplot-git/bin\" -DX11_DRIVER_DIR=\"/usr/local/gnuplot-git/libexec/gnuplot/5.5\" -DQT_DRIVER_DIR=\"/usr/local/gnuplot-git/libexec/gnuplot/5.5\" -DGNUPLOT_SHARE_DIR=\"/usr/local/gnuplot-git/share/gnuplot/5.5\" -DGNUPLOT_PS_DIR=\"/usr/local/gnuplot-git/share/gnuplot/5.5/PostScript\" -DGNUPLOT_JS_DIR=\"/usr/local/gnuplot-git/share/gnuplot/5.5/js\" -DGNUPLOT_LUA_DIR=\"/usr/local/gnuplot-git/share/gnuplot/5.5/lua\" -DCONTACT=\"gnuplot-bugs@lists.sourceforge.net\" -DHELPFILE=\"/usr/local/gnuplot-git/share/gnuplot/5.5/gnuplot.gih\" -DGNUPLOT_X11=\"`echo gnuplot_x11 | sed 's,x,x,'`\" -DXAPPLRESDIR=\"@X11_APPDEFAULTS_DIR@\" -DGNUPLOT_QT=\"`echo gnuplot_qt | sed 's,x,x,'`\" -DQTGNUPLOT_DATA_DIR=\"/usr/local/gnuplot-git/share/gnuplot/5.5/qt\" -DQT_NETWORK_LIB -DQT_SVG_LIB -DQT_PRINTSUPPORT_LIB -DQT_WIDGETS_LIB -DQT_GUI_LIB -DQT_CORE_LIB -I/usr/include/x86_64-linux-gnu/qt5/QtNetwork -I/usr/include/x86_64-linux-gnu/qt5 -I/usr/include/x86_64-linux-gnu/qt5/QtSvg -I/usr/include/x86_64-linux-gnu/qt5 -I/usr/include/x86_64-linux-gnu/qt5/QtPrintSupport -I/usr/include/x86_64-linux-gnu/qt5 -I/usr/include/x86_64-linux-gnu/qt5/QtWidgets -I/usr/include/x86_64-linux-gnu/qt5 -I/usr/include/x86_64-linux-gnu/qt5/QtGui -I/usr/include/x86_64-linux-gnu/qt5 -I/usr/include/x86_64-linux-gnu/qt5/QtCore -I/usr/include/x86_64-linux-gnu/qt5  -g -O2 -MT version.o -MD -MP -MF $depbase.Tpo -c -o version.o version.c &&\
mv -f $depbase.Tpo $depbase.Po
c++  -g -O2 -fPIC    -o gnuplot alloc.o amos_airy.o axis.o breaders.o boundary.o color.o command.o contour.o complexfun.o datablock.o datafile.o dynarray.o encoding.o eval.o external.o fit.o gadgets.o getcolor.o graph3d.o graphics.o help.o hidden3d.o history.o internal.o interpol.o jitter.o libcerf.o matrix.o misc.o mouse.o multiplot.o parse.o plot.o plot2d.o plot3d.o pm3d.o readline.o save.o scanner.o set.o show.o specfun.o standard.o stats.o stdfn.o tables.o tabulate.o term.o time.o unset.o util.o util3d.o variable.o version.o voxelgrid.o vplot.o xdg.o    qtterminal/qt_term.o -lreadline  -lncurses  -lz   -lQt5Network -lQt5Svg -lQt5PrintSupport -lQt5Widgets -lQt5Gui -lQt5Core -ldl -lm 
gcc  -g -O2   -o gnuplot_x11 gplt_x11.o gpexecute.o version.o getcolor_x11.o -lX11  -ldl -lm 
make[4]: Leaving directory '/home/tommaso/gnuplot/src'

it recompiles all the times version.c into version.o, then links version.o into the gnuplot executable.

[Tommaso Cucinotta]

from some further diggings:

• version.c does have one peculiarity: it is the only source-code including timestamp.h:

~/gnuplot/src$ grep timestamp.h *.{c,h}
version.c:#include "timestamp.h"

• I must be building with DEVELOPMENT_VERSION enabled, likely the problem cannot show up without that

~/gnuplot/src$ grep -A3 -B3 timestamp.h version.c
const char gnuplot_version[] = "5.5";
const char gnuplot_patchlevel[] = "0";
#ifdef DEVELOPMENT_VERSION
#include "timestamp.h"
#else
const char gnuplot_date[] = "2020-02-07 ";
#endif

• from the GNU make manual, and pretty much everywhere, .PHONY targets should not be used with real file-names, which seems to be causing the problem here:

~/gnuplot/src$ make -d | grep "Must remake"
      Must remake target 'timestamp.h'.
    Must remake target 'version.o'.
  Must remake target 'gnuplot'.
  Must remake target 'gnuplot_x11'.

• now, timestamp.h is NOT actually overwritten, as it stays the same:

~/gnuplot/src$ ls -ltr
[...]
-rw-rw-r-- 1 tommaso tommaso 2804744 Jan 23 23:39 term.o
-rw-rw-r-- 1 tommaso tommaso    4368 Jan 24 00:31 version.o
-rwxrwxr-x 1 tommaso tommaso 7019896 Jan 24 00:31 gnuplot
-rwxrwxr-x 1 tommaso tommaso  382088 Jan 24 00:31 gnuplot_x11

• however, due to the fact that the target is marked PHONY, make ignores the on-disk file timestamp, and it proceeds to rebuild its dependencies anyway, in this case only one, i.e., version.o:

~/gnuplot/src$ grep " timestamp.h" .deps/*.Po
.deps/version.Po: /usr/lib/gcc/x86_64-linux-gnu/10/include/stdbool.h timestamp.h

In other words, I'm inclined to think that my patch is actually correct :-).

Thanks.

[Tommaso Cucinotta]

updated patch mentioning this bug# for extended info...
https://sourceforge.net/u/kefren/gnuplot/ci/8db9fe2d58aba5317870f5ab843020fa27b5340a/

[Ethan Merritt]

timestamp.h is not a "real" file. It is not part of the git repository, and not part of any distribution release. It exists only transiently as a phony target for make in the development version. It is there so that the executable can correctly report when it was built. I.e. when you run the program it reports:

[~/git/gnuplot-eam/src] gnuplot

        G N U P L O T
        Version 5.5 patchlevel 0    last modified 2022-01-20
                                                ^^^^^^^^^^^^^

We went through several rounds of trying to set things so that you could tell exactly where in the development process the executable was built. For a while it was reporting instead the most recent git commit hash. That makes a lot of sense but had the large downside (for me anyway) that "make" would fail if run outside of the git repository directory. So we ended up using the date instead.

[Tommaso Cucinotta]

AFAIU, it is working fine, in that with DEVELOPMENT_VERSION version.c will include and depend on timestamp.h, which will be rebuilt by the Makefile rule fetching from git the timestamp. Just, it should not be .PHONY, it's a real file (not on repo), and its rebuild rule avoids overwriting it if the git timestamp does not change, thus once you remove the .PHONY, make does the right thing (attempt to rebuild timestamp.h, but ending up not updating it (if cmp -s ....), thus not following up on to rebuilding its dependant version.o)
This is just a little annoyance for devels, just as I noticed the git version was being rebuilt all the times, I thought to have a look....

[Ethan Merritt]

I have forgotten all the messy details of what was tried earlier and what broke, and hence why we ended up with things the way they are. I seem to recall that "git bisect" did not work correctly in the immediately prior mechanism, but I don't recall now whether the .PHONY attribute was a fix for that or something else.

Why is it a problem that it re-links when you say "make"? It only takes a couple of seconds. If the intent is to see whether anything in the source has changed recently, wouldn't it be better to type git status?

[Tommaso Cucinotta]

"make" would fail if run outside of the git repository directory. So we ended up using the date instead.

yeah, I'm seeing it in the commit [ef315e6f] by Henri, the part of supporting out-of-git tree make of the DEVELOPMENT_VERSION is preserved, as it is preserved the part that removed a direct Makefile dependency of timestamp.h from the .git/HEAD file.

-git_current := $(shell cut -c6- $(top_srcdir)/.git/HEAD)
-timestamp.h: $(top_srcdir)/.git/${git_current} Makefile
+.PHONY: timestamp.h
+timestamp.h: Makefile
        @echo Making $@
        @echo "#ifndef GNUPLOT_TIMEBASE_H_INCLUDED" >$@t
        @echo "#define GNUPLOT_TIMEBASE_H_INCLUDED" >>$@t
-       @echo "const char gnuplot_date[] = \"`git --git-dir '$(top_srcdir)/.git' log -1 --format=%ci | cut -b-11`\";" >>$@t
+       @echo "const char gnuplot_date[] = \"`git --git-dir '$(top_srcdir)/.git' log -1 --date=short --format=%cd || stat -c '%.10y' Makefile.am`\";" >>$@t

You can try rebuilding with my patch above,
https://sourceforge.net/u/kefren/gnuplot/ci/8db9fe2d58aba5317870f5ab843020fa27b5340a/
you should see that you're still able to build out of a git tree (just did it right now wiping out the .git/ folder altogether, then cleaning, dist-cleaning, reconfiguring and rebuilding from scractch, seems all right).

[Ethan Merritt]

I think our goals are at cross-purposes here. With your patch in place, the timestamp is not updated when I change something and rebuild. That kind of defeats the purpose of having it. Here's an example of my typical work-flow:

git checkout my-branch-in-progress
git rebase master
./prepare
./configure --various-options
make

The .prepare step will rebuild the Makefiles, but since timestamp.h no longer depends on the Makefile it remains as it was before and is now out of date.

I honestly don't understand this .PHONY: ... business. But with or without the .PHONY I think the dependence of timestamp on Makefile is necessary at a minimum. Ideally I suspect it should depend on all source files (maybe that's what .PHONY does??) so that any change to the source updates the timestamp.

[Tommaso Cucinotta]

Ah good, now I'm getting closer to what you need:
a) under git, you want the timestamp of the last git commit, not caring about dates of local sources;
b) out of git, you want a timestamp reflecting the latest changes to the code in the local sources (pls, confirm that's your intended behavior).

As I think a) is already there, let's focus on b): you can't achieve that the way the Makefile.am is written now, because the timestamp is coming from:

stat -c '%.10y' Makefile.am
[...]
@if cmp -s $@ $@t; then rm -f $@t; else mv $@t $@; fi

so if Makefile.am changes date on disk, then you get a new timestamp, and the last "if cmp -s..." line overwrites timestamp.h because it's now different from before. However, if Makefile.am doesn't change date on disk, then the last line "if cmp -s..." will see no change of contents, and it won't update the timestamp of timestamp.h. Plus, the execution of the whole rule might be skipped altogether if you add any pre-requisite to timestamp.h. In my patch, timestamp.h has NO prerequisites, so this allow us to control this in both a) and b) cases above (I would suggest to keep it as it is, with no prerequisites).

So, I'd propose this little further change, on top of my patch:

-       @echo "const char gnuplot_date[] = \"`git --git-dir '$(top_srcdir)/.git' log -1 --date=short --format=%cd || stat -c '%.10y' Makefile.am`\";" >>$@t
+       @echo "const char gnuplot_date[] = \"`git --git-dir '$(top_srcdir)/.git' log -1 --date=short --format=%cd || stat -c '%.19y' $$(ls -t Makefile.am $(gnuplot_SOURCES) | head -1)`\";" >>$@t

This, in case of an out-of-git build, gives you a full date/time timestamp (i.e., "2022-01-24 15:47:36") of the last updated file, among all of the on-disk GNUPLOT sources, plus Makefile.am.

Would it do what you need?

Thx!