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.

Sign up for GitHub

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

Jump to bottom

appender: fix compilation 32-bit platforms like PowerPC #1675

Merged
merged 3 commits into from Oct 22, 2021
Merged

appender: fix compilation 32-bit platforms like PowerPC #1675

merged 3 commits into from Oct 22, 2021

Conversation

hawkw
Copy link
Member

@hawkw hawkw commented Oct 22, 2021

This branch is @dzvon's PR #1508, with the following changes:

  • Add a newtype wrapping the error counter to hide that it's internally
    an Arc<AtomicUsize>. This would allow us to make additional changes
    to the implementation without potentially causing breaking changes.

  • Use saturating arithmetic when incrementing the counter to avoid
    wrapping to 0 on overflows. This is more likely to be an issue on
    32-bit platforms.

This is a breaking change that will be released as part of
tracing-appender 0.2.

Closes #1508

Description from @dzvon's original PR:

Motivation

Currently, tracing-appender crate cannot be compiled on PowerPC
platform. Because

PowerPC and MIPS platforms with 32-bit pointers do not have
AtomicU64 or AtomicI64 types.

quote from std library docs.
(https://doc.rust-lang.org/std/sync/atomic/index.html#portability)

Solution

Change AtomicU64 to AtomicUsize.

Co-authored-by: Dezhi Wu wu543065657@163.com

dzvon and others added 2 commits October 22, 2021 10:47
@dzvon @hawkw
chore(appender): fix compile on PowerPC
ebab010 
Signed-off-by: Dezhi Wu <wu543065657@163.com>
@hawkw
newtype error counter, use saturating arithmetic
36b709f 
Signed-off-by: Eliza Weisman <eliza@buoyant.io>
@hawkw hawkw self-assigned this Oct 22, 2021
@hawkw hawkw requested a review from a team as a code owner October 22, 2021 18:12
davidbarsky
davidbarsky approved these changes Oct 22, 2021
View reviewed changes
Copy link
Member

@davidbarsky davidbarsky left a comment

Choose a reason for hiding this comment

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

lgtm!

@hawkw hawkw merged commit c91504d into master Oct 22, 2021
@hawkw hawkw deleted the fix-compile-on-ppc branch October 22, 2021 19:47
hawkw added a commit that referenced this pull request Oct 22, 2021
@hawkw @dzvon
appender: fix compilation 32-bit platforms like PowerPC (#1675)
3634847 
This branch is @dzvon's PR #1508, with the following changes:

* Add a newtype wrapping the error counter to hide that it's internally
  an `Arc<AtomicUsize>`. This would allow us to make additional changes
  to the implementation without potentially causing breaking changes.

* Use saturating arithmetic when incrementing the counter to avoid
  wrapping to 0 on overflows. This is more likely to be an issue on
  32-bit platforms.

This is a breaking change that will be released as part of 
`tracing-appender` 0.2.

Closes #1508

Description from @dzvon's original PR:

## Motivation

Currently, tracing-appender crate cannot be compiled on PowerPC
platform. Because

> PowerPC and MIPS platforms with 32-bit pointers do not have
> `AtomicU64` or `AtomicI64` types.

quote from std library docs.
(https://doc.rust-lang.org/std/sync/atomic/index.html#portability)

## Solution

Change `AtomicU64` to `AtomicUsize`.

Co-authored-by: Dezhi Wu <wu543065657@163.com>
@hawkw hawkw mentioned this pull request Oct 22, 2021
Merged
hawkw added a commit that referenced this pull request Oct 22, 2021
@hawkw @dzvon
appender: fix compilation 32-bit platforms like PowerPC (#1675)
6d261cc 
This branch is @dzvon's PR #1508, with the following changes:

* Add a newtype wrapping the error counter to hide that it's internally
  an `Arc<AtomicUsize>`. This would allow us to make additional changes
  to the implementation without potentially causing breaking changes.

* Use saturating arithmetic when incrementing the counter to avoid
  wrapping to 0 on overflows. This is more likely to be an issue on
  32-bit platforms.

This is a breaking change that will be released as part of 
`tracing-appender` 0.2.

Closes #1508

Description from @dzvon's original PR:

## Motivation

Currently, tracing-appender crate cannot be compiled on PowerPC
platform. Because

> PowerPC and MIPS platforms with 32-bit pointers do not have
> `AtomicU64` or `AtomicI64` types.

quote from std library docs.
(https://doc.rust-lang.org/std/sync/atomic/index.html#portability)

## Solution

Change `AtomicU64` to `AtomicUsize`.

Co-authored-by: Dezhi Wu <wu543065657@163.com>
hawkw added a commit that referenced this pull request Oct 22, 2021
@hawkw @dzvon
appender: fix compilation 32-bit platforms like PowerPC (#1675)
f461c5b 
This branch is @dzvon's PR #1508, with the following changes:

* Add a newtype wrapping the error counter to hide that it's internally
  an `Arc<AtomicUsize>`. This would allow us to make additional changes
  to the implementation without potentially causing breaking changes.

* Use saturating arithmetic when incrementing the counter to avoid
  wrapping to 0 on overflows. This is more likely to be an issue on
  32-bit platforms.

This is a breaking change that will be released as part of 
`tracing-appender` 0.2.

Closes #1508

Description from @dzvon's original PR:

## Motivation

Currently, tracing-appender crate cannot be compiled on PowerPC
platform. Because

> PowerPC and MIPS platforms with 32-bit pointers do not have
> `AtomicU64` or `AtomicI64` types.

quote from std library docs.
(https://doc.rust-lang.org/std/sync/atomic/index.html#portability)

## Solution

Change `AtomicU64` to `AtomicUsize`.

Co-authored-by: Dezhi Wu <wu543065657@163.com>
hawkw added a commit that referenced this pull request Oct 22, 2021
@hawkw
appender: prepare to release v0.2.0
a4a121c 
# 0.2.0 (October 22, 2021)

This breaking change release adds support for the new v0.3.x series of
`tracing-subscriber`. In addition, it resolves the security advisory for
the `chrono` crate, [RUSTSEC-2020-0159].

This release increases the minimum supported Rust version (MSRV) to
1.51.0.
### Breaking Changes

- Updated `tracing-subscriber` to v0.3.x ([#1677])
- Changed `NonBlocking::error_counter` to return an `ErrorCounter` type,
  rather than an `Arc<AtomicU64>` ([#1675])
  ### Changed

- Updated `tracing-subscriber` to v0.3.x ([#1677])
  ### Fixed

- **non-blocking**: Fixed compilation on 32-bit targets ([#1675])
- **rolling**: Replaced `chrono` dependency with `time` to resolve
  [RUSTSEC-2020-0159] ([#1652])
- **rolling**: Fixed an issue where `RollingFileAppender` would fail to
  print errors that occurred while flushing a previous logfile ([#1604])

Thanks to new contributors @dzvon and @zvkemp for contributing to this
release!

[RUSTSEC-2020-0159]: https://rustsec.org/advisories/RUSTSEC-2020-0159.html
[#1677]: #1677
[#1675]: #1675
[#1652]: #1675
[#1604]: #1604
@hawkw hawkw mentioned this pull request Oct 22, 2021
Merged
hawkw added a commit that referenced this pull request Oct 23, 2021
@hawkw
appender: prepare to release v0.2.0 (#1678)
aaa419b 
# 0.2.0 (October 22, 2021)

This breaking change release adds support for the new v0.3.x series of
`tracing-subscriber`. In addition, it resolves the security advisory for
the `chrono` crate, [RUSTSEC-2020-0159].

This release increases the minimum supported Rust version (MSRV) to
1.51.0.
### Breaking Changes

- Updated `tracing-subscriber` to v0.3.x ([#1677])
- Changed `NonBlocking::error_counter` to return an `ErrorCounter` type,
  rather than an `Arc<AtomicU64>` ([#1675])
  ### Changed

- Updated `tracing-subscriber` to v0.3.x ([#1677])
  ### Fixed

- **non-blocking**: Fixed compilation on 32-bit targets ([#1675])
- **rolling**: Replaced `chrono` dependency with `time` to resolve
  [RUSTSEC-2020-0159] ([#1652])
- **rolling**: Fixed an issue where `RollingFileAppender` would fail to
  print errors that occurred while flushing a previous logfile ([#1604])

Thanks to new contributors @dzvon and @zvkemp for contributing to this
release!

[RUSTSEC-2020-0159]: https://rustsec.org/advisories/RUSTSEC-2020-0159.html
[#1677]: #1677
[#1675]: #1675
[#1652]: #1675
[#1604]: #1604
@davidbarsky davidbarsky mentioned this pull request Sep 26, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@davidbarsky davidbarsky davidbarsky approved these changes

Assignees

@hawkw hawkw

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@hawkw @davidbarsky @dzvon