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

[WIP] Continuous integration using Github Actions #4

Merged
merged 12 commits into from
May 3, 2021

Conversation

katyo
Copy link
Contributor

@katyo katyo commented Mar 25, 2021

Implemented tasks:

  • Check code formatting rules
  • Check code quality using clippy
  • Generate bindings for supported platforms (stable Rust)
  • Check building on all supported platforms (stable Rust)
  • Check building on different Rust channels (stable/beta/nightly, Linux x86_64)
  • Run tests on native Linux platforms (x86_64 and i686)
  • Create pull-requests for updating generated bindings
  • Publish crates when pushing version tag

All tasks runs on ubuntu 20.04.
To get publishing working you need set CRATES_TOKEN secret.

Currently supported prebuilt bindings:

  • Linux x86
  • Linux x86_64
  • Linux arm
  • Linux aarch64

Blocked by drm-rs#74

@katyo katyo force-pushed the ci branch 5 times, most recently from fdec357 to ef0fe40 Compare March 28, 2021 17:50
Copy link
Contributor

@MarijnS95 MarijnS95 left a comment

Choose a reason for hiding this comment

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

Now that Smithay/drm-rs#74 is merged this is unblocked, right? Can it be rebased and should the drm-rs branch be changed to develop or should this wait until 0.4 is out?

Should this be merged to master or the develop branch? What's the difference, given that both are working branches that have yet to be released?

@katyo and some commits based on Smithay/drm-rs#81 if you don't mind.

.github/workflows/ci.yml Outdated Show resolved Hide resolved
.github/workflows/ci.yml Outdated Show resolved Hide resolved
.github/workflows/ci.yml Show resolved Hide resolved
.github/workflows/ci.yml Outdated Show resolved Hide resolved
Cargo.toml Outdated Show resolved Hide resolved
Cargo.toml Outdated Show resolved Hide resolved
gbm-sys/Cargo.toml Outdated Show resolved Hide resolved
gbm-sys/build.rs Show resolved Hide resolved
src/buffer_object.rs Outdated Show resolved Hide resolved
@katyo
Copy link
Contributor Author

katyo commented Apr 14, 2021

@MarijnS95 @Drakulix I plans to rebase (and rewrite) this PR on develop branch after merging #80, #81 and #82 to drm-rs.

@MarijnS95
Copy link
Contributor

@katyo Sounds good, unfortunately all those PRs are blocked by nightly failing to compile clang-sys, should be fixed by rust-lang/rust#84130.

When you do, would you mind running the example in rustdoc through rustfmt, and then copying it back to the README? Thanks!

@MarijnS95
Copy link
Contributor

@katyo And they're all in now since nightly has been fixed :)

@katyo
Copy link
Contributor Author

katyo commented Apr 18, 2021

Seems I need relaunch this pr from the scratch, because a lot of changes happened in master.
@Drakulix Should I rebase it to develop branch instead of master?

@Drakulix
Copy link
Member

Seems I need relaunch this pr from the scratch, because a lot of changes happened in master.
@Drakulix Should I rebase it to develop branch instead of master?

Yeah sorry, the new release of drm-rs, usage of drm-fourcc and some general updates (including adding some new gbm-functions) produced a lot of changes.

Please target develop. I'll plan to merge that into master again eventually once new functionalities are all properly tested, but that may take some more time.

@katyo katyo changed the base branch from master to develop April 19, 2021 06:23
@katyo katyo force-pushed the ci branch 3 times, most recently from 947555b to 419d053 Compare April 19, 2021 09:48
@katyo
Copy link
Contributor Author

katyo commented Apr 20, 2021

Seems something went wrong on 32-bit targets.

@katyo
Copy link
Contributor Author

katyo commented Apr 21, 2021

@Drakulix All tests passed for now except minimum versions check.

@katyo katyo force-pushed the ci branch 2 times, most recently from efae9b6 to 35d00f8 Compare April 24, 2021 06:17
@katyo
Copy link
Contributor Author

katyo commented Apr 24, 2021

Unfortunately I does not understand how check minimal-versions actually works.

It does not fails on my machine. As I see wayland-sys v0.28.5 depends from pkg-config ^0.3.7.

On my machine cargo +nightly tree --all --all-features --all-targets -Z minimal-versions outputs the following (clipped):

└── wayland-sys v0.28.5
        [build-dependencies]
        └── pkg-config v0.3.19

But on CI pkg-config v0.3.0 is used instead. Why is it happens?

@MarijnS95
Copy link
Contributor

MarijnS95 commented Apr 24, 2021

@katyo Remove your Cargo.lock, then it'll show the same :)

EDIT: Notice how all other versions have a patch version as well, after this pretty much everything should have a version of .0.

@katyo
Copy link
Contributor Author

katyo commented Apr 24, 2021

@MarijnS95 Is it means that cargo does not follows semver specs?
The 0.7.0 does not satisfy constraint ^0.7.3.

@MarijnS95
Copy link
Contributor

MarijnS95 commented Apr 24, 2021

@katyo Not exactly sure what you mean. wayland-sys-0.28.0 has a dependency on pkg-config 0.3. Cargo uses semver caret matching by default meaning that this allows any pkg-config version >= 0.3.0.

In other words, I don't understand why drm-rs has an explicit caret match even if that's implicit already...

Where are you seeing versions of 0.7.0 and 0.7.3?

@katyo
Copy link
Contributor Author

katyo commented Apr 25, 2021

Hmm, seems I forget include wayland-server version fix into commit.
I hope this PR is ready to merge now.

@katyo katyo force-pushed the ci branch 2 times, most recently from 799d4d2 to e7e3ea8 Compare April 25, 2021 06:48
@MarijnS95
Copy link
Contributor

@katyo Neat, reminder that you don't need ^ though. That's implied by cargo.

Doesn't look like any GH Actions ran now? There's only the failed Travis run...

@katyo
Copy link
Contributor Author

katyo commented Apr 26, 2021

@MarijnS95 It because the PR doesn't merged. See the actions tab on my fork. https://github.com/katyo/gbm.rs/actions

@MarijnS95
Copy link
Contributor

@katyo Indeed that's how I remember it working (actions have to be merged to the default branch, or a non-fork branch first before anything runs), but somehow recall seeing a successful actions run in here. Not that it really matters though.

I guess this and #7 are now ready for merging, and perhaps a branch cleanup and crates release is nearby?

@Drakulix
Copy link
Member

@katyo Indeed that's how I remember it working (actions have to be merged to the default branch, or a non-fork branch first before anything runs), but somehow recall seeing a successful actions run in here. Not that it really matters though.

I guess this and #7 are now ready for merging, and perhaps a branch cleanup and crates release is nearby?

I will take the time to review (and likely merge) this in the next couple of days and yes, I plan to do a drm-rs and gbm-rs release shortly afterwards.

@MarijnS95
Copy link
Contributor

@katyo apologies, #7 introduced some conflicts. Should be easy for you to solve by simply rerunning rustfmt (at least simpler than having to reapply comment changes on top of rustfmt 😬). Thanks!

Copy link
Member

@Drakulix Drakulix left a comment

Choose a reason for hiding this comment

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

Thanks, I found some small nitpicks, after overall, this is again really solid. 👍

.github/workflows/ci.yml Show resolved Hide resolved
gbm-sys/build.rs Show resolved Hide resolved
src/lib.rs Outdated Show resolved Hide resolved
@katyo katyo force-pushed the ci branch 2 times, most recently from 47c9f6d to ef16eb0 Compare May 2, 2021 17:35
@katyo katyo requested a review from Drakulix May 3, 2021 05:07
Copy link
Member

@Drakulix Drakulix left a comment

Choose a reason for hiding this comment

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

Looks good to go!
One last cleanup step and then we hit that Merge button

.github/workflows/ci.yml Outdated Show resolved Hide resolved
@Drakulix Drakulix merged commit 74a823e into Smithay:develop May 3, 2021
MarijnS95 added a commit to MarijnS95/gbm.rs that referenced this pull request May 4, 2021
As mentioned in [1] the documentation example could use some
autoformatting, and recent changes have to be propagated into the README
too.

[1]: Smithay#4 (comment)
MarijnS95 added a commit to MarijnS95/gbm.rs that referenced this pull request May 4, 2021
As mentioned in [1] the documentation example could use some
autoformatting, and recent API changes have to be propagated into the
README too.

[1]: Smithay#4 (comment)
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

3 participants