Skip to content

Conversation

@eli-schwartz
Copy link
Contributor

Just like autotools, ragel should only be required when rl sources are actually out of date.

Fixes #3378

@eli-schwartz eli-schwartz force-pushed the ragel-meson branch 2 times, most recently from 9c9fb91 to 2f55154 Compare January 18, 2022 21:09
- run: sudo pip3 install fonttools meson==0.56.0
- name: run
run: meson build -Db_coverage=true --auto-features=enabled -Dgraphite=enabled -Dchafa=disabled -Dragel_subproject=true -Doptimization=2
run: meson build -Db_coverage=true --auto-features=enabled -Dgraphite=enabled -Dchafa=disabled -Dragel=disabled -Doptimization=2
Copy link
Collaborator

Choose a reason for hiding this comment

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

We no longer test the ragel subproject on ci builds, or am I missing something?

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 project options specify wrap_mode=nofallback

@khaledhosny khaledhosny added the meson meson build system label Jan 19, 2022
@behdad
Copy link
Member

behdad commented Feb 12, 2022

Is this ready to be merged?

@behdad
Copy link
Member

behdad commented Feb 12, 2022

Why is the fedora bot failing? Should we instruct it to install ragel then?

@khaledhosny
Copy link
Collaborator

khaledhosny commented Feb 12, 2022

Why is the fedora bot failing? Should we instruct it to install ragel then?

We shouldn’t need ragel on any CI jobs (unless we are specifically testing ragel generation, which i snot the case here). That being said, we do actually install ragel on this job but Fedora has the new ragel that we don’t want. This IMHO means the changes in this PR isn’t quite working yet because users shouldn’t need to do anything if they are simply building HarfBuzz and suitable ragel is not present.

)
endforeach
message('You have to install ragel if you are going to develop HarfBuzz itself')
ragel = 'error'
Copy link
Contributor

Choose a reason for hiding this comment

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

Why this?

ragel = 'error'
endif
ragel_helper = find_program('gen-ragel-artifacts.py')
foreach rl : hb_base_ragel_sources
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 part should remain in the else case of if not has_ragel, that's what make the CI fail AFAIK.

@behdad
Copy link
Member

behdad commented Jul 18, 2022

Is someone going to finish this?

@eli-schwartz
Copy link
Contributor Author

eli-schwartz commented Jul 18, 2022

I lost track of this PR (sorry!) but I am still interested in getting it to work.

@behdad
Copy link
Member

behdad commented Feb 28, 2023

I lost track of this PR (sorry!) but I am still interested in getting it to work.

Any chance you can finish this?

@behdad
Copy link
Member

behdad commented Feb 19, 2025

What's the status of this?

Do not run ragel unless the rl sources have actually been edited. If
they have been edited, error with an informative error message
…oject()

It does automatic subproject fallback and simplifies the code.
@eli-schwartz
Copy link
Contributor Author

Sorry I forgot about this again. :( I have just rebased it including some tweaks for the CI.

@behdad
Copy link
Member

behdad commented Apr 5, 2025

Let's decide one way or another here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

meson meson build system

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Harfbuzz is broken with --fatal-meson-warnings

4 participants