Add target u32/u64 backend overrides

[pinkforest]

Related before-PR-doc (rendered): #453 (comment)

Thou shall ask: #449 (comment)

Thou shall be giveth:

cfg

Test cfg matrix / expectation

All selected backends are mutually exclusive.

fiat_backend

Auto select fiat_u32 / fiat_u64 based on cfg(target_pointer_width) - default fiat_u32 on non-32/64 bits

serial (default)

Auto select u32 / u64 based on cfg(target_pointer_width) - default u32 on non-32/64 bits

fiat_backend w/ cfg(curve25519_dalek_bits = "32")

Selects fiat_u32 regardless of target_pointer on fiat_backend

fiat_backend w/ cfg(curve25519_dalek_bits = "64")

Selects fiat_u64 regardless of target_pointer

serial (default) w/ cfg(curve25519_dalek_bits = "32")

Selects u32 (serial) regardless of target_pointer

serial (default) w/ cfg(curve25519_dalek_bits = "64")

Selects u64 (serial) regardless of target_pointer

[tarcieri]

FWIW I just pushed up a PR to switch to cfg-based backend selection: #455

[pinkforest]

thx - I'll just send you a .patch then or re-base here to #455 - my initial attempt here was overly naive I see / realise now 🤦‍♀️

[pinkforest]

Ok this should not conflict with the backend selection PR now and made it more simple to put the bits lottery into lib.rs ?

[tarcieri]

Really like this general direction.

One nit from #455 was whether the cfg names should be curve25519_dalek_* instead of just dalek_*

[tarcieri]

@pinkforest I'm missing how these are "overrides"... it seems like they would be mandatory?

[pinkforest]

@pinkforest I'm missing how these are "overrides"... it seems like they would be mandatory?

It's in the lib.rs with global cfg_attr that sets it if it's not set - for some reason criterion doesn't set it ? unless I'm mistaken.

[pinkforest]

Ok I moved the target default detection thing into via build.rs and now the defaults w/ overrides works proper 🎉

[pinkforest]

One nit from #455 was whether the cfg names should be curve25519_dalek_* instead of just dalek_*

Will be waiting direction on this and rename after if decided so 👍

[tarcieri]

I think curve25519_dalek_* is fine and nicely unambiguous, if a bit long.

But people will probably be pasting examples of the cfg directives from the docs anyway, so it doesn't really matter

[pinkforest]

Problem with that may be the ergonomics -

If there is dalek "family of crates" and and one wants to switch dalek_bits it for all the dalek inter-dependency crates this then requires remembering to add cfg flags for all of them separately ?

I mean if we have similar switch for other dalek crates then it might be feasible to just have dalek_* for some things -

.. or at least for dalek_bits considering backend is mostly curve25519_dalek_backend specific thing ?

.. and also considering that backend stuff may be split into different crates in the future ?

• Separating backends into their own crates? #287

... maybe both dalek_bits and curve25519_dalek_bits where curve25519_dalek_bits overrides dalek_bits within curve25519-dalek crate but otherwise it is dalek_bits on other dalek family of crates - if there is a use-case for it ?

[tarcieri]

The "family of crates" is a good point. Many users will use curve25519-dalek transitively through ed25519-dalek or x25519-dalek, where curve25519_* is an implementation detail.

[pinkforest]

Déjà vu 5742176 😅

commit 574217694e952ad5193f273a541c633ea32207c2
Date:   Fri Oct 11 17:10:57 2019 -0700

    Remove build.rs.
    
    This was more useful at the time when we were determining, e.g., optimal lookup
    table sizes and could regenerate them more easily, but it came at a massive
    complexity cost.  It also meant that we were unable to implement backend
    autoselection.  This commit removes the `build.rs` entirely.  In the future, a
    different `build.rs` could be added that auto-selects a backend, but it seems
    like the current default-u64 setup has been working fine.

commit 4dc8073330f63fb23040f7aa7dd0159aac7a9923 (tag: 1.2.3)

git diff 4dc8073330f63fb23040f7aa7dd0159aac7a9923 574217694e952ad5193f273a541c633ea32207c2 build.rs

[rozbb]

This looks great! I think we can safely just use curve25519_dalek_bits for now.

If we want to add an umbrella cfg later, we can do that as a non-breaking change. The converse isn't true: if we make a separately low-level library then dalek_bits will either become an umbrella (breaking the package-specific use for curve-), or remains specific (making the naming bad).

Are we good to merge?

[pinkforest]

Are we good to merge?

👍 I've re-named it to curve25519_dalek_bits - this should be good to merge now