Refactor Makefile, update build and install targets, and fix Node.js conflict #4847

niccoloraspa · 2023-04-06T09:39:44Z

Closes: #3125

What is the purpose of the change

This PR refactors the Makefile to simplify the build and install process, as well as address an issue where the make install command may cause a conflict with Node.js's node command.

Now the make build (make install) command builds (installs) only the osmosisd binary while a new target make build-all builds all the binaries in the repo (chain, node, osmosisd, querygen).

Brief Changelog

The changes include:

Remove unnecessary targets and simplify the structure of the Makefile
Introduce GO_MODULE variable to automatically extract the module path from the go.mod file
Update build target to use GO_MODULE variable and build the osmosisd package alone
Add a new build-all target to build all packages in the current directory and its subdirectories, similar to the original build target
Update install target to use GO_MODULE variable and install the osmosisd package alone, avoiding conflicts with Node.js's node command

These changes make the Makefile more concise, improve maintainability, simplify the build and install process, and resolve the conflict with Node.js's node command.

Testing and Verifying

New build target:

❯  make build

mkdir -/github.com/osmosis-labs/osmosis-build/build/

GOWORK=off go build -mod=readonly  -tags "netgo ledger" \
-ldflags '-X github.com/cosmos/cosmos-sdk/version.Name=osmosis \
-X github.com/cosmos/cosmos-sdk/version.AppName=osmosisd \
-X github.com/cosmos/cosmos-sdk/version.Version=crosschain-swaps-v1.0.0-210-g95bb54e84 \
-X github.com/cosmos/cosmos-sdk/version.Commit=95bb54e8490a4961ea57987f613028b49632ba39 \
-X "github.com/cosmos/cosmos-sdk/version.BuildTags=netgo,ledger" \
-w -s' -trimpath -o /github.com/osmosis-labs/osmosis-build/build/ github.com/osmosis-labs/osmosis/v15/cmd/osmosisd

New build-all target:

❯ make build-all

mkdir -p /github.com/osmosis-labs/osmosis-build/build/

GOWORK=off go build -mod=readonly -tags "netgo ledger" \
-ldflags '-X github.com/cosmos/cosmos-sdk/version.Name=osmosis \
-X github.com/cosmos/cosmos-sdk/version.AppName=osmosisd \
-X github.com/cosmos/cosmos-sdk/version.Version=crosschain-swaps-v1.0.0-210-g95bb54e84 \
-X github.com/cosmos/cosmos-sdk/version.Commit=95bb54e8490a4961ea57987f613028b49632ba39 \
-X "github.com/cosmos/cosmos-sdk/version.BuildTags=netgo,ledger" \
-w -s' -trimpath -o /github.com/osmosis-labs/osmosis-build/build/ ./...

Old make build (same as the new make build-all) command:

# Old version
❯ make build 

GOWORK=off go build -mod=readonly -tags "netgo ledger" \
-ldflags '-X github.com/cosmos/cosmos-sdk/version.Name=osmosis \
-X github.com/cosmos/cosmos-sdk/version.AppName=osmosisd \
-X github.com/cosmos/cosmos-sdk/version.Version=15.0.0 \
-X github.com/cosmos/cosmos-sdk/version.Commit=ff18d8244fcda7313ec951fb1b3bee8369b8316b \
-X "github.com/cosmos/cosmos-sdk/version.BuildTags=netgo,ledger" \
-w -s' -trimpath -o /github.com/osmosis-labs/osmosis/build/ ./...

Documentation and Release Note

Does this pull request introduce a new feature or user-facing behavior changes? no
Is a relevant changelog entry added to the Unreleased section in CHANGELOG.md? no
How is the feature or change documented? not applicable

niccoloraspa · 2023-04-06T09:40:20Z

cc: @mikedotexe

niccoloraspa · 2023-04-06T09:44:23Z

Was the old make build command used in e2e anywhere @p0mvn ?

p0mvn · 2023-04-09T16:35:52Z

Was the old make build command used in e2e anywhere @p0mvn ?

No, I don't think so. There are separate Docker-image-building-related makefile steps that are used by e2e

niccoloraspa · 2023-04-10T16:49:22Z

Then we are good to merge :)

ValarDragon · 2023-04-11T13:24:17Z

Makefile

+build-all: check_version go.sum
 	mkdir -p $(BUILDDIR)/
+	GOWORK=off go build -mod=readonly $(BUILD_FLAGS) -o $(BUILDDIR)/ ./...


do we have binaries that aren't just osmosisd?

yeah! This was the cause of the issue.
The following binaries get created:

osmosisd
chain (e2e)
node (e2e)
querygen

ValarDragon · 2023-04-11T13:24:40Z

Can we get a changelog entry for the bugfix for node users?

niccoloraspa · 2023-04-11T13:27:56Z

Added CHANGELOG entry!

niccoloraspa · 2023-04-11T13:28:49Z

BTW

CODEOWNER feature works :)

…conflict (#4847) * Update targets in Makefile * Restore go 1.20 * Update CHANGELOG.md (cherry picked from commit 353933a) # Conflicts: # CHANGELOG.md # Makefile

…conflict (backport #4847) (#4890) * Refactor Makefile, update build and install targets, and fix Node.js conflict (#4847) * Update targets in Makefile * Restore go 1.20 * Update CHANGELOG.md (cherry picked from commit 353933a) # Conflicts: # CHANGELOG.md # Makefile * Update CHANGELOG.md * Update Makefile --------- Co-authored-by: Niccolo Raspa <6024049+niccoloraspa@users.noreply.github.com>

mikedotexe · 2023-04-19T16:51:54Z

cc: @mikedotexe

Thank you, and thanks Osmosis team!

daniel-farina · 2023-05-09T11:24:40Z

Friendly reminder to remove node and reinstall node in order to fix this issue.

which node
rm /root/go/bin/node

Then reinstall your preferred node version. Simply reinstalling node w/o removing the old one did not work on my end.

Update targets in Makefile

95bb54e

github-actions bot added the T:build label Apr 6, 2023

niccoloraspa requested review from p0mvn and czarcas7ic April 6, 2023 09:40

niccoloraspa added V:state/compatible/backport State machine compatible PR, should be backported A:backport/v15.x backport patches to v15.x branch labels Apr 6, 2023

Restore go 1.20

15c75cd

ValarDragon reviewed Apr 11, 2023

View reviewed changes

ValarDragon approved these changes Apr 11, 2023

View reviewed changes

Update CHANGELOG.md

fee4975

niccoloraspa requested a review from a team as a code owner April 11, 2023 13:27

niccoloraspa merged commit 353933a into main Apr 11, 2023

niccoloraspa deleted the nicco/fix-make-install-conflicts branch April 11, 2023 14:12

mergify bot mentioned this pull request Apr 11, 2023

Refactor Makefile, update build and install targets, and fix Node.js conflict (backport #4847) #4890

Merged

niccoloraspa mentioned this pull request Apr 12, 2023

Fix make build-reproducible #4901

Merged

github-actions bot mentioned this pull request Feb 15, 2024

Broken Links Detected #7491

Closed

github-actions bot mentioned this pull request Mar 15, 2024

Broken Links Detected #7752

Closed

github-actions bot mentioned this pull request Apr 1, 2024

Broken Links Detected #7903

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Refactor Makefile, update build and install targets, and fix Node.js conflict #4847

Refactor Makefile, update build and install targets, and fix Node.js conflict #4847

niccoloraspa commented Apr 6, 2023 •

edited

Loading

niccoloraspa commented Apr 6, 2023

niccoloraspa commented Apr 6, 2023

p0mvn commented Apr 9, 2023

niccoloraspa commented Apr 10, 2023

ValarDragon Apr 11, 2023

niccoloraspa Apr 11, 2023 •

edited

Loading

ValarDragon commented Apr 11, 2023

niccoloraspa commented Apr 11, 2023

niccoloraspa commented Apr 11, 2023 •

edited

Loading

mikedotexe commented Apr 19, 2023

daniel-farina commented May 9, 2023

Refactor Makefile, update build and install targets, and fix Node.js conflict #4847

Refactor Makefile, update build and install targets, and fix Node.js conflict #4847

Conversation

niccoloraspa commented Apr 6, 2023 • edited Loading

What is the purpose of the change

Brief Changelog

Testing and Verifying

Documentation and Release Note

niccoloraspa commented Apr 6, 2023

niccoloraspa commented Apr 6, 2023

p0mvn commented Apr 9, 2023

niccoloraspa commented Apr 10, 2023

ValarDragon Apr 11, 2023

Choose a reason for hiding this comment

niccoloraspa Apr 11, 2023 • edited Loading

Choose a reason for hiding this comment

ValarDragon commented Apr 11, 2023

niccoloraspa commented Apr 11, 2023

niccoloraspa commented Apr 11, 2023 • edited Loading

mikedotexe commented Apr 19, 2023

daniel-farina commented May 9, 2023

niccoloraspa commented Apr 6, 2023 •

edited

Loading

niccoloraspa Apr 11, 2023 •

edited

Loading

niccoloraspa commented Apr 11, 2023 •

edited

Loading