build: deprecate mk-deb.sh mk-tar.sh and use make targets instead #2644

charlie0129 · 2023-07-23T17:53:33Z

What problem does this PR solve?

Closes #2024

What is changed and how it works?

Major changes:

Deprecate mk-deb.sh and mk-tar.sh.
Use make targets instead: make <deb|tar> [options]
- make tar dep=1
- make deb release=0 dep=0

The original version builds on the host. I have to make heavy changes to the script to adapt to the new containerized build system. Now all build process should fit in the new containerized manner, i.e., using docker build conatiner, just like make build.

Minor changes:

Makefile of etcdclient has been improved to avoid unnecessary rebuilds and clones. It should be faster when building depencencies after the first time.
Allow multiple dirs in only like make xxx only=src/*,tools/*, instead of just one.
Allow specifying custom build options on util/build_in_image.sh
Common docker-related arguments have been put into docker_opts.sh
Made scripts in util/* executable.

Side effects(Breaking backward compatibility? Performance regression?):

The build result should be the same, to the best of my knowledge. I have tested it on a bare machine.

Documents that are affected by this change has been updated.

Check List

Relevant documentation/comments is changed or added
I acknowledge that all my contributions will be made under the project's license

charlie0129 · 2023-07-23T17:54:10Z

cicheck

wuhongsong · 2023-07-26T06:13:22Z

cicheck

Cyber-SiKu · 2023-07-29T08:40:25Z

There is a problem with mk-tar now, you can fix it:
Compile the project first, and then compile curvefs_python. When the following statement is executed, some binary files will not be found. For example here:

curve/mk-tar.sh

Line 227 in 4a0372b

cp ./bazel-bin/src/mds/main/curvemds build/curve/curve-mds/bin/curve-mds

The cause is this:
In a certain change, the code for compiling the entire project was changed to the following:

curve/mk-tar.sh

Line 175 in 4a0372b

make build stor=bs release=0 dep=1 only=src/*

But compiling curvefs_python is directly using bazel.

curve/mk-tar.sh

Lines 108 to 119 in 4a0372b

    
           if [ "$1" = "release" ]; then 
        
               bazel build curvefs_python:curvefs --copt -DHAVE_ZLIB=1 --copt -O2 -s \ 
        
                   --define=with_glog=true --define=libunwind=true --copt -DGFLAGS_NS=google \ 
        
                   --copt -Wno-error=format-security --copt -DUSE_BTHREAD_MUTEX --linkopt \ 
        
                   -L${dir}/curvefs_python/tmplib/ --copt -DCURVEVERSION=${curve_version} \ 
        
                   ${bazelflags} 
        
           else 
        
               bazel build curvefs_python:curvefs --copt -DHAVE_ZLIB=1 --compilation_mode=dbg -s \ 
        
                   --define=with_glog=true --define=libunwind=true --copt -DGFLAGS_NS=google \ 
        
                   --copt -Wno-error=format-security --copt -DUSE_BTHREAD_MUTEX --linkopt \ 
        
                   -L${dir}/curvefs_python/tmplib/ --copt -DCURVEVERSION=${curve_version} \ 
        
                   ${bazelflags}

The inconsistency of the compilation parameters caused bazel-bin to establish a new soft link. There is no corresponding binary file under the new soft link.

wuhongsong · 2023-08-14T03:12:32Z

@charlie0129 continue？

charlie0129 · 2023-08-14T07:59:01Z

Yes, I am going to fix that. The build script changed to use docker builder instead before I was able to make changes so I am adapting my script to use the new method.

charlie0129 · 2023-08-14T13:14:26Z

Hi @Cyber-SiKu How can I build curvefs_python using the new util/build.sh script, if we want to avoid using bazel directly?

Cyber-SiKu · 2023-08-22T02:22:01Z

@charlie0129 You can see how the script does it.

charlie0129 · 2023-08-22T12:33:24Z

@Cyber-SiKu Most build procedure is now done. But one problem, I was only able to compile /curvefs_python/libcurvefs.a, not a .so lib using make build only=curvefs_python/curvefs (with appropriate linker options to curvefs_python/tmplib).

PS: The old build command (using bazel directly) is not working in the new docker build container. The error is:

src/client/iomanager4chunk.cpp:62:59: error: 'new' of type 'curve::client::IOTracker' with extended alignment 64 [-Werror=aligned-new=]
   62 |     IOTracker* temp = new IOTracker(this, &mc_, scheduler_);

wuhongsong · 2023-09-08T08:54:52Z

@Cyber-SiKu

Cyber-SiKu · 2023-09-08T09:18:17Z

Most build procedure is now done. But one problem, I was only able to compile /curvefs_python/libcurvefs.a, not a .so lib using make build only=curvefs_python/curvefs (with appropriate linker options to curvefs_python/tmplib).

PS: The old build command (using bazel directly) is not working in the new docker build container. The error is:

This is related to the gcc version, please add --config=gcc7-later when compiling bazel

charlie0129 · 2023-09-13T08:43:31Z

cicheck

charlie0129 · 2023-09-13T09:32:06Z

Hi, @Cyber-SiKu the script now works fine for me. What do you think? Is there any problems?

Cyber-SiKu · 2023-09-14T02:26:32Z

good job!

Cyber-SiKu · 2023-09-14T02:39:05Z

Please add some output, how to compile, and screenshots of successful compilation.

Cyber-SiKu · 2023-09-14T02:39:10Z

cicheck

charlie0129 · 2023-09-14T06:37:28Z

Please add some output, how to compile, and screenshots of successful compilation.

How to compile:

install required dependencies, like docker, git, and etc. I use docker in the script to do subsequent compilation so no build toolchain is required on the host machine.
clone this branch and cd into it.
make tar dep=1 or make deb. dep=1 is required for the first run to build dependencies. It should take several minutes on a 64 core machine to do a full compile.
Build output should be in the current dir.

Screenshots:

Compiling

Successful compilation

Artifacts

Cyber-SiKu · 2023-09-14T02:40:54Z

thirdparties/etcdclient/Makefile

@@ -27,7 +38,7 @@ intall-go:

 install-etcdclient:
 	mkdir -p $(pwd)/tmp/gosrc/src/go.etcd.io
-	cd $(pwd)/tmp/gosrc/src/go.etcd.io && git clone --branch v3.4.0 https://gitee.com/mirrors/etcd
+	cd $(pwd)/tmp/gosrc/src/go.etcd.io && git clone --branch v3.4.0 --depth=1 https://gitee.com/mirrors/etcd


What does --depth=1 do?

It will only clone the most recent commit, rather than the entire history. Since we will certainly not use the commit history when we are building etcd, ignoring the commit history will make it faster to clone.

Cyber-SiKu · 2023-09-14T07:39:40Z

util/build.sh

+        -w /curve \
+        --user $(id -u ${USER}):$(id -g ${USER}) \
+        -v $(pwd):/curve \
+        -v ${HOME}:${HOME} \
+        -v /etc/passwd:/etc/passwd:ro \
+        -v /etc/group:/etc/group:ro \
+        -v /etc/shadow:/etc/shadow:ro \
+        -e BUILD_OPTS="${BUILD_OPTS}" \
+        --privileged \
+        opencurvedocker/curve-base:build-$g_os \
+        bash util/build_in_image.sh "$@"


Can the docker related stuff be organized and put in util/docker.sh? for reference only

Of course. I will add to docker.sh

Do you mean we just add the docker-related stuff to util/docker.sh without calling them? Or we add and call it from other scripts?

I have assumed the latter one. After some investigation, I think I will need to edit docker.sh to an extent that I think is too much to make it flexible enough to call in other scripts. Provided it already have some useful contents, maybe I will leave docker.sh as is.

Do you mean we just add the docker-related stuff to util/docker.sh without calling them? Or we add and call it from other scripts?

I have assumed the latter one. After some investigation, I think I will need to edit docker.sh to an extent that I think is too much to make it flexible enough to call in other scripts. Provided it already have some useful contents, maybe I will leave docker.sh as is.

the latter one.What you write now can also be used, but the latter one is more general and better.

Hi I organized common docker-related arguments into a separate file called docker_opts.sh. It should be easier to manage all docker things from one place now.

I only extracted common docker-related arguments, so other scripts can source them and make modifications freely. The reason why I did not use util/docker.sh is that if I edit it to be flexible engough to be used in other scripts, I am afraid to break other things that are already using this script.

Makefile

util/build.sh

util/package.sh

charlie0129 · 2023-09-25T03:50:05Z

@Cyber-SiKu hi, are there any further issues on this pr?

SeanHai · 2023-09-27T01:53:50Z

LGTM!

Cyber-SiKu · 2023-09-27T06:14:54Z

cicheck

Cyber-SiKu

LGTM

Cyber-SiKu · 2023-09-27T06:15:36Z

cicheck

Cyber-SiKu · 2023-09-28T02:50:16Z

cicheck

charlie0129 · 2023-09-28T10:54:47Z

@Cyber-SiKu Is there something in my code that is causing the CI to fail? a bit confused

SeanHai · 2023-10-11T01:36:33Z

cicheck

charlie0129 · 2023-10-11T02:08:03Z

cicheck

charlie0129 · 2023-10-11T06:00:07Z

cicheck

charlie0129 · 2023-10-11T07:04:43Z

cicheck

SeanHai · 2023-10-11T07:47:02Z

cicheck

SeanHai · 2023-10-11T08:20:03Z

cicheck

charlie0129 · 2023-10-12T16:27:59Z

cicheck

SeanHai · 2023-10-17T09:33:46Z

cicheck

SeanHai · 2023-10-17T09:35:00Z

cicheck

SeanHai · 2023-10-17T11:09:13Z

cicheck

Signed-off-by: Charlie Chiang <charlie_c_0129@outlook.com>

charlie0129 · 2023-10-19T01:51:46Z

cicheck

Cyber-SiKu reviewed Sep 14, 2023

View reviewed changes

SeanHai reviewed Sep 18, 2023

View reviewed changes

Makefile Outdated Show resolved Hide resolved

util/build.sh Show resolved Hide resolved

util/package.sh Outdated Show resolved Hide resolved

charlie0129 force-pushed the deprecate-mk-tar-deb branch from 57d4810 to e2df731 Compare September 18, 2023 08:43

SeanHai approved these changes Sep 27, 2023

View reviewed changes

Cyber-SiKu approved these changes Sep 27, 2023

View reviewed changes

charlie0129 force-pushed the deprecate-mk-tar-deb branch from 2d189c4 to 89959c0 Compare October 11, 2023 05:34

charlie0129 force-pushed the deprecate-mk-tar-deb branch 4 times, most recently from 2d189c4 to fb8cd73 Compare October 11, 2023 05:58

build: deprecate mk-deb.sh mk-tar.sh and use make targets instead

6cc3803

Signed-off-by: Charlie Chiang <charlie_c_0129@outlook.com>

charlie0129 force-pushed the deprecate-mk-tar-deb branch from fb8cd73 to 6cc3803 Compare October 19, 2023 01:50

SeanHai merged commit 7aa35ee into opencurve:master Oct 19, 2023
5 checks passed

build: deprecate mk-deb.sh mk-tar.sh and use make targets instead #2644

build: deprecate mk-deb.sh mk-tar.sh and use make targets instead #2644

Conversation

charlie0129 commented Jul 23, 2023 • edited

What problem does this PR solve?

What is changed and how it works?

Check List

charlie0129 commented Jul 23, 2023

wuhongsong commented Jul 26, 2023

Cyber-SiKu commented Jul 29, 2023

wuhongsong commented Aug 14, 2023

charlie0129 commented Aug 14, 2023

charlie0129 commented Aug 14, 2023

Cyber-SiKu commented Aug 22, 2023

charlie0129 commented Aug 22, 2023 • edited

wuhongsong commented Sep 8, 2023

Cyber-SiKu commented Sep 8, 2023

charlie0129 commented Sep 13, 2023

charlie0129 commented Sep 13, 2023

Cyber-SiKu commented Sep 14, 2023

Cyber-SiKu commented Sep 14, 2023

Cyber-SiKu commented Sep 14, 2023

charlie0129 commented Sep 14, 2023 • edited

Cyber-SiKu Sep 14, 2023

Choose a reason for hiding this comment

charlie0129 Sep 14, 2023 • edited

Choose a reason for hiding this comment

Cyber-SiKu Sep 14, 2023

Choose a reason for hiding this comment

charlie0129 Sep 14, 2023 • edited

Choose a reason for hiding this comment

charlie0129 Sep 14, 2023

Choose a reason for hiding this comment

Cyber-SiKu Sep 14, 2023 • edited

Choose a reason for hiding this comment

charlie0129 Sep 15, 2023

Choose a reason for hiding this comment

charlie0129 commented Sep 25, 2023 • edited

SeanHai commented Sep 27, 2023

Cyber-SiKu commented Sep 27, 2023

Cyber-SiKu left a comment

Choose a reason for hiding this comment

Cyber-SiKu commented Sep 27, 2023

Cyber-SiKu commented Sep 28, 2023

charlie0129 commented Sep 28, 2023

SeanHai commented Oct 11, 2023

charlie0129 commented Oct 11, 2023

charlie0129 commented Oct 11, 2023

charlie0129 commented Oct 11, 2023

SeanHai commented Oct 11, 2023

SeanHai commented Oct 11, 2023

charlie0129 commented Oct 12, 2023

SeanHai commented Oct 17, 2023

SeanHai commented Oct 17, 2023

SeanHai commented Oct 17, 2023

charlie0129 commented Oct 19, 2023

charlie0129 commented Jul 23, 2023 •

edited

charlie0129 commented Aug 22, 2023 •

edited

charlie0129 commented Sep 14, 2023 •

edited

charlie0129 Sep 14, 2023 •

edited

charlie0129 Sep 14, 2023 •

edited

Cyber-SiKu Sep 14, 2023 •

edited

charlie0129 commented Sep 25, 2023 •

edited