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

Fixed troff warning in versions above groff-1.23 #416

Closed
wants to merge 1 commit into from

Conversation

gordonwwang
Copy link

@gordonwwang gordonwwang commented Aug 10, 2023

  • There are problems:

When the compilation dependency is groff-1.23, the following message is displayed in the compilation log, and the compilation fails:

make[1]: Entering directory '/builddir/build/BUILD/openvswitch-3.1.1/build'
troff:vswitchd/ovs-vswitchd.8:1298: warning: cannot select font 'CW'
make[1]: Leaving directory '/builddir/build/BUILD/openvswitch-3.1.1/build'
make[1]: *** [Makefile:6761: manpage-check] Error 1

The full build log is here:https://build.stream.opencloudos.tech/kojifiles/work/tasks/5893/35893/build.log

  • How to reproduce:

Install groff-1.23.0 and perform the make operation on the Makefile.

I found this problem during the %build process when compiling openvswitch.spec.

  • Solution:

I check submitted records of groff (https://git.savannah.gnu.org/cgit/groff.git/commit/?id=d75ea8b2e283e37bd560e821fa4597065f36725f), and found out they removed the "CW" font.
openvswitch is still using "CW" here, so I think it needs to be tweaked here. You can substitute ".ft CR" or ".ft R", depending on which font you want to use.
I temporarily use ".ft R "here. If you need, I can adjust my code.

@igsilya
Copy link
Member

igsilya commented Aug 10, 2023

Hi, @GitHubGDong . Thanks for the patch!
I believe, we should use CR instead of R, because CW is supposed to be an alias for CR.

Please add some information about the issue and a fix to the commit message directly.
And you also need to provide a Signed-off-by tag so we can accept the change. It should be in a following format:
Signed-off-by: Firstname Lastname <[email protected]>

Here is what FAQ says about that:

Q: What's a Signed-off-by and how do I provide one?

    A: Free and open source software projects usually require a contributor to
    provide some assurance that they're entitled to contribute the code that
    they provide.  Some projects, for example, do this with a Contributor
    License Agreement (CLA) or a copyright assignment that is signed on paper
    or electronically.

    For this purpose, Open vSwitch has adopted something called the Developer's
    Certificate of Origin (DCO), which is also used by the Linux kernel and
    originated there.  Informally stated, agreeing to the DCO is the
    developer's way of attesting that a particular commit that they are
    contributing is one that they are allowed to contribute.  You should visit
    https://developercertificate.org/ to read the full statement of the DCO,
    which is less than 200 words long.

    To certify compliance with the Developer's Certificate of Origin for a
    particular commit, just add the following line to the end of your commit
    message, properly substituting your name and email address:

        Signed-off-by: Firstname Lastname <[email protected]>

    Git has special support for adding a Signed-off-by line to a commit
    message: when you run "git commit", just add the -s option, as in "git
    commit -s".  If you use the "git citool" GUI for commits, you can add a
    Signed-off-by line to the commit message by pressing Control+S.  Other Git
    user interfaces may provide similar support.

If you plan to submit patches regularly, then you should try our main process for submitting patches via OVS development mailing list: https://docs.openvswitch.org/en/latest/internals/contributing/submitting-patches/ . We do not use GitHub PRs normally, but it's fine to create PRs once in a while.

@gordonwwang
Copy link
Author

Ok, I've tweaked the code and provided the Signed-off-by tag.
In the future, I will try to develop mailing list through OVS for new patch submission.

@igsilya
Copy link
Member

igsilya commented Aug 11, 2023

Hi, thanks for the update! The code looks good to me.

I see you added 2 sign-off tags, that is causing a checkpatch warning:

$ ./utilities/checkpatch.py -1

== Checking 9eb8d1dfce63 ("[PATCH v2] Use font CR") ==
WARNING: Unexpected sign-offs from developers who are not authors or co-authors or committers: jackchan-x <[email protected]>
Lines checked: 27, Warnings: 1, Errors: 0

It should be one sign-off per person. If you want to add a co-author, you should also add a Co-authored-by tag for that person.

Also, please, squash all the patches into one and force-push. We do not do merges, rebase only.

@gordonwwang
Copy link
Author

gordonwwang commented Aug 14, 2023

@igsilya
Ok, thank you for your patience~
I have submitted my code according to the latest format requirements.

@igsilya
Copy link
Member

igsilya commented Aug 16, 2023

Hi, @GitHubGDong. Thanks for the update! There is still one issue with the patch:

$ ./utilities/checkpatch.py -1
== Checking 499a1daffe03 ("[PATCH 1/1] lib/ovs.tmac: Fixed troff warning in versions above groff-1.23") ==
ERROR: Co-author jackchan-x <[email protected]> needs to sign off.
Lines checked: 27, Warnings: 0, Errors: 1

As I said before, it should be one sign-off per person, so we need Signed-off-by tag from a Co-author as well.
You can check the patch yourself by running a checkpatch script as shown above.

Also, nicknames in tags are not generally accepted. Real names should be used whenever possible, since it is a form of a legal agreement.

…-1.23

Signed-off-by: gordonwwang <[email protected]>
Signed-off-by: Xiaojie Chen <[email protected]>
Co-authored-by: Xiaojie Chen <[email protected]>
@gordonwwang
Copy link
Author

gordonwwang commented Aug 17, 2023

@igsilya
Sorry, I just noticed that you used the script utilities/checkpatch.py, I thought CI access was enough.
I read the code for utilities/checkpatch.py, and when I add a "Co-authored-by", I also need to add a "Signed-off-by" tag.

I updated the signature and now check the result as follows:

./utilities/checkpatch.py -1
== Checking 2428050aef9e ("[PATCH 1/1] lib/ovs.tmac: Fixed troff warning in versions above groff-1.23") ==
Lines checked: 28, no obvious problems found

igsilya pushed a commit that referenced this pull request Aug 17, 2023
When the compilation dependency is groff-1.23, the following message is
displayed in the compilation log, and the compilation fails:

  troff:vswitchd/ovs-vswitchd.8:1298: warning: cannot select font 'CW'
  make[1]: *** [Makefile:6761: manpage-check] Error 1

CW font was removed and and now groff warns about non-existent font:
 https://git.savannah.gnu.org/cgit/groff.git/commit/?id=d75ea8b2e283e37bd560e821fa4597065f36725f)

Fix that by replacing CW with CR.  CW supposed to be an alias of CR
anyway.

Submitted-at: #416
Co-authored-by: Xiaojie Chen <[email protected]>
Signed-off-by: Xiaojie Chen <[email protected]>
Signed-off-by: gordonwwang <[email protected]>
Signed-off-by: Ilya Maximets <[email protected]>
igsilya pushed a commit that referenced this pull request Aug 17, 2023
When the compilation dependency is groff-1.23, the following message is
displayed in the compilation log, and the compilation fails:

  troff:vswitchd/ovs-vswitchd.8:1298: warning: cannot select font 'CW'
  make[1]: *** [Makefile:6761: manpage-check] Error 1

CW font was removed and and now groff warns about non-existent font:
 https://git.savannah.gnu.org/cgit/groff.git/commit/?id=d75ea8b2e283e37bd560e821fa4597065f36725f)

Fix that by replacing CW with CR.  CW supposed to be an alias of CR
anyway.

Submitted-at: #416
Co-authored-by: Xiaojie Chen <[email protected]>
Signed-off-by: Xiaojie Chen <[email protected]>
Signed-off-by: gordonwwang <[email protected]>
Signed-off-by: Ilya Maximets <[email protected]>
igsilya pushed a commit that referenced this pull request Aug 17, 2023
When the compilation dependency is groff-1.23, the following message is
displayed in the compilation log, and the compilation fails:

  troff:vswitchd/ovs-vswitchd.8:1298: warning: cannot select font 'CW'
  make[1]: *** [Makefile:6761: manpage-check] Error 1

CW font was removed and and now groff warns about non-existent font:
 https://git.savannah.gnu.org/cgit/groff.git/commit/?id=d75ea8b2e283e37bd560e821fa4597065f36725f)

Fix that by replacing CW with CR.  CW supposed to be an alias of CR
anyway.

Submitted-at: #416
Co-authored-by: Xiaojie Chen <[email protected]>
Signed-off-by: Xiaojie Chen <[email protected]>
Signed-off-by: gordonwwang <[email protected]>
Signed-off-by: Ilya Maximets <[email protected]>
igsilya pushed a commit that referenced this pull request Aug 17, 2023
When the compilation dependency is groff-1.23, the following message is
displayed in the compilation log, and the compilation fails:

  troff:vswitchd/ovs-vswitchd.8:1298: warning: cannot select font 'CW'
  make[1]: *** [Makefile:6761: manpage-check] Error 1

CW font was removed and and now groff warns about non-existent font:
 https://git.savannah.gnu.org/cgit/groff.git/commit/?id=d75ea8b2e283e37bd560e821fa4597065f36725f)

Fix that by replacing CW with CR.  CW supposed to be an alias of CR
anyway.

Submitted-at: #416
Co-authored-by: Xiaojie Chen <[email protected]>
Signed-off-by: Xiaojie Chen <[email protected]>
Signed-off-by: gordonwwang <[email protected]>
Signed-off-by: Ilya Maximets <[email protected]>
igsilya pushed a commit that referenced this pull request Aug 17, 2023
When the compilation dependency is groff-1.23, the following message is
displayed in the compilation log, and the compilation fails:

  troff:vswitchd/ovs-vswitchd.8:1298: warning: cannot select font 'CW'
  make[1]: *** [Makefile:6761: manpage-check] Error 1

CW font was removed and and now groff warns about non-existent font:
 https://git.savannah.gnu.org/cgit/groff.git/commit/?id=d75ea8b2e283e37bd560e821fa4597065f36725f)

Fix that by replacing CW with CR.  CW supposed to be an alias of CR
anyway.

Submitted-at: #416
Co-authored-by: Xiaojie Chen <[email protected]>
Signed-off-by: Xiaojie Chen <[email protected]>
Signed-off-by: gordonwwang <[email protected]>
Signed-off-by: Ilya Maximets <[email protected]>
@igsilya
Copy link
Member

igsilya commented Aug 17, 2023

Thanks! Applied as commit 0945e1a. Also backported to all branches down to 2.17.

@igsilya igsilya closed this Aug 17, 2023
roseoriorden pushed a commit to roseoriorden/ovs that referenced this pull request Jul 1, 2024
When the compilation dependency is groff-1.23, the following message is
displayed in the compilation log, and the compilation fails:

  troff:vswitchd/ovs-vswitchd.8:1298: warning: cannot select font 'CW'
  make[1]: *** [Makefile:6761: manpage-check] Error 1

CW font was removed and and now groff warns about non-existent font:
 https://git.savannah.gnu.org/cgit/groff.git/commit/?id=d75ea8b2e283e37bd560e821fa4597065f36725f)

Fix that by replacing CW with CR.  CW supposed to be an alias of CR
anyway.

Submitted-at: openvswitch#416
Co-authored-by: Xiaojie Chen <[email protected]>
Signed-off-by: Xiaojie Chen <[email protected]>
Signed-off-by: gordonwwang <[email protected]>
Signed-off-by: Ilya Maximets <[email protected]>
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.

2 participants