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

Fix link to AVX2 docs #275

Merged

Conversation

Pratyush
Copy link
Contributor

@Pratyush Pratyush commented Aug 6, 2019

Currently the library doesn't compile on nightly when the avx2_backend is enabled; this PR fixes it by fixing the links to the relevant docs.

@hdevalence
Copy link
Contributor

Hi -- what nightly are you using? Recent nightlies changed the path root for doc(include) to refer to the crate root rather than the current file (cf. #269, rust-lang/rust#60938), and we updated the crate to match the current nightly behaviour. Is it still broken?

@Pratyush
Copy link
Contributor Author

Pratyush commented Aug 6, 2019

Hmm I am using the latest nightly. I tried changing the path to be "docs/avx2-notes.md" and "docs/parallel-formulas.md" and that didn't fix it:

➜ cargo +nightly --version
cargo 1.38.0-nightly (26092da33 2019-07-31)

➜ rustc +nightly --version
rustc 1.38.0-nightly (c4715198b 2019-08-05)

➜ cargo +nightly check --features avx2_backend
    Checking curve25519-dalek v1.2.2 (/tmp/curve25519-dalek)
error: couldn't read src/backend/vector/docs/parallel-formulas.md: No such file or directory (os error 2)
  --> src/backend/vector/mod.rs:22:19
   |
22 |     doc(include = "docs/parallel-formulas.md")
   |                   ^^^^^^^^^^^^^^^^^^^^^^^^^^^ couldn't read file

error: couldn't read src/backend/vector/avx2/docs/avx2-notes.md: No such file or directory (os error 2)
  --> src/backend/vector/avx2/mod.rs:22:19
   |
22 |     doc(include = "docs/avx2-notes.md")
   |                   ^^^^^^^^^^^^^^^^^^^^ couldn't read file

error: aborting due to 2 previous errors

error: Could not compile `curve25519-dalek`.

@Pratyush
Copy link
Contributor Author

Pratyush commented Aug 6, 2019

Also, your links indicate that the new root is relative to the current file, not the crate root, so I think my fix makes sense by making the paths have the additional ../?

@hdevalence
Copy link
Contributor

Uh oh, that's bad news, I thought it was already fixed.... let me dig into this

@hdevalence
Copy link
Contributor

Yep, you're absolutely right, sorry -- I think that 5c18bfb didn't fix it, because I just added one .., but I'm confused why the CI didn't pick it up.

@hdevalence hdevalence changed the base branch from master to develop August 6, 2019 23:29
@hdevalence
Copy link
Contributor

Hmm, the CI job did print an error, but it was reported as passing: https://travis-ci.org/dalek-cryptography/curve25519-dalek/jobs/566199718

@hdevalence
Copy link
Contributor

@Pratyush thanks for spotting this -- could you amend the commit to also update to doc(include = "../../../../docs/ifma-notes.md") in src/backend/vector/ifma/mod.rs, or alternately give me access to push that commit to your branch (the "allow edits from maintainers" option), whichever is easier?

@Pratyush Pratyush force-pushed the fix-docs-link-on-avx2 branch from 0baad83 to 912fe47 Compare August 6, 2019 23:54
@Pratyush
Copy link
Contributor Author

Pratyush commented Aug 6, 2019

Force-pushed; should be fixed now, I think?

@hdevalence
Copy link
Contributor

Great, I confirmed that running make doc-internal using cargo 1.38.0-nightly (26092da33 2019-07-31) / rustc 1.38.0-nightly (c4715198b 2019-08-05) produces the correct output.

@hdevalence hdevalence merged commit 4bbcc28 into dalek-cryptography:develop Aug 7, 2019
@Pratyush Pratyush deleted the fix-docs-link-on-avx2 branch August 7, 2019 00:18
@Pratyush
Copy link
Contributor Author

Pratyush commented Aug 7, 2019

Awesome =)

pinkforest added a commit to pinkforest/curve25519-dalek that referenced this pull request Jun 27, 2023
* Added items to changelog for 2.0 release

* Removed unnecessary uses of std in doctests

* Gated `Context` behind `digest`

* Fixed noncompiling doctest when only `digest` is enabled

* README feature flag list mostly done

* Copied changelog to readme

* Redid the malleability section in README

* Added CONTRIBUTING.md

* Bumped version number to 2.0.0-pre.0; small changes to README

* Updated changelog for dalek-cryptography#277

* Added pem feature description

Co-authored-by: pinkforest(she/her) <[email protected]>
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.

2 participants