Build some examples on Linux by strager · Pull Request #9 · dvyukov/relacy

strager · 2018-11-22T20:35:44Z

Add a basic GNU Makefile which can build (make) and run (make check) two example programs on Linux (and possibly other POSIX-y platforms).

Also enable continuous integration with Travis CI [1] to catch mistakes in future commits.

[1] https://travis-ci.org/

dvyukov · 2018-11-23T08:43:12Z

Nice!
This is beyond my makefile-foo and travis-foo, so I will ask some questions.

dvyukov · 2018-11-23T08:44:33Z

GNUmakefile

+check-examples: $(example_exe_files:=.check-result)
+check-tests: $(test_exe_files:=.check-result)
+
+$(build_dir)/%.check-result: $(build_dir)/% always-run


Can't we declare this is .PHONY instead of using the fake always-run dependency? How is it different?

Can't we declare this is .PHONY

I wish! That was the first thing I tried. Frustratingly, .PHONY doesn't work in this case.

Proposal A: .PHONY

With the following code, the .PHONY line accomplishes nothing, because % isn't handled:

.PHONY: $(build_dir)/%.check-result $(build_dir)/%.check-result: $(build_dir)/%

Demonstration:

$ make $ make check example/cli_ws_deque/cli_ws_deque ... ws_deque_test iterations: 1000 [snip] throughput: 100000 test/ntest/ntest OK $ make check make: Nothing to be done for `check'.

Proposal B: .PHONY with explicit target list

In the following code, I took proposal A and manually expanded % in .PHONY.

.PHONY: $(build_dir)/example/cli_ws_deque/cli_ws_deque.check-result $(build_dir)/test/ntest/ntest.check-result $(build_dir)/%.check-result: $(build_dir)/%

The effect is that the rules are ignored when running make check, and make check always does nothing:

$ make clean $ make check make: Nothing to be done for `check'. $ make $ make check make: Nothing to be done for `check'.

See the following remark on .PHONY and implicit rules from GNU make's documentation:

The implicit rule search (see Implicit Rules) is skipped for .PHONY targets. This is why declaring a target as .PHONY is good for performance, even if you are not worried about the actual file existing.

I see.

We could also remove printf %d "$${status}" >$(@); \ line so that output files are not actually create. But I don't know what is better. Not producing output files looks a bit dirty.

dvyukov · 2018-11-23T08:46:15Z

GNUmakefile

@@ -0,0 +1,76 @@
+# User-customizable variables:


Why is this GNUmakefile rather than just Makefile? Makefile is much more common and GNU make actually does not recommend using GNUmakefile:
https://www.gnu.org/software/make/manual/html_node/Makefile-Names.html

Are you referring to the following sentence:

The first name checked, GNUmakefile, is not recommended for most makefiles.

Notice this sentence immediately after:

You should use [GNUmakefile] if you have a makefile that is specific to GNU make, and will not be understood by other versions of make.

If this file was using only standard make features, I would name it Makefile. However, this file uses a few GNU-specific features, so I wanted to be clear that this makefile only works with GNU make.

I can definitely rename it to Makefile if you prefer that name.

Yes, I would prefer this to be renamed to Makefile. This is the first time it see GNUmakefile in my entire life.

dvyukov · 2018-11-23T08:46:45Z

GNUmakefile

+TARGET_ARCH ?=
+build_dir = build
+
+.DELETE_ON_ERROR:


Do we need these 3 lines? What do they do? Was is copy-pasted some kind of template?

If I drop .SECONDARY:, make check && make check && make check compiles sources twice (once for the first make and once for the second make).

If I drop .DELETE_ON_ERROR or .SUFFIXES, things seem to work fine.

I'll keep .SECONDARY:.

dvyukov · 2018-11-23T08:56:11Z

GNUmakefile

@@ -0,0 +1,76 @@
+# User-customizable variables:
+CPPFLAGS ?=


This looks a bit over-engineered. Do we really need that many user knobs?
I would leave just CFLAGS and LDFLAGS for user, it is sufficient and enough.
TARGET_ARCH can be specified in CFLAGS. We can't have a separate var for just any compiler feature. E.g. if user wants to override warnings flags, we introduce WARNFLAGS?
LDLIBS and LOADLIBES, I don't understand difference between these two and between LDFLAGS. Libraries can always be added to LDFLAGS, no?
Supporting both CPPFLAGS and CXXFLAGS looks excessive.
Why do we need to parametrize OUTPUT_OPTION? Why not just say -o where needed?

This looks a bit over-engineered. Do we really need that many user knobs?

Good question! I do not need all these knobs; most of them are unnecessary for my needs.

I'm just supporting the variables which GNU make supports out-of-the-box if you don't have a makefile at all:

$ mkdir tmp $ cd tmp $ echo 'int main() {}' >hello.cpp $ CXX=/usr/bin/clang++ CXXFLAGS=-std=c++11 LDFLAGS=-L/ LDLIBS=-lm TARGET_ARCH=-m32 CPPFLAGS=-I/ make -B hello /usr/bin/clang++ -std=c++11 -I/ -L/ -m32 hello.cpp -lm -o hello $ ls -la total 24 drwxr-xr-x 4 mg staff 136 Nov 26 21:56 . drwxr-xr-x 14 mg staff 476 Nov 26 21:53 .. -rwxr-xr-x 1 mg staff 4228 Nov 26 21:56 hello -rw-r--r-- 1 mg staff 14 Nov 26 21:56 hello.cpp $ make --version GNU Make 3.81 Copyright (C) 2006 Free Software Foundation, Inc. This is free software; see the source for copying conditions. There is NO warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. This program built for i386-apple-darwin11.3.0

(DEPFLAGS is not used by built-in rules. I added DEPFLAGS specifically for this project.)

I would use GNU make's built-in rules without recreating them, but I couldn't see how to do that with a build directory.

LDLIBS and LOADLIBES, I don't understand difference between these two and between LDFLAGS. Libraries can always be added to LDFLAGS, no?

LDLIBS is placed after the .o files, whereas LDFLAGS is placed before the .o files. The distinction is explained in GNU make's documentation for the linking built-in rule, and is hinted at in GNU make's documentation for the LDFLAGS and LDLIBS flags.

Let me know if you still want me to remove most of these variables.

Yes.
Let's start simple and do only what we actually need. Which is no variables at all besides probably CFLAGS/LDFLAGS as they are the most common and well known.
FWIW the doc you linked says LOADLIBES is deprecated.
We can make this arbitrary complex later.

dvyukov · 2018-11-23T12:55:52Z

This builds only 1 test and 1 example for me. Is it intentional? Can we build all of them?

strager · 2018-11-27T06:06:06Z

This builds only 1 test and 1 example for me. Is it intentional?

Yes, this is intentional. I wanted to build just enough code to validate the makefile and Travis CI machinery.

Can we build all of them?

I had problems building most of the examples and tests on my machine. Many of the programs use relacy_std.hpp. relacy_std.hpp overrides names in the std namespace, and that breaks on my setup. (#2 looks related.)

I didn't try finding all of the programs which worked. I only picked the first program in tests/ and the first program in examples/ which worked.

We can extend list of tests and examples in future commits.

dvyukov · 2018-11-27T14:24:17Z

This builds only 1 test and 1 example for me. Is it intentional?

Yes, this is intentional. I wanted to build just enough code to validate the makefile and Travis CI machinery.

Can we build all of them?

I had problems building most of the examples and tests on my machine. Many of the programs use relacy_std.hpp. relacy_std.hpp overrides names in the std namespace, and that breaks on my setup. (#2 looks related.)

I didn't try finding all of the programs which worked. I only picked the first program in tests/ and the first program in examples/ which worked.

We can extend list of tests and examples in future commits.

Acknowledged.

dvyukov · 2018-11-27T14:32:15Z

Overall, if you make it a bit simpler by removing all knobs and .DELETE_ON_ERROR, I think I now understand it and ready to merge.
Thanks

Add a basic GNU Makefile which can build (`make`) and run (`make check`) two example programs on Linux (and possibly other POSIX-y platforms). Also enable continuous integration with Travis CI [1] to catch mistakes in future commits. [1] https://travis-ci.org/

dvyukov · 2018-11-30T09:15:55Z

OK, let's go with this version.
Thanks for bearing with me.

dvyukov · 2018-11-30T09:25:52Z

Woohoo! We are green:
https://github.com/dvyukov/relacy/blob/master/README.md

ChrisMThomasson · 2018-12-01T03:25:34Z

Wonderful! :^)

…

On Fri, Nov 30, 2018 at 1:25 AM Dmitry Vyukov ***@***.***> wrote: Woohoo! We are green: https://github.com/dvyukov/relacy/blob/master/README.md — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#9 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ANRPn-ccOgf1XywMWcfCC7qFqbBQcTRdks5u0PmhgaJpZM4Yv3rm> .

oracleloyall · 2020-12-25T02:54:16Z

You can try!

…

------------------ Original ------------------ From: Dmitry Vyukov <notifications@github.com> Date: Fri,Nov 23,2018 8:55 PM To: dvyukov/relacy <relacy@noreply.github.com> Cc: Subscribed <subscribed@noreply.github.com> Subject: Re: [dvyukov/relacy] Build some examples on Linux (#9) This builds only 1 test and 1 example for me. Is it intentional? Can we build all of them? — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub, or mute the thread.

dvyukov reviewed Nov 23, 2018

View reviewed changes

strager force-pushed the linux-ci branch from a8a7399 to 78b63bf Compare November 30, 2018 04:37

dvyukov merged commit 6169389 into dvyukov:master Nov 30, 2018

strager deleted the linux-ci branch November 30, 2018 18:14

Conversation

strager commented Nov 22, 2018

Uh oh!

dvyukov commented Nov 23, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Proposal A: .PHONY

Proposal B: .PHONY with explicit target list

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dvyukov commented Nov 23, 2018

Uh oh!

strager commented Nov 27, 2018

Uh oh!

dvyukov commented Nov 27, 2018

Uh oh!

dvyukov commented Nov 27, 2018

Uh oh!

dvyukov commented Nov 30, 2018

Uh oh!

dvyukov commented Nov 30, 2018

Uh oh!

ChrisMThomasson commented Dec 1, 2018 via email

Uh oh!

oracleloyall commented Dec 25, 2020 via email

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Proposal A: `.PHONY`

Proposal B: `.PHONY` with explicit target list