Skip to content

Commit

Permalink
fix(build): correctly set target-specific variable values
Browse files Browse the repository at this point in the history
Summary:
Previously, the `HGNAME` and `OSS` variables were not being set correctly when
building with `make oss`.  In my version of GNU Make on Ubuntu 22.04:
```
GNU Make 4.3
Built for x86_64-pc-linux-gnu
```
instead of the `OSS` variable being set to `true` and `HGNAME` being set to
`sl`, the `OSS` variable was being set to `true HGNAME=sl`. This was causing
the eden binary to be built with the default `HGNAME` value of `hg`.

Here is a minimal example of the issue:

```makefile
HG_NAME = hg
oss: OSS=true HG_NAME=sl
oss: local

local:
	@echo "OSS is $(OSS)"
	@echo "Building for $(HG_NAME)"
```

On my machine `make oss` prints

```
OSS is true HG_NAME=sl
Building for hg
```

However,

```makefile
HG_NAME = hg
oss: OSS=true
oss: HG_NAME=sl
oss: local

local:
	@echo "Building for $(HG_NAME)"
```

prints

```
Building for sl
```

Test Plan:
1. On Ubuntu 22.04 and GNU Make 4.3, build sapling with `make oss`
2. should see an error like

```bash
running build_mo
rm -f hg
cp build/scripts-3*/hg hg
cp: cannot stat 'build/scripts-3*/hg': No such file or directory
make: *** [Makefile:85: local] Error 1
```

3. apply this patch and repeat step 1
4. `sl` should build successfully
  • Loading branch information
vegerot committed Oct 2, 2023
1 parent 3ce839f commit 9fc34f0
Showing 1 changed file with 2 additions and 1 deletion.
3 changes: 2 additions & 1 deletion eden/scm/Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,8 @@ help:

all: build

oss: OSS=true HGNAME=$(SL_NAME)
oss: OSS=true
oss: HGNAME=$(SL_NAME)
oss: local

install-oss: oss
Expand Down

0 comments on commit 9fc34f0

Please sign in to comment.