Skip to content

Migrate CouchDB to rebar3#4089

Open
jiahuili430 wants to merge 1 commit into
apache:mainfrom
jiahuili430:82-rebar3
Open

Migrate CouchDB to rebar3#4089
jiahuili430 wants to merge 1 commit into
apache:mainfrom
jiahuili430:82-rebar3

Conversation

@jiahuili430
Copy link
Copy Markdown
Contributor

@jiahuili430 jiahuili430 commented Jul 5, 2022 •
edited
Loading

Overview

  1. Change src to apps and all symlinks: In order to use the
    local plugins rebar3 ic setup_eunit [-f], we need to use correct
    umbrella project structure.
    See Support local plugins with is_umbrella settings erlang/rebar3#2729

  2. Use configure script instead of rebar.config.script to download
    dependencies: make eunit will get dependency cycle error if using
    rebar.config.script

  3. rebar3 has no options such as skip_deps or -r, so they were removed.

Testing recommendations

./configure --spidermonkey-version 60
make
./dev/run --admin=adm:pass -n 1

make dist
make check
make release

Related Issues or Pull Requests

Checklist

  • Code is written and works correctly
  • Changes are covered by tests
  • Any new configurable parameters are documented in rel/overlay/etc/default.ini
  • A PR for documentation changes has been made in https://github.com/apache/couchdb-documentation

@jiahuili430 jiahuili430 force-pushed the 82-rebar3 branch 30 times, most recently from 754abb6 to 181ab8a Compare July 8, 2022 16:19
@jiahuili430 jiahuili430 force-pushed the 82-rebar3 branch 14 times, most recently from fa438c6 to dc73196 Compare July 13, 2022 14:39
iilyak
iilyak reviewed Jul 13, 2022
View reviewed changes
Comment thread build-aux/couchdb-build-release.sh Outdated
for repo in *; do
cd ${repo}
if [ -d ".git" ]; then
mkdir -p ../../${REL_DIR}/src/${repo}
mkdir -p ../../../../${REL_DIR}/apps/${repo}
Copy link
Copy Markdown
Contributor

@iilyak iilyak Jul 13, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To improve readability let's set a variable and use that instead.

SELF_DIR="$( cd "$( dirname "${BASH_SOURCE[0]}" )" && pwd )"
COUCHDB_ROOT="${SELF_DIR}/.."

You would need to update #!/bin/sh -e in the begining of the script to use bash in order to make it work.

@jiahuili430 jiahuili430 force-pushed the 82-rebar3 branch 8 times, most recently from b5e2e5a to 21828be Compare July 14, 2022 13:21
iilyak
iilyak reviewed Jul 15, 2022
View reviewed changes
Comment thread apps/couch/include/couch_eunit.hrl
filename:join([?BUILDDIR(), "tmp", "tmp_data"])).

-define(APPDIR, filename:dirname(element(2, file:get_cwd()))).
-define(APPDIR, element(2, file:get_cwd())).
Copy link
Copy Markdown
Contributor

@iilyak iilyak Jul 15, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why this change? I think it forces you to update all places where ?ABS_PATH macro is used.

iilyak
iilyak reviewed Jul 15, 2022
View reviewed changes
Comment thread rel/reltool.config
@@ -11,7 +11,7 @@
% the License.

{sys, [
{lib_dirs, ["../src"]},
Copy link
Copy Markdown
Contributor

@iilyak iilyak Jul 15, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we still need this file. I thought we are switching to relx?

Copy link
Copy Markdown
Contributor Author

@jiahuili430 jiahuili430 Jul 15, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because ./dev/run is using reltool. I'll look for a solution later, thanks for the review.

iilyak
iilyak reviewed Jul 15, 2022
View reviewed changes
Comment thread rel/overlay/etc/default.ini
@@ -14,7 +14,7 @@ view_index_dir = {{view_index_dir}}
; The actual limit may be slightly lower depending on how
; many schedulers you have as the allowance is divided evenly
; among them.
;max_dbs_open = 500
max_dbs_open = 500
Copy link
Copy Markdown
Contributor

@iilyak iilyak Jul 15, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seem like an unrelated change

Copy link
Copy Markdown
Contributor Author

@jiahuili430 jiahuili430 Jul 15, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

APPDIR and this one are used to pass make eunit.

nickva
nickva reviewed Jul 15, 2022
View reviewed changes
Comment thread rel/overlay/etc/default.ini
@@ -14,7 +14,7 @@ view_index_dir = {{view_index_dir}}
; The actual limit may be slightly lower depending on how
; many schedulers you have as the allowance is divided evenly
; among them.
;max_dbs_open = 500
max_dbs_open = 500
Copy link
Copy Markdown
Contributor

@nickva nickva Jul 15, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wonder why we had to uncomment it?

@nickva
Copy link
Copy Markdown
Contributor

nickva commented Jul 15, 2022 •
edited
Loading

@jiahuili430 quite impressive, well done!

I wonder if it would make it easier to review if we could extract some the changes into a preliminary PR so to speak. Basically any fixups like whitespace changes, app dependencies, etc which could make sense even without switching to rebar3.

Some examples could be:

  • app dependency updates like adding the couch_tests / config dependencies to *.app.src files
  • Updating descriptions in app.src files
  • removing crypto from couch_epi app.src.script
  • some whitespace changes to httpd_global_handlers
  • test_util:stop_applications([config, couch_log]).
  • application:ensure_started(couch_epi).

So then with those changes out of the way, and everything working as is, we'd have a second PR more focused on just switching from src to apps. What do you think?

Also, it might help to have a description of what the ic plugin does and why we need it.

@jiahuili430
Migrate CouchDB to rebar3
26d44a0 
1. Change `src` to `apps` and all symlinks: In order to use the
local plugins `rebar3 ic setup_eunit [-f]`, we need to use correct
umbrella project structure. See erlang/rebar3#2729

2. Use `configure` script instead of `rebar.config.script` to download
dependencies: `make eunit` will get dependency cycle error if using
`rebar.config.script`

3. rebar3 has no options such as `skip_deps` or `-r`, so they were removed.
@iilyak iilyak mentioned this pull request Jul 29, 2022
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@nickva nickva nickva left review comments

@iilyak iilyak iilyak left review comments

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@jiahuili430 @nickva @iilyak