Am/chore/next tfhe wip #673
Conversation
|
@slab-ci cpu_fast_test |
IceTDrinker
force-pushed
the
am/chore/next_tfhe_wip
branch
from
d485c9e
to
10b49f6
Compare
|
@slab-ci cpu_fast_test |
Makefile
Outdated
| @@ -17,6 +17,10 @@ FAST_TESTS?=FALSE | |||
| FAST_BENCH?=FALSE | |||
| BENCH_OP_FLAVOR?=DEFAULT | |||
| NODE_VERSION=20 | |||
| TFHE_CURRENT_VERSION:=$(shell grep '^version[[:space:]]*=' tfhe/Cargo.toml | cut -d '=' -f 2 | xargs) | |||
There was a problem hiding this comment.
This could give the wrong result if a crate dependency is defined before the package version and its version is in new line
That's unlikely but still
There was a problem hiding this comment.
I agree, if you have a better proposal I take it :)
There was a problem hiding this comment.
Not sure if it exist, but inserting a utility which asserts that a stream is only one line and forwards it would be more secure (even if not perfect)
There was a problem hiding this comment.
though it looks like the Cargo.toml format does not allow new lines for { version = "..." } stuff
There was a problem hiding this comment.
I think you can have
[dependencies.dep_name]
version = "1.0"
There was a problem hiding this comment.
no reason to have dependencies before package metadata, so even though it's not perfect, it's much better
There was a problem hiding this comment.
There was a problem hiding this comment.
or maybe a light toml parser
There was a problem hiding this comment.
a light toml parser
That seems overkill to me
There was a problem hiding this comment.
got something a bit more robust
|
gotta love that the unambiguous spec for run (and only run) does not work |
IceTDrinker
force-pushed
the
am/chore/next_tfhe_wip
branch
from
10b49f6
to
c8d522e
Compare
|
@slab-ci cpu_fast_test |
IceTDrinker
force-pushed
the
am/chore/next_tfhe_wip
branch
from
c8d522e
to
f5eb85e
Compare
|
@slab-ci cpu_fast_test |
|
http status 502 Bad Gateway code not expected |
- adding the tfhe package as a dependency is currently causing issues with Cargo because of unified feature resolution it seems, it needs an additional version specifier to disambiguate which package we are referring to, an issue exists on their end but I don't think a fix is to be expected soon rust-lang/cargo#12891 - commiting this to main and then backporting the relevant pieces to 0.4.x
IceTDrinker
force-pushed
the
am/chore/next_tfhe_wip
branch
from
f5eb85e
to
b08f5c8
Compare
|
@slab-ci cpu_fast_test |
|
looks ok to me |
Approve if you think it can go for merging pretty please 😄 |
|
Pull Request has been approved 🎉 |
PR content/description
Various improvements to the build system to be able to use the crate as a dependency in a semver-trick setting, main change is a way to by pass the build.rs script which was erroring due to weird cargo conflicts on package names (and maybe the fact our build.rs is not that smart to begin with with path allocations and such, so being able to skip it entirely just makes things easier for us for the semver trick)
The added TFHE_SPEC in the Makefile is to make sure Cargo can disambiguate the fact we are talking about our package and not a dependency, see rust-lang/cargo#12891
Check-list: