Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Migrate CouchDB to rebar3 #4089

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Conversation

jiahuili430
Copy link
Contributor

@jiahuili430 jiahuili430 commented Jul 5, 2022

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 7 times, most recently from 3404c9b to 8c3ee2f Compare July 15, 2022 02:49
@jiahuili430 jiahuili430 marked this pull request as ready for review July 15, 2022 03:33
@jiahuili430 jiahuili430 changed the title Try using rebar3 Migrate CouchDB to rebar3 Jul 15, 2022
-define(TEMPDIR,
filename:join([?BUILDDIR(), "tmp", "tmp_data"])).

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

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.

@@ -11,7 +11,7 @@
% the License.

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

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
Contributor Author

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.

@@ -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
Contributor

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
Contributor Author

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.

@@ -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
Contributor

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
Contributor

nickva commented Jul 15, 2022

@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 jiahuili430 force-pushed the 82-rebar3 branch 11 times, most recently from 71492c5 to 4ff7ed2 Compare July 22, 2022 02:42
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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants