Skip to content
This repository has been archived by the owner on Nov 1, 2021. It is now read-only.

Initial pass on API stability guarantees #1131

Merged
merged 1 commit into from
Jul 29, 2018
Merged

Initial pass on API stability guarantees #1131

merged 1 commit into from
Jul 29, 2018

Conversation

ddevault
Copy link
Contributor

@ddevault ddevault commented Jul 13, 2018

This introduces -DWLR_USE_UNSTABLE and adds information regarding the
stability status to all headers. I started with a conservative set of
headers to mark as stable:

  • types/wlr_matrix.h
  • util/edges.h
  • util/log.h
  • util/region.h
  • xcursor.h

Updates #1008

void wlr_matrix_multiply(float mat[static 9], const float a[static 9],
const float b[static 9]);

void wlr_matrix_transpose(float mat[static 9], const float a[static 9]);
Copy link
Member

Choose a reason for hiding this comment

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

mat ← aᵀ

@emersion
Copy link
Member

It may make sense to update wlroots.syms to drop the 0_0_0.

@ascent12
Copy link
Member

Breaking changes are announced by email and follow a 1-year deprecation schedule

This is far too long of a promise. In fact, I'm against time-based promises at all, especially when it's something still as cutting edge as this.
It should be release-based, and probably just follow basic semver like most other people do (once we actually have a versioned release).

util/log.h

I disagree with any of the wlr_log functions (excluding wlr_log_init) even being part of the API. We don't need to provide some generic logging framework; we just need to give people log messages when they want them.

It's because of those functions being exported that sway{lock,bg,idle,bar,msg} are massively over-linked; just because they were using some rather worthless logging functions.

@ddevault
Copy link
Contributor Author

This is far too long of a promise. In fact, I'm against time-based promises at all, especially when it's something still as cutting edge as this.
It should be release-based, and probably just follow basic semver like most other people do (once we actually have a versioned release).

I want to make proper stability promises, not some empty semver-based promise which'll just lead to releasing wlroots 138.19.2 in 6 months.

I disagree with any of the wlr_log functions (excluding wlr_log_init) even being part of the API. We don't need to provide some generic logging framework; we just need to give people log messages when they want them.

The important part is making it so that the application can process our logging and presumably incoproprate it with their own logging.

@Timidger
Copy link
Member

The important part is making it so that the application can process our logging and presumably incoproprate it with their own logging.

As a possible example of this, wlroots-rs uses the logging system of wlroots-rs to log additional information (e.g. the lifetimes of handles, when they are dropped, and if there are any outstanding handles when it's dropped. This can help debugging for both wlroots-rs authors and compositor authors because a crash from using a dropped handle can now be seen that it needs to be cleaned up in the destructor).

In Way Cooler we use our own logging system. It sounds like the real problem is the sway utility programs using the logging functionality. Perhaps there should just be a utility library for those programs that defines their own logging?

@Ongy
Copy link

Ongy commented Jul 16, 2018

Most logging systems I've seen, that are intended to be re-used, have a way to push down their own logging fuction, to e.g. redirect to syslog, from the current stderr approach.

Might be worth a consideration as well.

@ddevault
Copy link
Contributor Author

wlroots has that.

@ddevault
Copy link
Contributor Author

This can't sit dead in the water forever. We need to address these two points:

  • I want to use a 1 year deprecation lifecycle, and I don't want to use semver. I don't want to make half promises about stability when in reality we're just cranking up that major version number all of the time as an excuse not to be. Once we finish implementing a protocol, until a new protocol version comes out our implementation is unlikely to need changes and when the new protocol does come out, it'll be backwards compatible because we only mark stable protocols as stable in wlroots.
  • I want to include wlr_log. We're definitely going to keep using it internally - we need a logging system of some kind. What's the harm in exposing it? We need to at least expose a means for the user to provide their own callback instead of printf. I don't see why we can't expose the rest, too. It's not like we change it often, the only changes since December have been renaming things to use a WLR_ prefix.

@emersion
Copy link
Member

I want to use a 1 year deprecation lifecycle, and I don't want to use semver.

That's pretty dangerous and could be potentially harmful. Also bye bye cool renderer stuff, bye bye DRM planes.

I want to include wlr_log

At least don't mark it as stable.

@ddevault
Copy link
Contributor Author

Also bye bye cool renderer stuff, bye bye DRM planes.

We don't have to start labelling things as stable any time soon.

@ddevault
Copy link
Contributor Author

I want to include wlr_log

At least don't mark it as stable.

Whyyyyyyy

@emersion
Copy link
Member

We don't have to start labelling things as stable any time soon.

Ah. In this case the whole library won't ever be stable for years.

I want to include wlr_log

/shrug

@ddevault
Copy link
Contributor Author

In this case the whole library won't ever be stable for years.

Right. We can roll this out nice and slow.

@Ongy
Copy link

Ongy commented Jul 20, 2018

Can you add tablet-v2 header to the files addapted in this PR?

This introduces -DWLR_USE_UNSTABLE and adds information regarding the
stability status to all headers. I started with a conservative set of
headers to mark as stable:

- types/wlr_matrix.h
- util/edges.h
- util/log.h
- util/region.h
- xcursor.h
@ddevault
Copy link
Contributor Author

Aight, we've gotta get some movement on this, so I'm going to merge this once the build passes.

@ddevault ddevault merged commit 1c7957c into master Jul 29, 2018
@emersion emersion deleted the initial-stable branch August 10, 2018 07:55
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Development

Successfully merging this pull request may close these issues.

5 participants