Skip to content

Conversation

@levitte
Copy link
Member

@levitte levitte commented Sep 18, 2025

This contains multiple fixes:

  • Fix util/find-doc-nits' environment variable check exceptions

    Some files in @except_env_files are located in the build directory,
    not the source directory.

    Furthermore, because the files and directories in @except_dirs and
    @except_env_files may look different than the elements in what find()
    returns, realpath() must be used to ensure that file name comparison
    matches when it should.

  • Fix util/find-doc-nits' check_env_vars to show where envvars were found

    This displays the list of files with line number for each envvar.

  • Fix util/find-doc-nits' check_env_vars to look for files with 'git ls-files'

    If that fails, it will fall back to finding the files with Find::file.

Some files in @except_env_files are located in the build directory,
not the source directory.

Furthermore, because the files and directories in @except_dirs and
@except_env_files may look different than the elements in what find()
returns, realpath() must be used to ensure that file name comparison
matches when it should.
This displays the list of files with line number for each envvar.
…-files'

If that fails, it will fall back to finding the files with Find::file.
@levitte levitte added branch: master Merge to master branch approval: review pending This pull request needs review by a committer tests: exempted The PR is exempt from requirements for testing branch: 3.6 Merge to openssl-3.6 labels Sep 18, 2025
@levitte
Copy link
Member Author

levitte commented Sep 18, 2025

Perhaps this should be propagated to older releases as well? I haven't checked, can somebody help?

@levitte
Copy link
Member Author

levitte commented Sep 18, 2025

Should anyone wonder, all these changes remove a number of false positives that can happen because of the presence of untracked files that are set to be ignored (through .gitignore or .git/info/exclude), as well as file names that should have matched but didn't for diverse reasons.

@levitte
Copy link
Member Author

levitte commented Sep 20, 2025

Ping?

@levitte levitte requested a review from a team September 23, 2025 09:32
Copy link
Contributor

@jogme jogme left a comment

Choose a reason for hiding this comment

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

Looking good; the output is very useful! Thank you!

@levitte
Copy link
Member Author

levitte commented Oct 27, 2025

Ping? @openssl/committers

Copy link
Member

@beldmit beldmit left a comment

Choose a reason for hiding this comment

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

LGTM

@beldmit beldmit added approval: done This pull request has the required number of approvals and removed approval: review pending This pull request needs review by a committer labels Oct 27, 2025
@openssl-machine openssl-machine added approval: ready to merge The 24 hour grace period has passed, ready to merge and removed approval: done This pull request has the required number of approvals labels Oct 30, 2025
@openssl-machine
Copy link
Collaborator

This pull request is ready to merge

@nhorman
Copy link
Contributor

nhorman commented Oct 30, 2025

merged to master and 3.6, thank you!

@nhorman nhorman closed this Oct 30, 2025
openssl-machine pushed a commit that referenced this pull request Oct 30, 2025
Some files in @except_env_files are located in the build directory,
not the source directory.

Furthermore, because the files and directories in @except_dirs and
@except_env_files may look different than the elements in what find()
returns, realpath() must be used to ensure that file name comparison
matches when it should.

Reviewed-by: Neil Horman <nhorman@openssl.org>
Reviewed-by: Dmitry Belyavskiy <beldmit@gmail.com>
(Merged from #28601)
openssl-machine pushed a commit that referenced this pull request Oct 30, 2025
This displays the list of files with line number for each envvar.

Reviewed-by: Neil Horman <nhorman@openssl.org>
Reviewed-by: Dmitry Belyavskiy <beldmit@gmail.com>
(Merged from #28601)
openssl-machine pushed a commit that referenced this pull request Oct 30, 2025
…-files'

If that fails, it will fall back to finding the files with Find::file.

Reviewed-by: Neil Horman <nhorman@openssl.org>
Reviewed-by: Dmitry Belyavskiy <beldmit@gmail.com>
(Merged from #28601)
openssl-machine pushed a commit that referenced this pull request Oct 30, 2025
Some files in @except_env_files are located in the build directory,
not the source directory.

Furthermore, because the files and directories in @except_dirs and
@except_env_files may look different than the elements in what find()
returns, realpath() must be used to ensure that file name comparison
matches when it should.

Reviewed-by: Neil Horman <nhorman@openssl.org>
Reviewed-by: Dmitry Belyavskiy <beldmit@gmail.com>
(Merged from #28601)

(cherry picked from commit 29fa220)
openssl-machine pushed a commit that referenced this pull request Oct 30, 2025
This displays the list of files with line number for each envvar.

Reviewed-by: Neil Horman <nhorman@openssl.org>
Reviewed-by: Dmitry Belyavskiy <beldmit@gmail.com>
(Merged from #28601)

(cherry picked from commit 56d138e)
openssl-machine pushed a commit that referenced this pull request Oct 30, 2025
…-files'

If that fails, it will fall back to finding the files with Find::file.

Reviewed-by: Neil Horman <nhorman@openssl.org>
Reviewed-by: Dmitry Belyavskiy <beldmit@gmail.com>
(Merged from #28601)

(cherry picked from commit 4da42df)
MegaManSec pushed a commit to MegaManSec/openssl that referenced this pull request Nov 11, 2025
Some files in @except_env_files are located in the build directory,
not the source directory.

Furthermore, because the files and directories in @except_dirs and
@except_env_files may look different than the elements in what find()
returns, realpath() must be used to ensure that file name comparison
matches when it should.

Reviewed-by: Neil Horman <nhorman@openssl.org>
Reviewed-by: Dmitry Belyavskiy <beldmit@gmail.com>
(Merged from openssl#28601)
MegaManSec pushed a commit to MegaManSec/openssl that referenced this pull request Nov 11, 2025
This displays the list of files with line number for each envvar.

Reviewed-by: Neil Horman <nhorman@openssl.org>
Reviewed-by: Dmitry Belyavskiy <beldmit@gmail.com>
(Merged from openssl#28601)
MegaManSec pushed a commit to MegaManSec/openssl that referenced this pull request Nov 11, 2025
…-files'

If that fails, it will fall back to finding the files with Find::file.

Reviewed-by: Neil Horman <nhorman@openssl.org>
Reviewed-by: Dmitry Belyavskiy <beldmit@gmail.com>
(Merged from openssl#28601)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approval: ready to merge The 24 hour grace period has passed, ready to merge branch: master Merge to master branch branch: 3.6 Merge to openssl-3.6 tests: exempted The PR is exempt from requirements for testing

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants