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

when COMPILE_OPTIONS is not set, skip the REMOVE_ITEM in ${HOSTSRCS}, fix msvc reported case. #14499

Merged
merged 1 commit into from
Oct 24, 2024

Conversation

jasonbu
Copy link
Contributor

@jasonbu jasonbu commented Oct 24, 2024

Summary

sim/cmake: compatible when nuttx COMPILE_OPTIONS is not set yet
#14440
when COMPILE_OPTIONS is not set, cmake will report an error:
HOST_COMPILE_OPTIONS-NOTFOUND

as we only want to do item remove. so let's skip the remove procedure at this case.

@simbit18 I did not try the win host and re-produce the problem, please help me verify if it will work, I will do verify later.

Impact

fix bug when msvc build.

Testing

CI-test & ubuntu sim:nsh.

Signed-off-by: buxiasen <buxiasen@xiaomi.com>
@github-actions github-actions bot added Arch: simulator Issues related to the SIMulator Size: S The size of the change in this PR is small labels Oct 24, 2024
@xiaoxiang781216
Copy link
Contributor

@lupyuen could we turn on windows ci to see how much budget will be used?

@lupyuen
Copy link
Member

lupyuen commented Oct 24, 2024

@xiaoxiang781216 I'm very sorry, can we do this after 30 Oct?

Remember that ASF Infra Team is watching our GitHub Spending very closely. If we exceed their budget on 30 Oct, they will shut down GitHub Actions.

Thank you so much for your understanding, I'm very sorry we have to bear with this 🙏

@simbit18
Copy link
Contributor

simbit18 commented Oct 24, 2024

@jasonbu build nuttx with this PR is now OK !
Thank you

https://github.com/simbit18/nuttx-testing-ci/actions/runs/11499906609/job/32009014581

@lupyuen
Copy link
Member

lupyuen commented Oct 24, 2024

BTW we could fork the NuttX Repo to a new GitHub Org and test there: #14407

@jasonbu jasonbu changed the title when COMPILE_OPTIONS is not set, skip the REMOVE_ITEM in ${HOSTSRCS}, fix mscv reported case. when COMPILE_OPTIONS is not set, skip the REMOVE_ITEM in ${HOSTSRCS}, fix msvc reported case. Oct 24, 2024
@xiaoxiang781216
Copy link
Contributor

xiaoxiang781216 commented Oct 24, 2024

@xiaoxiang781216 I'm very sorry, can we do this after 30 Oct?

Remember that ASF Infra Team is watching our GitHub Spending very closely. If we exceed their budget on 30 Oct, they will shut down GitHub Actions.

Thank you so much for your understanding, I'm very sorry we have to bear with this 🙏

share, let's wait a moment. Thanks for your hard work to avoiding foundation turn off our ci.

@xiaoxiang781216 xiaoxiang781216 merged commit 974db76 into apache:master Oct 24, 2024
14 checks passed
@simbit18
Copy link
Contributor

simbit18 commented Oct 24, 2024

@xiaoxiang781216 @lupyuen I created this simple workflow to build nuttx on Windows, which starts automatically every hour, or the workflow can be activated manually. If you want, you can use it for testing in any repository.

https://github.com/simbit18/nuttx_test_pr/blob/main/.github/workflows/ci_windows.yml

this is for repository: apache/nuttx

  • name: Checkout nuttx repo
    uses: actions/checkout@v4
    with:
    repository: apache/nuttx
    ref: master

For testing, just change this
example commit jasonbu

  • name: Checkout nuttx repo
    uses: actions/checkout@v4
    with:
    repository: jasonbu/nuttx
    ref: sim_cmake

@lupyuen
Copy link
Member

lupyuen commented Oct 24, 2024

That's very cool thanks! :-)

@anchao
Copy link
Contributor

anchao commented Oct 24, 2024

@lupyuen could we turn on windows ci to see how much budget will be used?

@xiaoxiang781216 @xuxin930
why not enable MSVC ci on xiaomi internal server?
Shouldn't similar changes be built and validated internally before being submitted to the community?

@lupyuen
Copy link
Member

lupyuen commented Oct 24, 2024

msvc build is successful in our NuttX Mirror Repo yay! 🎉

https://github.com/NuttX/nuttx/actions/runs/11501541770

@jasonbu
Copy link
Contributor Author

jasonbu commented Oct 24, 2024

@lupyuen could we turn on windows ci to see how much budget will be used?

@xiaoxiang781216 @xuxin930 why not enable MSVC ci on xiaomi internal server? Shouldn't similar changes be built and validated internally before being submitted to the community?

@anchao
the #14440 is created because I found the mac sim:nsh break in my personal environment.
And I totally trust the community CI. So only fix mac sim:nsh case and test by CI & local linux sim:nsh.
Also the internal server not fully running by cmake, so maybe later will able to cover mscv cmake case too.

@anchao
Copy link
Contributor

anchao commented Oct 25, 2024

@lupyuen could we turn on windows ci to see how much budget will be used?

@xiaoxiang781216 @xuxin930 why not enable MSVC ci on xiaomi internal server? Shouldn't similar changes be built and validated internally before being submitted to the community?

@anchao the #14440 is created because I found the mac sim:nsh break in my personal environment. And I totally trust the community CI. So only fix mac sim:nsh case and test by CI & local linux sim:nsh. Also the internal server not fully running by cmake, so maybe later will able to cover mscv cmake case too.

Because setting up the MSVC environment is not a huge work, you only need to install the MSVC environment in wine or support MSVC in docker, and the compilation process does not require the support of GUI interface, CMD is enough

@xuxin930
Copy link
Contributor

why not enable MSVC ci on xiaomi internal server? Shouldn't similar changes be built and validated internally before being submitted to the community?

sure @anchao, we have a CI task for msvc, but it is daily, but there are no achieve per patch yet :-(

@anchao
Copy link
Contributor

anchao commented Oct 25, 2024

why not enable MSVC ci on xiaomi internal server? Shouldn't similar changes be built and validated internally before being submitted to the community?

sure @anchao, we have a CI task for msvc, but it is daily, but there are no achieve per patch yet :-(

@xuxin930 @jasonbu Thank you, sounds a good start.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Arch: simulator Issues related to the SIMulator Size: S The size of the change in this PR is small
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants