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

Add a --enable-dev-mode to enable everything #975

Closed
wants to merge 2 commits into from

Conversation

elichai
Copy link
Contributor

@elichai elichai commented Sep 15, 2021

When modifying the internals I usually want to run all the tests and make sure that I don't break any of the modules.
but I never remember exactly if module comes before or after the module name (i.e. --enable-module-ecdh vs --enable-ecdh-module) and it's a lot to enable all modules.

So I added a short simple flag --enable-dev-mode that enables everything.
(If we want to make it more configurable, we can set all default values before, then process dev-mode, and then process the rest of the flags without any action-if-not-given, so that further flags can override the default dev-mode, but because it's for development I think this is currently enough)

This also fixes the fact that we've assigned use_experimental but used enable_experimental.

configure.ac Outdated
[use_experimental=$enableval],
[use_experimental=no])
[enable_experimental=$enableval],
[enable_experimental=no])
Copy link
Contributor

@real-or-random real-or-random Sep 15, 2021

Choose a reason for hiding this comment

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

Hm I was wondering why this has been working then since ever. The reason is that enable_experimental is automatically assigned, and so we don't need to set this at all here...

See https://www.gnu.org/software/autoconf/manual/autoconf-2.70/html_node/Package-Options.html or https://autotools.io/autoconf/arguments.html .

And there's no reason to have sometimes use and sometimes enable. we may want to do some cleanup in our configure.ac. That does not need to happen in this PR but to keep it consistent for now, it may be cleaner to check for use_experimental below instead of making the change here.

Copy link
Contributor

@real-or-random real-or-random left a comment

Choose a reason for hiding this comment

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

Concept ACK

@sipa
Copy link
Contributor

sipa commented Sep 24, 2021

Concept ACK

Copy link
Contributor

@jonasnick jonasnick left a comment

Choose a reason for hiding this comment

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

concept ACK

(even more useful in secp-zkp)

@elichai elichai changed the title Add a --dev-mode to enable everything Add a --enable-dev-mode to enable everything Oct 4, 2021
@practicalswift
Copy link
Contributor

Concept ACK

Copy link
Contributor

@real-or-random real-or-random left a comment

Choose a reason for hiding this comment

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

This currently disables all the options if you pass --disable-dev-mode or equivalently --enable-dev-mode=no. But these should just be noops.
(https://autotools.io/autoconf/arguments.html is a nice document explaining how autoconf works here)

I think it would also be nice to first handle dev-mode and then other options, so you can enable all modules but then disable a particular one.

@real-or-random
Copy link
Contributor

@elichai I still think this very useful. Are you planning to work on this? If not, maybe someone could take over.

@real-or-random
Copy link
Contributor

Closing in favor of #1079.

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.

6 participants