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

configure: Make rpmlint optional #5658

Closed

Conversation

stanislavlevin
Copy link
Contributor

Distributions may want to run comprehensive fastcheck or lint tasks, but rpmlint tool is mandatory for these targets while some platforms don't have it at all.

With this change the rpmlint becomes optional for fastcheck, devcheck and lint make targets.

Note: rpmlint option is disabled by default.
To enable: ./configure --enable-rpmlint
To explicitly disable: ./configure --disable-rpmlint

Fixes: https://pagure.io/freeipa/issue/8768

configure.ac Outdated
@@ -672,6 +689,7 @@ echo "
Python: ${PYTHON} (${PYTHON_VERSION})
pylint: ${PYLINT}
jslint: ${JSLINT}
rpmlint: $enable_rpmlint
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: shouldn't this follow the same pattern as the others, e.g. upper-case ${ENABLE_RPMLINT} ?

Copy link
Contributor

Choose a reason for hiding this comment

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

Other than the nit the patch LGTM.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

PYLINT and JSLINT are shell variables in this case as well as enable_rpmlint. There is no common convention in IPA configure.ac regarding upper/low case(both exist). It doesn't matter for me. If you think the upper is more appropriate then I'll change it.

Copy link
Contributor

Choose a reason for hiding this comment

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

That's true about inconsistency but most (I didn't check for all) of the optional/detected features use upper case. They universally use the ${} format.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you, pushed the 'upper' version.

Distributions may want to run comprehensive fastcheck or lint tasks,
but rpmlint tool is mandatory for these targets while some platforms
don't have it at all.

With this change the rpmlint becomes optional for fastcheck, devcheck
and lint make targets.

Note: rpmlint option is disabled by default.
To enable: ./configure --enable-rpmlint
To explicitly disable: ./configure --disable-rpmlint

Fixes: https://pagure.io/freeipa/issue/8768
Signed-off-by: Stanislav Levin <slev@altlinux.org>
Template the autoconf phase.

Fixes: https://pagure.io/freeipa/issue/8768
Signed-off-by: Stanislav Levin <slev@altlinux.org>
@rcritten
Copy link
Contributor

Maybe it's just that I have a tough time understanding how Azure actually works but the template script(s) aren't displayed in the output AFAICT so its difficult to verify that the right template is actually applied. For example the closest I can find to calling autogen.sh is:

2021-03-24T15:20:53.8776078Z [command]/usr/bin/bash --noprofile --norc /__w/_temp/0ed4b940-be80-48fa-be3a-705ef9e693a2.sh
2021-03-24T15:20:53.8776736Z Running autoconf generator
2021-03-24T15:20:54.3655448Z Copying file ABOUT-NLS

But what are the contents of /__w/_temp/0ed4b940-be80-48fa-be3a-705ef9e693a2.sh ?

@stanislavlevin
Copy link
Contributor Author

The content of script task is copied into xxx.sh and then executed.

  5 - script: |                                                                     
  6     set -e                                                                      
  7     echo "Running autoconf generator"                                           
  8     ./autogen.sh \                                                              
  9         ${{ parameters.options }} \                                             
 10         --enable-rpmlint \

The traditional way is set -x. Another option is system diagnostic option in Azure Web interface, but this requires manual run/rerun: https://dev.azure.com/freeipa/freeipa/_build/results?buildId=8324&view=results

@abbra abbra added ipa-4-9 Mark for backport to ipa 4.9 ipa-4-8 Mark for backport to ipa 4.8 labels Mar 29, 2021
@abbra
Copy link
Contributor

abbra commented Mar 29, 2021

LGTM.

@abbra abbra added ack Pull Request approved, can be merged pushed Pull Request has already been pushed labels Mar 29, 2021
@abbra
Copy link
Contributor

abbra commented Mar 29, 2021

master:

  • 7fc0c04 configure: Make rpmlint optional
  • 83e16a4 Azure: Run rpmlint on Fedora

@abbra
Copy link
Contributor

abbra commented Mar 29, 2021

@stanislavlevin could you please do ipa-4-8 backport? Automation failed:

Applying to ipa-4-8: configure: Make rpmlint optional
Applying to ipa-4-8: Azure: Run rpmlint on Fedora
Failure to apply patches: error: Не удалось слить изменения.
подсказка: Use 'git am --show-current-patch=diff' to see the failed patch

Failed to apply patches onto origin/ipa-4-8. Manual backport is needed.

@stanislavlevin
Copy link
Contributor Author

@abbra, thank you!

@stanislavlevin could you please do ipa-4-8 backport? Automation failed:

Applying to ipa-4-8: configure: Make rpmlint optional
Applying to ipa-4-8: Azure: Run rpmlint on Fedora
Failure to apply patches: error: Не удалось слить изменения.
подсказка: Use 'git am --show-current-patch=diff' to see the failed patch

Failed to apply patches onto origin/ipa-4-8. Manual backport is needed.

Sure, #5677

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ack Pull Request approved, can be merged ipa-4-8 Mark for backport to ipa 4.8 ipa-4-9 Mark for backport to ipa 4.9 pushed Pull Request has already been pushed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants