Add Makefile

[ashwin95r]

This change is

[coveralls]

Coverage remained the same at 52.639% when pulling 9b2f9bd on feature/makefile into 5e9d829 on develop.

[manishrjain]

Reviewed 1 of 1 files at r1.
Review status: all files reviewed at latest revision, 3 unresolved discussions.

---

Makefile, line 8 [r1] (raw file):

build: BUILDMODE = build
build: install
  mkdir dgraph;\

create a var for tmp directory. Possibly point it to /tmp/dgraph-build or something. Delete if it already exists.

Do the tar from within it, because we're no longer doing dgraph/binary. Remember to delete it at the end.

---

Makefile, line 10 [r1] (raw file):

  mkdir dgraph;\
  cd dgraph;\
  cp ../cmd/dgraph/dgraph .;\

This seems relative to current directory.

---

Makefile, line 25 [r1] (raw file):

dgraph:
  cd "./cmd/dgraph" ; \
  $(GO) $(BUILDMODE) --tags=embed . ; 

tags embed should be part of buildmode.

---

Comments from Reviewable

[ashwin95r]

Review status: 0 of 1 files reviewed at latest revision, 3 unresolved discussions.

---

Makefile, line 8 [r1] (raw file):

Previously, manishrjain (Manish R Jain) wrote…

create a var for tmp directory. Possibly point it to /tmp/dgraph-build or something. Delete if it already exists.

Do the tar from within it, because we're no longer doing dgraph/binary. Remember to delete it at the end.

It creates a directory in the dgraph root folder, copies the binaries into it and creates a tar file without the parent directory and then deletes the parent directory.

---

Makefile, line 10 [r1] (raw file):

Previously, manishrjain (Manish R Jain) wrote…

This seems relative to current directory.

Yes

---

Makefile, line 25 [r1] (raw file):

Previously, manishrjain (Manish R Jain) wrote…

tags embed should be part of buildmode.

Done.

---

Comments from Reviewable

[coveralls]

Coverage remained the same at 52.639% when pulling c57b044 on feature/makefile into 5e9d829 on develop.

[coveralls]

Coverage remained the same at 52.639% when pulling e767e49 on feature/makefile into 5e9d829 on develop.

[manishrjain]

Reviewed 1 of 1 files at r3.
Review status: all files reviewed at latest revision, 2 unresolved discussions.

---

Makefile, line 8 [r1] (raw file):

Previously, ashwin95r (Ashwin Ramesh) wrote…

It creates a directory in the dgraph root folder, copies the binaries into it and creates a tar file without the parent directory and then deletes the parent directory.

Yeah, I understood that. My point was to not create a directory in the repo itself. If the build fails, you end up with all these remaining files in your git folder.

---

Makefile, line 14 [r3] (raw file):

  cp ../cmd/dgraphmerge/dgraphmerge .;\
  cd ..;\
  tar -zcf dgraph-linux-amd64-v0.4.0.tar.gz -C dgraph-build .;\

Add a var for version number. And for linux or darwin. Can't this makefile figure out which platform this is and name accordingly?

---

Comments from Reviewable

[coveralls]

Coverage decreased (-0.04%) to 52.598% when pulling bbc978d on feature/makefile into 5e9d829 on develop.

[coveralls]

Coverage remained the same at 52.639% when pulling bbc978d on feature/makefile into 5e9d829 on develop.

[manishrjain]

Looks clean and easy to understand. Good work!

---

Reviewed 2 of 2 files at r4.
Review status: all files reviewed at latest revision, 6 unresolved discussions, some commit checks failed.

---

build.sh, line 11 [r4] (raw file):

  rm -rf $tmp_dir
fi

Test if strip is available.

---

build.sh, line 18 [r4] (raw file):

echo -e "\033[1;33mBuilding binaries\033[0m"
echo "dgraph"
cd $dgraph_cmd/dgraph && go build .;

Use a build flags variable.

---

build.sh, line 31 [r4] (raw file):

cd $tmp_dir;
cp $dgraph_cmd/dgraph/dgraph $dgraph_cmd/dgraphassigner/dgraphassigner $dgraph_cmd/dgraphlist/dgraphlist $dgraph_cmd/dgraphmerge/dgraphmerge $dgraph_cmd/dgraphloader/dgraphloader .;

Strip the binaries here. Report total size after strip.

---

build.sh, line 35 [r4] (raw file):

tar_file=dgraph-"$(uname | tr '[:upper:]' '[:lower:]')"-amd64-v$release_version
tar -zcf $tar_file.tar.gz dgraph dgraphassigner dgraphlist dgraphmerge dgraphloader;

Report size of tar.

---

Comments from Reviewable

[pawanrawal]

Review status: 0 of 1 files reviewed at latest revision, 6 unresolved discussions.

---

build.sh, line 11 [r4] (raw file):

Previously, manishrjain (Manish R Jain) wrote…

Test if strip is available.

Doing it before stripping the files.

---

build.sh, line 18 [r4] (raw file):

Previously, manishrjain (Manish R Jain) wrote…

Use a build flags variable.

Did you mean using `-tags=embed` or a variable for it?

---

Comments from Reviewable

[coveralls]

Coverage remained the same at 52.639% when pulling a03821b on feature/makefile into 5e9d829 on develop.

[coveralls]

Coverage remained the same at 52.639% when pulling 884b8e7 on feature/makefile into 5e9d829 on develop.

[pawanrawal]

Review status: 0 of 1 files reviewed at latest revision, 6 unresolved discussions.

---

build.sh, line 11 [r4] (raw file):

Previously, pawanrawal (Pawan Rawal) wrote…

Doing it before stripping the files.

Done

---

build.sh, line 18 [r4] (raw file):

Previously, pawanrawal (Pawan Rawal) wrote…

Did you mean using -tags=embed or a variable for it?

Done

---

build.sh, line 31 [r4] (raw file):

Previously, manishrjain (Manish R Jain) wrote…

Strip the binaries here. Report total size after strip.

Done

---

build.sh, line 35 [r4] (raw file):

Previously, manishrjain (Manish R Jain) wrote…

Report size of tar.

Done

---

Makefile, line 14 [r3] (raw file):

Previously, manishrjain (Manish R Jain) wrote…

Add a var for version number. And for linux or darwin. Can't this makefile figure out which platform this is and name accordingly?

Done

---

Comments from Reviewable

[coveralls]

Coverage remained the same at 52.639% when pulling c0e6a5b on feature/makefile into 5e9d829 on develop.

[manishrjain]

One small wording change. Otherwise, LG! Good job!

---

Reviewed 1 of 1 files at r7.
Review status: all files reviewed at latest revision, 2 unresolved discussions.

---

build.sh, line 16 [r7] (raw file):

if ! type strip > /dev/null; then
  echo -e "\033[0;31mYou don't have strip command line tool available. Download it and try again.\033[0m"

Install it and try again.

---

Comments from Reviewable

[pawanrawal]

Merged.