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 windows compilation errors #52

Merged
merged 6 commits into from Sep 2, 2019
Merged

Fix windows compilation errors #52

merged 6 commits into from Sep 2, 2019

Conversation

rivy
Copy link
Contributor

@rivy rivy commented Jul 20, 2019

This PR set fixes currrent windows compilation errors (see AppVeyor build/test) and increases backwards window version compatibility.

- fixes "unresolved import `winapi::um::fileapi`" errors for windows builds
@rivy
Copy link
Contributor Author

@rivy rivy commented Jul 20, 2019

I can add an AppVeyor build status badge as well...

Build status

[![Build status](https://ci.appveyor.com/api/projects/status/q1of8msauxe0f8u6/branch/master?svg=true)](https://ci.appveyor.com/project/ogham/rust-ansi-term) 

But I'll need to know the AppVeyor assigned project tag (under https://ci.appveyor.com/project/ogham/rust-ansi-term \ Settings \ Badges; it'll replace the q1of8msauxe0f8u6 tag).

@rivy
Copy link
Contributor Author

@rivy rivy commented Aug 22, 2019

Ping @ogham . Have you had a chance to look this over?

@Finomnis
Copy link

@Finomnis Finomnis commented Aug 23, 2019

Yes, please merge this ASAP. The current version on crates.io is unusable on Windows.

@mainrs
Copy link

@mainrs mainrs commented Aug 23, 2019

I changed the needed winapi features too in my PR #55. What do the other changes do? I could compile the project with just adding fileapi.

@rivy
Copy link
Contributor Author

@rivy rivy commented Aug 24, 2019

Further explanation, by commit, (notably, also explained in the commit messages)...

  • [bcb89a2] fix ~ add missing winapi module feature ("fileapi")
    • fixes the baseline windows compilation issue
  • [bc76cd1] fix 'long_and_detailed' test for cfg!(windows)
    • fixes failing test (which I thought initially was a windows issue, but is a differing rustc version issue)

... both of the above are required to get passing AppVeyor CI builds

  • [093d0dd] fix minimum rust version
    • minimum rust version has increased based on changes (see commit notes for more explanation)
    • needed to define which versions to attempt with the AppVeyor builds
  • [ed28592] refactor ~ load winapi constants into the enable_ansi_support()
    • refactors the constants to use names in the style already within the code
  • [a4e42b7] change ~ increase windows early version compatibility
    • the prior windows change I PR'd created a dependency on Windows 8+, this pulls back and allows compilation for earlier Windows versions (7, ..., probably back to XP and Vista, although I haven't had any chance to test that far back)

I'm also going to be adding a commit which inserts an AppVeyor testing badge, so that failures are easier to see on the first page of the repo.

- customize expected pretty-print output for windows platforms
  - work-around for debug print format changes between versions (see <rust-lang/rust#62794>)
rivy added 3 commits Aug 24, 2019
- *serde* configuration attributes require MRV >= 1.18.0
  - introduced in commit 67f173d; "Optional serde serialization feature for colours and styles"
- `cargo test` now includes transitive dependencies which require MRV >= 1.28.0
- use of the `#[must_use]` configuration attribute causes compiler warnings
  - warnings appear for compilers >= 1.22.0 and < 1.28.0
  - introduced in commit afe5c93; "Mark Style::paint and Colour::paint as #[must_use]"
- use `CreateFileW` (usable in WinXP+) instead of `CreateFile2` (only usable in Win8+)
@mainrs
Copy link

@mainrs mainrs commented Aug 25, 2019

Makes more sense to base my PR on yours tbh. I'll just rebase accordingly after @ogham merges.

@Finomnis
Copy link

@Finomnis Finomnis commented Aug 29, 2019

Is @ogham even active any more?

@mainrs
Copy link

@mainrs mainrs commented Aug 29, 2019

I created an organization where I merged the PRs of this crate. They can both live side by side. Just specify the depndency as a git url. If this gets merged later on, you can switch back.

https://github.com/rust-ansi-term/rust-ansi-term/

@ogham ogham merged commit bad66a3 into ogham:master Sep 2, 2019
2 checks passed
@mainrs
Copy link

@mainrs mainrs commented Sep 2, 2019

Nice! I'll rebase my PR later on :)

@ogham
Copy link
Owner

@ogham ogham commented Sep 2, 2019

Thanks for this; sorry it took me so long to get round to. I hope the Windows CI fares better this time.

Released as v0.12.1.

@rivy rivy deleted the fix.win-build branch Sep 12, 2019
sholderbach pushed a commit to nushell/nu-ansi-term that referenced this issue Mar 14, 2022
Fix windows compilation errors
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.

None yet

4 participants