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

[R-package] remove temporary files created in configure.win #4752

Merged
merged 1 commit into from
Oct 30, 2021

Conversation

jameslamb
Copy link
Collaborator

On Windows builds in #3946, I see the following NOTE from R CMD check

* checking top-level files ... NOTE

Non-standard file/directory found at top level:

  'conftest.cpp'

Example build: https://github.com/microsoft/LightGBM/runs/3367486204?check_suite_focus=true.

After pushing the same change from this PR to #3946, I can confirm that this R CMD check NOTE no longer shows up in cran Windows builds. e.g. https://github.com/microsoft/LightGBM/runs/4053638197?check_suite_focus=true.

The file conftest.cpp is created by tests run in configure.win which compile test programs to figure out compiler flags to use when compiling LightGBM.

cat > conftest.cpp <<EOL

cat > conftest.cpp <<EOL

This PR proposes adding code to configure.win to ensure that such files are removed as soon as those tests are done.

Shouldn't a similar change be made in configure.ac / configure?

I don't think this is necessary for configure.ac because I believe configure (created by autoconf) handles that automatically through the use of autoconf's AC_LANG_CONFTEST directive.

rm -f -r conftest* confdefs.h

Why haven't we seen this note from R CMD check before?

I think this is only being observed in #3946 because with vignettes added, R CMD build has to install the package to build it (since R CMD build bundles the vignette HTML into source packages by default).

Copy link
Collaborator

@StrikerRUS StrikerRUS left a comment

Choose a reason for hiding this comment

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

LGTM!
Thank you very much for answering to my questions even before I asked them 😄

@StrikerRUS StrikerRUS merged commit d574fcb into master Oct 30, 2021
@StrikerRUS StrikerRUS deleted the fix/windows-configure branch October 30, 2021 16:14
@StrikerRUS StrikerRUS mentioned this pull request Jan 6, 2022
13 tasks
@jameslamb jameslamb mentioned this pull request Oct 7, 2022
40 tasks
@github-actions
Copy link

This pull request has been automatically locked since there has not been any recent activity since it was closed. To start a new related discussion, open a new issue at https://github.com/microsoft/LightGBM/issues including a reference to this.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 23, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants