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

Never run ragel when generated files exist #173

Merged
merged 1 commit into from
Nov 18, 2019
Merged

Never run ragel when generated files exist #173

merged 1 commit into from
Nov 18, 2019

Conversation

SimonSapin
Copy link
Member

This is untested, but hopefully works around servo/servo#24611.

This is not suitable for development of Harfbuzz itself (as opposed to bindings), but hopefully that doesn’t happen in this repository and being required to run cargo clean after updating the Harfbuzz version is acceptable.

@SimonSapin
Copy link
Member Author

r? @jdm

Copy link
Member

@jdm jdm left a comment

Choose a reason for hiding this comment

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

Despite the fact that a bunch of updates to harfbuzz have merged since the last harfbuzz-sys release April, I've checked the changes to the Rust library and doing a minor version update here looks ok to me.

@@ -308,7 +308,7 @@ EXTRA_DIST += \
$(NULL)
# We decided to add ragel-generated files to git...
#MAINTAINERCLEANFILES += $(RAGEL_GENERATED)
$(srcdir)/%.hh: $(srcdir)/%.rl
$(srcdir)/%.hh:
Copy link
Member

Choose a reason for hiding this comment

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

If we apply this change, we need some way to preserve this when we update harfbuzz to new releases, which happens fairly regularly.

@jdm
Copy link
Member

jdm commented Nov 18, 2019

Rather than changing an in-tree makefile, let's update https://github.com/servo/rust-harfbuzz/blob/master/harfbuzz-sys/makefile.touch with the hh files instead.

@jdm
Copy link
Member

jdm commented Nov 18, 2019

@bors-servo r+

@bors-servo
Copy link
Contributor

📌 Commit 129e6b7 has been approved by jdm

@bors-servo
Copy link
Contributor

⌛ Testing commit 129e6b7 with merge eba6132...

bors-servo pushed a commit that referenced this pull request Nov 18, 2019
Never run ragel when generated files exist

This is untested, but hopefully works around servo/servo#24611.

This is not suitable for development of Harfbuzz itself (as opposed to bindings), but hopefully that doesn’t happen in this repository and being required to run `cargo clean` after updating the Harfbuzz version is acceptable.
@bors-servo
Copy link
Contributor

☀️ Test successful - checks-travis
Approved by: jdm
Pushing eba6132 to master...

@bors-servo bors-servo merged commit 129e6b7 into master Nov 18, 2019
SimonSapin added a commit to servo/servo that referenced this pull request Nov 18, 2019
… to pick up servo/rust-harfbuzz#173, which hopefully fixes #24611
SimonSapin added a commit to servo/servo that referenced this pull request Nov 18, 2019
… to pick up servo/rust-harfbuzz#173, which hopefully fixes #24611
bors-servo pushed a commit to servo/servo that referenced this pull request Nov 18, 2019
Update harfbuzz-sys

… to pick up servo/rust-harfbuzz#173, which hopefully fixes #24611
@waywardmonkeys
Copy link
Collaborator

waywardmonkeys commented Nov 19, 2019

This is another thing that would be fixed by PR #170

@SimonSapin SimonSapin deleted the ragelless branch November 19, 2019 08:07
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.

4 participants