Fix windows compilation and testing #62

rivy · 2020-01-22T17:15:27Z

adds GitHub-Actions CI workflow (includes arm-linux, x86-linux, macos, and windows builds/testing)
- automatic deployment to Github Releases when commit is version tagged (see example at https://github.com/rivy/rs.dust/releases/tag/v0.4.9; note: fully automatically generated)
improves path manipulation portability
builds / tests successfully locally and on all CI hosts
wip / needs further investigation
- feasability of (currently disabled) symlink tests on "windows"
- investigate and/or update tests to handle minor size variations in testing output for different "windows" hosts
  - some one byte differences between local and CI host file sizes; ? bug or expected host variation
- cargo tarpaulin inability to handle test coverage (use alternate framework?)

bootandy · 2020-01-22T23:08:42Z

src/utils/mod.rs

-        || parent == "/"
+    let path_parent = std::path::Path::new(parent);
+    let path_child = std::path::Path::new(child);
+    (path_child.starts_with(path_parent) && !path_parent.starts_with(path_child))


I've been staring at this for sometime and I think it is good.

Can you add a test for this case as it was bothering me:

is_a_parent_of('/usr/folder', '/usr/folder_not_a_child') == False

?
@rivy

I tried to find a portable std::path-based method to determine parent-child relation and couldn't find anything. This was what I finally came up with as a possible design. But, yep, it's a bit twisty and I mentally bounced the code around for a while as well. I had to add a few test cases just to help prove it to myself. I'll happily add any extra test cases.

Done, and it passes (unlike std::String::starts_with(), std::path::Path.starts_with() says that it "only considers whole path components to match" which is why this works).

I'll add the commit later tonight or tomorrow.

bootandy · 2020-01-22T23:15:31Z

"windows" hosts some one byte differences between local and CI host file sizes; ? bug or expected host variation

We also have a similar problem on linux if the filesystem is a different type we will see different numbers. #60. I'm not sure what the best way round this is. I'm considering running it once to see what the value should be and then dynamically generating the tests. Or we could add a statement like: pass if test=A or test=B.

bootandy · 2020-01-22T23:16:19Z

Overall It looks good, I want to play with this branch myself for a bit but I think I'd be happy to merge it.

bootandy · 2020-01-22T23:16:49Z

Although I am going to be offline for the next 10 days, so apologies in advance for going quiet.

rivy · 2020-01-23T01:59:37Z

No worries on the timing.

The testing is breaking (solely based on sizes) on my local windows wsl ubuntu as well. I'll have a look at it over the next week or two and see if I can come up with anything that would make testing more robust.

I also cut some CI code that builds/deploys a deb package. It might satisfy issue #7. I'll add it back in as a separate commit.

I also don't see the GitHub-Actions CI building on your repo side. I'm not quite sure how to get it enabled, but I'll google around if it's not obvious from your end.

rivy · 2020-01-23T02:16:41Z

I just looked through my repository settings and it looks like you'll need to enable Actions via https://github.com/bootandy/dust/settings/actions for the CICD workflow to run for the PR.

I have "Enable local and third party Actions for this repository" selected. I think that's the setting needed since the CI here pulls in actions to setup rust and cross-compile.

rivy · 2020-01-23T02:23:13Z

Just an additional note here to prompt myself ...

I'm seeing "Did not have permissions for all directories" (though I surely have permissions for all searched directories) during execution on windows platforms. I'll investigate further.

bootandy · 2020-01-23T10:13:11Z

I may be stating the obvious here but:

"Did not have permissions for all directories"

This message is printed to stderr if you don't have permission on 1 or more files or directories which dust encounters.

rivy · 2020-01-23T17:21:23Z

I may be stating the obvious here but:

"Did not have permissions for all directories"

This message is printed to stderr if you don't have permission on 1 or more files or directories which dust encounters.

I found the problem. winapi_util::Handle::from_path() was being used to enumerate directory entries, but the handles returned for directories are useless.

"If you use from_path() on a directory, then subsequent queries using that handle will fail."

ref: "Struct winapi_util::Handle" from https://docs.rs/crate/winapi-util

winapi_util::Handle::from_path_any() needs to be used when the path might be a file or directory.

The bug was fixed with one change at the first use. But now I'm looking at the second use, trying to figure out if a directory could ever be an argument. I would think that from_path_any() should be used in all cases, but I'm not sure if there's a performance penalty.

rivy · 2020-01-28T19:45:40Z

I fixed/updated and polished a few things based upon this discussion.

Additionally, I've also created a commit that refactors the code to use PathBuf instead of String. I think it simplifies some things, making portable path handling easier, and avoids some of the conversion churn between strings and paths.

Here's the commit (based on top of this branch) ... rivy/rs.dust@add.gha...rivy:rf.pathbuf.

If you think the change is useful, I'd like to add it to this PR. But I can open it in a new PR if, again, you'd like the change but prefer a new / separate PR.

It's just a start and I'm sure that some of the remaining conversions between paths and strings can be avoided and overall portable path handling improved with some more study and development.

…erage

…ion'

…emporarily disable tests which vary by 'windows' hosts'

…isable two symlink tests which may not be possible on 'windows''

…t be a directory .# [why] "If you use `from_path()` on a directory, then subsequent queries using that handle will fail." Instead, use `from_path_any()` which supports opening files or directories. ref: "Struct winapi_util::Handle" from <https://docs.rs/crate/winapi-util>

…f path manipulation'

…ccurate descriptor)

rivy mentioned this pull request Jan 22, 2020

Commit bdc3d40 breaks windows builds #56

Closed

bootandy reviewed Jan 22, 2020

View reviewed changes

rivy force-pushed the add.gha branch from 824abb4 to d83c668 Compare January 26, 2020 04:56

rivy added 12 commits January 29, 2020 18:36

Maint/CI ~ add GitHub-Actions CI (aka GHA)

1851013

Fix ~ improve portability of path manipulation

7c34389

Tests ~ fix windows testing

416ad51

Tests ~ disable two symlink tests which may not be possible on 'windows'

684994e

Tests ~ temporarily disable tests which vary by 'windows' hosts

5ac1688

Maint/CI ~ temporarily disable non-working cargo tarpaulin test cov…

0898ee6

…erage

Tests ~ add test case to 'Fix ~ improve portability of path manipulat…

26ef8c3

…ion'

Tests ~ simplify by ignoring instead of skipping tests for 'Tests ~ t…

affafcc

…emporarily disable tests which vary by 'windows' hosts'

Tests ~ simplify by ignoring instead of skipping tests for 'Tests ~ d…

1af6e1f

…isable two symlink tests which may not be possible on 'windows''

Tests ~ improve portability of tests for 'Fix ~ improve portability o…

e126a01

…f path manipulation'

refactor ~ strip_trailing_curdirs() -> normalize_path() (a more a…

36ebb1b

…ccurate descriptor)

rivy force-pushed the add.gha branch from 7791edd to 36ebb1b Compare January 30, 2020 00:42

bootandy merged commit d2b959f into bootandy:master Feb 3, 2020

rivy deleted the add.gha branch February 3, 2020 23:01

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix windows compilation and testing #62

Fix windows compilation and testing #62

rivy commented Jan 22, 2020

bootandy Jan 22, 2020

rivy Jan 23, 2020

rivy Jan 23, 2020

bootandy commented Jan 22, 2020

bootandy commented Jan 22, 2020

bootandy commented Jan 22, 2020

rivy commented Jan 23, 2020

rivy commented Jan 23, 2020 •

edited

Loading

rivy commented Jan 23, 2020

bootandy commented Jan 23, 2020

rivy commented Jan 23, 2020

rivy commented Jan 28, 2020 •

edited

Loading

Fix windows compilation and testing #62

Fix windows compilation and testing #62

Conversation

rivy commented Jan 22, 2020

bootandy Jan 22, 2020

Choose a reason for hiding this comment

rivy Jan 23, 2020

Choose a reason for hiding this comment

rivy Jan 23, 2020

Choose a reason for hiding this comment

bootandy commented Jan 22, 2020

bootandy commented Jan 22, 2020

bootandy commented Jan 22, 2020

rivy commented Jan 23, 2020

rivy commented Jan 23, 2020 • edited Loading

rivy commented Jan 23, 2020

bootandy commented Jan 23, 2020

rivy commented Jan 23, 2020

rivy commented Jan 28, 2020 • edited Loading

rivy commented Jan 23, 2020 •

edited

Loading

rivy commented Jan 28, 2020 •

edited

Loading