Skip to content

Autodetect spidermonkey version in ./configure#3932

Closed
frapa wants to merge 1030 commits intoapache:mainfrom
frapa:autodetect_spidermonkey_version
Closed

Autodetect spidermonkey version in ./configure#3932
frapa wants to merge 1030 commits intoapache:mainfrom
frapa:autodetect_spidermonkey_version

Conversation

@frapa
Copy link

@frapa frapa commented Feb 17, 2022

Hi, this is my first contribution here!

I have struggled with this point when building couchDB, because it's written nowhere in the documentation that you need to configure the version properly.

It seems that other people had the problem as well: https://couchdb.slack.com/archives/C49LEE7NW/p1632249155263400

The updated configure script automatically finds out the installed library version. It also introduces a dependency on sed, ldconfig and head which I hope are fine (will they work on MacOS?), and remove the default to version 1.8.5 which is old and not in most distributions anymore.

The argument was also removed from the CI scripts because it does not exist anymore.

iilyak and others added 30 commits November 17, 2020 05:39
This is useful so that read conflicts on the changes feed will
eventually be resolved. Without an end key specified a reader could end
up in an infinite conflict retry loop if there are clients updating
documents in the database.
This flips the view indexer to grab the database update_seq outside of
the update transaction. Previously we would cosntantly refresh the
db_seq value on every retry of the transactional loop.

We use a snapshot to get the update_seq so that we don't trigger
spurious read conflicts with any clients that might be updating the
database.
Add missing default headers to responses
New `elixir-suite` Makefile target is added. It runs a predefined set of elixir
integration tests.

The feature is controlled by two files:
- test/elixir/test/config/suite.elixir - contains list of all available tests
- test/elixir/test/config/skip.elixir - contains list of tests to skip

In order to update the `test/elixir/test/config/suite.elixir` when new tests
are added. The one would need to run the following command:

```
MIX_ENV=integration mix suite > test/elixir/test/config/suite.elixir
```
Add ability to control which Elixir integration tests to run
All endpoints but _session support gzip encoding and there's no practical reason for that.

This commit enables gzip decoding on compressed requests to _session.
1. The caching effort was a bust and has been removed. 2) chunkify can be done externally with a custom persist_fun.
* Simplify and speedup dev node startup

This patch introduces an escript that generates an Erlang .boot script
to start CouchDB using the in-place .beam files produced by the compile
phase of the build. This allows us to radically simplify the boot
process as Erlang computes the optimal order for loading the necessary
modules.

In addition to the simplification this approach offers a significant
speedup when working inside a container environment. In my test with
the stock .devcontainer it reduces startup time from about 75 seconds
down to under 5 seconds.

* Rename boot_node to monitor_parent

* Add formatting suggestions from python-black

Co-authored-by: Paul J. Davis <paul.joseph.davis@gmail.com>
* Add a development container config for VS Code

This creates a development environment with a FoundationDB server
and a CouchDB layer in two containers, sharing a network through
Docker Compose.

It uses the FDB image published to Docker Hub for the FDB container,
and downloads the FDB client packages from foundationdb.org to provide
the development headers and libraries. www.foundationdb.org is actually
not trusted in Debian Buster by default, so we have to download the
GeoTrust_Global_CA.pem. The following link has more details:

https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=962596

Once the Docker Compose setup is running, VS Code executes the
create_cluster_file.bash script to write down a cluster file containing
the IP address in the compose network where the FDB service can be
found. This cluster file is used both for a user-driven invocation of
`./dev/run`, as well as for unit tests that require a running CouchDB.

Additionally, I've got a small fix to the way we run explicitly specified
eunit tests:

* Run eunit tests for each app separately

The `eunit` target executes a for loop that appears intended to use a
separate invocation of rebar for each Erlang application's unit tests.
When running `make eunit` without any arguments this works correctly,
as the for loop processes the output of `ls src`. But if you specify a
comma-delimited list of applications the for loop will treat that as a
single argument and pass it down to rebar. This asymmetry is
surprising, but also seems to cause some issues with environment
variables not being inherited by the environment used to execute the
tests for the 2..N applications in the list. I didn't bother digging
into the rebar source code to figure out what was happening there.

This patch just parses the incoming comma-delimited list with `sed` to
create a whitespace-delimited list for the loop, so we get the same
behavior regardless of whether we are specifying applications
explicitly or not.
…g: chunked (apache#3360)

Transfer-Encoding: chunked causes the server to wait indefinitely, then issue a a 500 error when the client finally hangs up, when PUTing a multipart/related document + attachments.

This commit fixes that issue by adding proper handling for chunked multipart/related requests.
This allows users to verify that compaction processes are suspended
outside of any configured strict_window.
…s-main

Show process status in active_tasks
@frapa frapa force-pushed the autodetect_spidermonkey_version branch 2 times, most recently from 482eb3e to 76bcc6e Compare February 17, 2022 21:40
@kocolosk
Copy link
Member

Thanks! I've been wanting to do something like this ever since I refactored those CI config files and found myself littering them with --spidermonkey-version flags everywhere. How do you feel about still allowing the use of --spidermonkey-version, and just using the autodetected version as a default if the user doesn't specify one?

It seems we have an unrelated problem with Jenkins today, we'll investigate that separately.

Also, Trento? That takes me back. I spent a few weeks at ECT when I was a graduate student years ago. Lovely place :)

@frapa
Copy link
Author

frapa commented Feb 18, 2022

@kocolosk If you prefer keeping the options and using this as default, it's fine for me. In that case, should the CI script still be updated? I'll wait for more instructions and update the PR.

How can I find out if the CI works again?

Also, Trento? That takes me back. I spent a few weeks at ECT when I was a graduate student years ago. Lovely place :)

Cool! I come from very close to Trento, it's a really nice place. Looking forward to move back one day :-)

@kocolosk
Copy link
Member

CI is upgraded and back online courtesy of @nickva . I just re-submitted the job for this PR, but in order to test changes to Jenkinsfile.full I'll need to publish your changes in a specially-named branch in our repo. I can do that once we're confident about the changes here.

@kocolosk
Copy link
Member

In that case, should the CI script still be updated?

Yes, I like what you did there.

@frapa frapa force-pushed the autodetect_spidermonkey_version branch 3 times, most recently from 019faae to 8fa8abc Compare February 19, 2022 15:12
@frapa
Copy link
Author

frapa commented Feb 19, 2022

@kocolosk I reintroduced the --spidermonkey-version, added a check for empty SM_VSN and removed the environment block. Let me know if can do anything else.

@frapa frapa force-pushed the autodetect_spidermonkey_version branch from 8fa8abc to d8062c2 Compare February 19, 2022 15:15
@frapa
Copy link
Author

frapa commented Feb 22, 2022

@kocolosk Any news on this PR?

@kocolosk
Copy link
Member

@frapa I think this is looking good modulo a small typo or two in the error message. I pushed it to a specially-named jenkins-branch so we can test your changes to Jenkinsfile.full in CI:

https://ci-couchdb.apache.org/blue/organizations/jenkins/jenkins-cm1%2FFullPlatformMatrix/detail/jenkins-frapa-autodetect_spidermonkey_version/1/pipeline

Assuming all goes well there we can fix the typo and merge.

@kocolosk
Copy link
Member

Looks like this fails on macOS and FreeBSD. FreeBSD doesn't have the -p option to ldconfig, while macOS doesn't use it at all.

I'd be OK to make the ldconfig dance conditional on discovering Linux as the underlying platform if you want to go that route, otherwise maybe you can figure out a way to support this discovery for the other Unix-like systems?

@frapa frapa force-pushed the autodetect_spidermonkey_version branch 2 times, most recently from 4fe17b7 to b1e53d4 Compare February 23, 2022 21:06
@frapa
Copy link
Author

frapa commented Feb 23, 2022

Thanks for running the CI. I made it Linux-only because I have no access to freeBSD or macOS to test alternatives. Could you update the branch so we see if the CI passes everywhere?

@kocolosk
Copy link
Member

OK that's fine. Can you add the spidermonkey_vsn back into the Jenkinsfile for the macOS and FreeBSD builds and add the flag back in generateNativeStage? Otherwise macOS is going to fail because the 1.8.5 default is incorrect.

@frapa frapa force-pushed the autodetect_spidermonkey_version branch from b1e53d4 to fa54f11 Compare February 24, 2022 19:45
I have struggled with this point when building
couchDB, because it's written nowhere in the documentation
that you need to configure the version properly.

It seems that other people had the problem as well:
https://couchdb.slack.com/archives/C49LEE7NW/p1632249155263400

The updated configure script automatically finds
out the installed library version. It also introduces
a dependency on `sed`, `ldconfig` and `head`,
and remove the default to version 1.8.5 which is old
and not in most distributions anymore.

The argument was also removed from the CI scripts
because it does not exist anymore.
@frapa frapa force-pushed the autodetect_spidermonkey_version branch from fa54f11 to d915a57 Compare February 25, 2022 17:10
@frapa
Copy link
Author

frapa commented Mar 2, 2022

I think this PR should now be finished.

@big-r81
Copy link
Contributor

big-r81 commented Aug 25, 2022

Hi Francesco,

would you like to update and rebase your PR against the current main branch to easier review it?

@janl janl closed this Jul 29, 2023
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.