tikv / raft-engine

A persistent storage engine for Multi-Raft log
Apache License 2.0
564 stars 88 forks source link

build on windows system. #322

Closed MichaelScofield closed 1 year ago

MichaelScofield commented 1 year ago

Let cargo build pass under windows system:

  1. use windows os specific symlink API in fork.rs
  2. use different LogFd implementations in each os. Guarded (or choosen) by feature flags
    • for non-linux or non-unix system, use the plain std::fs APIs instead
    • for linux and unix system, still use the nix raw bindings
MichaelScofield commented 1 year ago

@tabokie PTAL

MichaelScofield commented 1 year ago

@tabokie PTAL. Can you approve the workflow to run so I can debug the windows build in CI?

MichaelScofield commented 1 year ago

@tabokie can you make this PR always run workflow? Need another approval again. Debugging github workflow is a pain, especially for os = windows. I can see more commits to debug it in the future.

tabokie commented 1 year ago

I don't have the permission to do that. It's weird though because my fork repo runs the workflow just fine: https://github.com/tabokie/raft-engine/actions, I wonder why your fork repo ignores the workflow file.

tabokie commented 1 year ago

@MichaelScofield You can work around this by merging another small PR first. E.g. Open another PR to update clap manually (https://github.com/tikv/raft-engine/pull/306)

MichaelScofield commented 1 year ago

@tabokie Having trouble with building pingcap grpc in windows os: https://github.com/tikv/raft-engine/actions/runs/5493202302/jobs/10013725416 . Any ideas? Thx!

tabokie commented 1 year ago

It can build now, you just need to fix the tests.

MichaelScofield commented 1 year ago

Running into infamous "access denied (os error 5)" error in windows. Convert this PR to draft first, to reduce github notification noise.

codecov[bot] commented 1 year ago

Codecov Report

Patch coverage: 95.34% and no project coverage change.

Comparison is base (58041ad) 97.98% compared to head (94a96db) 97.99%.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #322 +/- ## ======================================= Coverage 97.98% 97.99% ======================================= Files 31 33 +2 Lines 12329 12388 +59 ======================================= + Hits 12081 12140 +59 Misses 248 248 ``` | [Files Changed](https://app.codecov.io/gh/tikv/raft-engine/pull/322?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=tikv) | Coverage Δ | | |---|---|---| | [src/env/mod.rs](https://app.codecov.io/gh/tikv/raft-engine/pull/322?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=tikv#diff-c3JjL2Vudi9tb2QucnM=) | `85.00% <ø> (ø)` | | | [src/file\_pipe\_log/pipe.rs](https://app.codecov.io/gh/tikv/raft-engine/pull/322?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=tikv#diff-c3JjL2ZpbGVfcGlwZV9sb2cvcGlwZS5ycw==) | `98.10% <ø> (ø)` | | | [src/fork.rs](https://app.codecov.io/gh/tikv/raft-engine/pull/322?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=tikv#diff-c3JjL2ZvcmsucnM=) | `100.00% <ø> (ø)` | | | [src/lib.rs](https://app.codecov.io/gh/tikv/raft-engine/pull/322?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=tikv#diff-c3JjL2xpYi5ycw==) | `100.00% <ø> (ø)` | | | [src/env/log\_fd/unix.rs](https://app.codecov.io/gh/tikv/raft-engine/pull/322?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=tikv#diff-c3JjL2Vudi9sb2dfZmQvdW5peC5ycw==) | `91.15% <91.15%> (ø)` | | | [src/env/default.rs](https://app.codecov.io/gh/tikv/raft-engine/pull/322?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=tikv#diff-c3JjL2Vudi9kZWZhdWx0LnJz) | `93.75% <100.00%> (+1.60%)` | :arrow_up: | | [src/env/log\_fd/plain.rs](https://app.codecov.io/gh/tikv/raft-engine/pull/322?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=tikv#diff-c3JjL2Vudi9sb2dfZmQvcGxhaW4ucnM=) | `100.00% <100.00%> (ø)` | | | [src/swappy\_allocator.rs](https://app.codecov.io/gh/tikv/raft-engine/pull/322?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=tikv#diff-c3JjL3N3YXBweV9hbGxvY2F0b3IucnM=) | `99.34% <100.00%> (+<0.01%)` | :arrow_up: | | [tests/failpoints/test\_engine.rs](https://app.codecov.io/gh/tikv/raft-engine/pull/322?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=tikv#diff-dGVzdHMvZmFpbHBvaW50cy90ZXN0X2VuZ2luZS5ycw==) | `99.90% <100.00%> (ø)` | | | [tests/failpoints/test\_io\_error.rs](https://app.codecov.io/gh/tikv/raft-engine/pull/322?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=tikv#diff-dGVzdHMvZmFpbHBvaW50cy90ZXN0X2lvX2Vycm9yLnJz) | `100.00% <100.00%> (ø)` | |

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

MichaelScofield commented 1 year ago

@tabokie PTAL. The tests are all passed (at least in my repo's action, see here). Let's approve the workflow in this PR to see how it goes!

tabokie commented 1 year ago

@MichaelScofield #325 is merged. Feel free to ask questions. https://github.com/tikv/raft-engine/pull/322#discussion_r1265127742 might be too much work for this PR, you can follow up in another one or let me handle it later.

MichaelScofield commented 1 year ago

@tabokie Sorry for the late reply, busy week.

I actually considered using the raw Windows API earlier. However, skimmed through the "windows" crate and the "official win32 API docs", I find it's not a quick and easy task. It requires detailed understanding of Windows filesystem to do it well, which I'm currently lacking of, nor do I have the time to study it. Besides, the windows binding APIs are all "unsafe" to call in rust. So I decide to switch back to the good old std fs, make it right first.

I'll make the features worked in the new test matrix this weekend.

MichaelScofield commented 1 year ago

@tabokie tests are ok now, PTAL

MichaelScofield commented 1 year ago

@tabokie PTAL

Now the "test_matrix" is only used in coverage job, and only runs under ubuntu.

MichaelScofield commented 1 year ago

@tabokie PTAL

tabokie commented 1 year ago

GitHub action space is not enough.. I'll take a look tomorrow.