这是indexloc提供的服务,不要输入任何密码
Skip to content

Conversation

@Finii
Copy link
Collaborator

@Finii Finii commented Apr 22, 2022

Description

Fix release CI pipeline that does not patch all fonts.

Requirements / Checklist

What does this Pull Request (PR) do?

Allow the release script to also patch fonts where the directory name of the font in src/ is not the same as the (beginning of the ) font file name.

How should this be manually tested?

Let the CI run and examine the job. All sub-jobs from the matrix have to succeed and that means now that they patch at least one font file. Afterwards examine that all fonts in patched-fonts/ have been rebuild.

Any background context you can provide?

What are the relevant tickets (if any)?

#824

Please also consider related #786

Screenshots (if appropriate or helpful)

Finii added 3 commits April 22, 2022 13:29
[why]
When we do CI we probably want to raise an error if no font file could
be found.

[how]
Check how many font files have been generated and return with exit
code 1 if the number is zero.

Signed-off-by: Fini Jastrow <ulf.fini.jastrow@desy.de>
[why]
We can limit the font files to patch with the gotta-patch-em-all script
to some specific file names; the case insensitive beginning of the
basename of the font file, to be specific.

Sometimes we do not know how the actual font files are named, we just
know the directory name.

[how]
The old filter checked for the beginning of a file name, so we keep this
behavior and allow to check for the beginning of a directory name.

As differentiator the first character of the passed filter is used. As a
slash is not allowed in the file name filters: If the first char is a
slash, we search for font files in a directory that begins with that
name. The file name is not considered in this case.

Examples:

gotta-patch-em-all-font-patcher!.sh Fira

  Patches all font files that begin (case insensitive) with 'fira'.
  It must be the basename of the font file, the path to the file is not
  considered. (Old behavior.)
  Translated to a glob this is roughly **/Fira*.[ot]tf

gotta-patch-em-all-font-patcher!.sh /Fira

  Patches all font files that are in directories that begin (case
  insensitive) with 'fira'. It can not be the beginning of the font
  file basename.
  Translated to a glob this is roughly **/Fira*/**/*.[ot]tf

Signed-off-by: Fini Jastrow <ulf.fini.jastrow@desy.de>
[why]
The font matix contains the directory names of the font files. The
directory names are taken from
  bin/scripts/lib/fonts.json
and specifically extracted with
  bin/scripts/get-font-names-from-json.sho

That script returns the .folderNames

Later in the release script we use the folder name to limit the fonts
that patch-em-all shall process. Unfortunately patch-em-all could (until
the previous commit) just work with font-filenames and not with
directory names. But the matrix gives us just directory names.

This is for example a problem with this fonts:
$ ll src/unpatched-fonts/BigBlueTerminal
-rw-rw-r-- 1 fini fini 25632 Jan  1 14:03 BigBlue_Terminal_437TT.TTF
-rw-rw-r-- 1 fini fini 69964 Jan  1 14:03 BigBlue_TerminalPlus.TTF

The *directory* of that fonts is correctly noted in fonts.json as
"BigBlueTerminal" but the font files do not begin with that!

[how]
Now, that patch-em-all can also filter on the directory name, we just
need to utilized that in the workflow run. If the filter shall work on
directory names the first character needs to be a slash, so we just
prepend it to name we got from the matix.

Fixes: #824

Signed-off-by: Fini Jastrow <ulf.fini.jastrow@desy.de>
Copy link

@kamack38 kamack38 left a comment

Choose a reason for hiding this comment

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

LGTM

@Finii Finii added the Bug fix label Apr 29, 2022
@Finii
Copy link
Collaborator Author

Finii commented Apr 29, 2022

LGTM

After looking in their Contribution activity timeline ... Leaving 'LGTM' in random repos to get the reviews count up?

@kamack38
Copy link

I only review things I think I know about.

@Finii Finii added this to the v2.2.0 milestone Aug 18, 2022
@Finii Finii merged commit 6e37782 into master Aug 18, 2022
@Finii Finii deleted the bugfix/ci-release-run branch August 18, 2022 09:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants