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

Bump bitflags to 0.7 #431

Closed
wants to merge 1 commit into from
Closed

Bump bitflags to 0.7 #431

wants to merge 1 commit into from

Conversation

nox
Copy link
Contributor

@nox nox commented Sep 27, 2016

No description provided.

@nox
Copy link
Contributor Author

nox commented Sep 30, 2016

Somehow had forgotten the actual bump... This is fixed now and Travis should be happy.

src/macros.rs Outdated
@@ -150,7 +150,7 @@ macro_rules! libc_bitflags {
// (non-pub) Entry rule.
(
$(#[$attr:meta])*
flags $BitFlags:ident: $T:ty {
pub flags $BitFlags:ident: $T:ty {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is a (or maybe even the) problem. The macro libc_bitflags already differentiated between pub and non-pub bitflags (as the comments indicate).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The macro libc_bitflags was broken because bitflags 0.4 did not support non-public flags.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh wait, misunderstood what you mean. Will fix this.

@nox
Copy link
Contributor Author

nox commented Oct 2, 2016

1.2.0 and 1.7.0 are not supported by bitflags anymore, can we stop supporting them here? No idea what these beta linking failures are though.

@fiveop
Copy link
Contributor

fiveop commented Oct 3, 2016

I would not mind dropping support, see #432. I also don't know what the problem on the i686 builds for beta (and nighlty) is.

@homu
Copy link
Contributor

homu commented Oct 21, 2016

☔ The latest upstream changes (presumably #415) made this pull request unmergeable. Please resolve the merge conflicts.

@posborne
Copy link
Member

@nox We have dropped support for versions prior to 1.7.0 but are still depending on 1.7.0 for the time being. I would consider dropping 1.7.0 but for now CI depends on it for testing non-host architectures.

@fiveop
Copy link
Contributor

fiveop commented Oct 28, 2016

I also opend a bitflags-issue asking whether the drop in support was intentional, though, I have not got an answer, yet.

@nox
Copy link
Contributor Author

nox commented Nov 7, 2016

1.7 is like an eon old, could we just merge this?

@posborne
Copy link
Member

posborne commented Nov 7, 2016

1.7 is like an eon old, could we just merge this?

No, we cannot simply break the current oldest supported version. To merge, we will need to do the additional work to increase the minimum version (including CI updates).

@nox
Copy link
Contributor Author

nox commented Nov 7, 2016

Yeah of course, that's obvious. Can it be done?

@posborne
Copy link
Member

posborne commented Nov 7, 2016

Yeah of course, that's obvious. Can it be done?

Yeah, I think we are open to it. The larger chunk of work that needs to be bitten off is further improving the CI infrastructure to allow moving between versions more easily, probably based on https://github.com/japaric/trust. Some discussion here: rust-embedded/wg#16

@homu
Copy link
Contributor

homu commented Nov 15, 2016

☔ The latest upstream changes (presumably #460) made this pull request unmergeable. Please resolve the merge conflicts.

@dimbleby
Copy link

Recent bitflags doesn't generate dead code warnings. So when this eventually gets merged, can remove this code. At which point you'll find that it has been masking things:

warning: constant item is never used: `WSTOPPED`, #[warn(dead_code)] on by default
  --> src/sys/wait.rs:40:1
   |
40 | const WSTOPPED: WaitPidFlag = WUNTRACED;
   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

@nox
Copy link
Contributor Author

nox commented Feb 14, 2017

Can we land this?

@homu
Copy link
Contributor

homu commented Feb 17, 2017

☔ The latest upstream changes (presumably #515) made this pull request unmergeable. Please resolve the merge conflicts.

@posborne
Copy link
Member

@nox I had forgotten about this and ended up merging #515 which was basically the same change submitted by @SimonSapin. Closing this but thanks for the contribution (and sorry for missing this when going back through the backlog).

@posborne posborne closed this Feb 17, 2017
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.

5 participants