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

winapi 0.3 overhaul #316

Closed
9 tasks done
retep998 opened this issue Aug 13, 2016 · 24 comments
Closed
9 tasks done

winapi 0.3 overhaul #316

retep998 opened this issue Aug 13, 2016 · 24 comments
Assignees

Comments

@retep998
Copy link
Owner

retep998 commented Aug 13, 2016

All progress is in the dev branch.

  • Remove all trait impls except Copy and Clone.
    • Update all macros to do so.
    • Get rid of any derives that are floating around, not behind a macro.
  • Make enums nothing more than integer constants with the enum type being an alias for u32.
  • Update all copyrights and license stuff. Update license and copyright header #308
  • For each header do the following:
    • Move it to the appropriate directory based on where it is located in the Windows SDK such as um or shared.
    • Create a feature for that header such as foo.
    • Add the appropriate pub mod foo; to that folder's module.
    • Add a cfg to that mod such as #[cfg(feature = "foo")].
    • Get rid of all :: and add imports to pull in any definitions it needs from other headers.
    • Add the feature to build.rs and specify its dependencies on other headers and also libraries.
  • For each -sys crate do the following:
    • Move the function definitions to the header module where it is actually defined.
    • Ensure that any header that has functions from that library specifies that dependency in build.rs.
  • Update all usage of UNION! to UNION2!. Once that is done, rename UNION2! to UNION!.
  • Inform everyone with an open PR that they'll need to update their PR for the new world order.
@retep998 retep998 self-assigned this Aug 13, 2016
@retep998 retep998 mentioned this issue Aug 14, 2016
3 tasks
@alexcrichton
Copy link
Contributor

One change I might also recommend is #![no_std] by default if you can get away with it, libc right now only does so because it was stable before #![no_std] was stable. Other than that though this sounds great to me!

Once the refactoring is done, perhaps some measurements could be done to ensure that issues like #218 are addressed as well? Seems like it'll definitely get smoothed over regardless.

@retep998
Copy link
Owner Author

retep998 commented Aug 14, 2016

@alexcrichton Unless drastic improvements are made in rustc to improve compile times, I don't think winapi will ever be able to compile quickly. Just winnt alone (needed for basic things like HANDLE) takes a couple seconds. This is already with all trait impls gone except Copy and Clone and enums replaced with mere integer constants, so there's no way to make it any faster aside from simply not including a lot of code. Still, it is faster than the old winapi.

$cargo build --features shared.winnt
   Compiling winapi v0.3.0-dev (file:///C:/msys64/home/Peter/winapi-rs)
    Finished debug [unoptimized + debuginfo] target(s) in 2.36 secs

According to -Ztime-passes item-types checking is the main offender taking up half the compile time. Perhaps there are some improvements that could be done there.

And yes, I have #![no_std] by default already for winapi 0.3.

@alexcrichton
Copy link
Contributor

Ah well 2s is much better than the 30 today! I may have also misunderstood the issue, though, in thinking that most of winapi would be turned off by default and you'd have to opt-in to enable portions of the API. Maybe though the default set is still quite large?

@retep998
Copy link
Owner Author

All of winapi (aside from the basic C types and macros) is feature gated and disabled by default in 0.3. It is just that winnt, which provides some definitions used pervasively, is rather large. winnt.rs is already 4 times bigger than the libc crate on Windows, and winnt.h is a whopping 30 times larger than libc.

@retep998
Copy link
Owner Author

retep998 commented Oct 2, 2016

Due to rust-lang/crates.io#7250 I will need to change the naming scheme for the features.

@DoumanAsh
Copy link
Contributor

Oh... it sounds grand and i'm glad that compile time will be improved.
I'll stay tuned for your progress here. Wanna re-write my winapi related crates.

Good job dealing with this awful winapi :)

retep998 pushed a commit that referenced this issue Jan 15, 2017
* implement avrt with regard to #316

* implements maintainer comments

* unsing EXTERN macro + formating
@schulzch
Copy link

Anything residing inside lib is currently broken, i.e., not accessible, because the kernel32-sys and similar crates no longer work with 0.3, or am I wrong?

@retep998
Copy link
Owner Author

@schulzch Code inside lib simply hasn't been moved to its new location yet inside src. When 0.3 comes out you'll be able to access all that functionality directly through winapi without all those sys crates.

@Susurrus
Copy link
Contributor

Re: only implementing Copy and Clone for structs, it would be useful if Debug could be generated, even if it is behind a feature gate. libc has that plan. I don't know if this is the right place to raise this issue, but since it's mentioned here I thought I'd comment.

@retep998
Copy link
Owner Author

@Susurrus The problem is I cannot trivially generate Debug for all structs. Some structs contain primitive types which are not Debug (such as large arrays or function pointers) and hence deriving Debug would fail for those types.

@Susurrus
Copy link
Contributor

What about just doing it for the easy structs then?

@retep998
Copy link
Owner Author

Then someone would have to go through and tag every single struct appropriately.

@austinwagner
Copy link
Contributor

I needed a few functions in advapi32 and ended up going overboard and working on every possible function in there as well as many of the supporting structures. For v0.3 is it correct that I need to move the function declarations into an .rs file that lines up with the header file it's found in (I've already done that with the structs and consts)? And should everything be put in the same order as they are in the header files? Is there a known good way to handle string constants given that &str can't just be passed directly into the Windows functions?

@retep998
Copy link
Owner Author

For v0.3 is it correct that I need to move the function declarations into an .rs file that lines up with the header file it's found in (I've already done that with the structs and consts)?

Yes.

And should everything be put in the same order as they are in the header files?

Yes.

Is there a known good way to handle string constants given that &str can't just be passed directly into the Windows functions?

Just use &str for now. The next time I bump the minimum Rust version there will hopefully be a good way to have UTF-16 string literals, perhaps with a procedural macro.

@Susurrus
Copy link
Contributor

Note that I already did some of advapi32 in rust-lang/cargo#438. I suggest you rebase your work off of that for merging.

@austinwagner
Copy link
Contributor

@retep998 Thanks for clarifying! It's unfortunate that the language doesn't have UTF-16 literals. I was close to putting them in using array syntax.

@Susurrus I'll be sure to do that. I don't want to make merging more painful than it needs to be!

@austinwagner
Copy link
Contributor

@Susurrus I've never actually dealt with PRs on GitHub. How do I take your PR against Peter's repo into my copy of the repo? I don't even care about maintaining my commits properly since a simple text merge is going to be messier than just comparing my additions completely by hand.

@Susurrus
Copy link
Contributor

  1. Add my repository as a remote remote add susurrus https://github.com/susurrus/winapi-rs
  2. Fetch all info from my repo: git fetch susurrus
  3. Create a new branch based on my PR: git checkout -b finish_advapi32 susurus/securitybaseapi

Now you have a version to build on. There are other ways to do this, like if you've already done all the code:

  1. Add my repository as a remote remote add susurrus https://github.com/susurrus/winapi-rs
  2. Fetch all info from my repo: git fetch susurrus
  3. When on your branch with your commits: git rebase susurrus/securitybaseapi

No go through and fix all conflicts following the directions that git status shows.

@Susurrus
Copy link
Contributor

Susurrus commented Jul 26, 2017

All code save one function has now been ported.

@austinwagner
Copy link
Contributor

As long as @Susurrus keeps telling me to rebase, I'll keep my PR up to date. I want Windows to be as easy for people to support with their libraries as possible (though that won't be fully possible until people stop using OpenSSL, which is a pain in the butt, especially in older versions of the crate).

@retep998
Copy link
Owner Author

All points have been covered. At this point winapi 0.3's overhaul is feature complete and people should start porting and testing against the git version to ensure everything works before I finally publish winapi 0.3.

@GabrielMajeri
Copy link
Contributor

@retep998 could you please push the updated 0.3 docs to https://retep998.github.io/doc/winapi/ ? It would help with porting to have the updated docs available.

@retep998
Copy link
Owner Author

That link will remain as the source of documentation for 0.2 (because it is spread across multiple crates and docs.rs does not provide search across multiple crates). docs.rs will be the source of documentation for 0.3, as soon as I publish winapi 0.3.

gentoo90 added a commit to gentoo90/winreg-rs that referenced this issue Nov 12, 2017
this should be merged when retep998/winapi-rs#316 is landed
gentoo90 added a commit to gentoo90/winreg-rs that referenced this issue Nov 13, 2017
this should be merged when retep998/winapi-rs#316 is landed
@retep998
Copy link
Owner Author

winapi 0.3 has been published.

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

No branches or pull requests

7 participants