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

[BUG] submodule sync script assumes cudf commits are required to see any changes #2484

Closed
jlowe opened this issue Oct 9, 2024 · 1 comment · Fixed by #2504
Closed

[BUG] submodule sync script assumes cudf commits are required to see any changes #2484

jlowe opened this issue Oct 9, 2024 · 1 comment · Fixed by #2504
Assignees
Labels
bug Something isn't working build

Comments

@jlowe
Copy link
Member

jlowe commented Oct 9, 2024

The cudf submodule sync script has this logic:

if [[ "${cudf_sha}" == "${cudf_prev_sha}" ]]; then
  echo "Submodule is up to date."
  exit 0
fi

The problem with this logic is that there could be changes to cudf dependencies without changes to cudf itself, and the only way to pick up those changes is to run the submodule sync. That will build with -Dlibcudf.dependency.mode=latest causing any new versions of cudf dependencies to be picked up in the pinned versions file.

@jlowe jlowe added ? - Needs Triage bug Something isn't working build labels Oct 9, 2024
@pxLi
Copy link
Collaborator

pxLi commented Oct 15, 2024

Eliminating the check that skips certain runs and executing the build every time (validate part), or comparing the latest commits across all RAPIDS ecosystem projects, might help resolve the issue.

Currently, with the patch framework, each submodule-sync run takes approximately 2.5 hours (to run the validate to update cudf-pins would cost ~1hr). The check was initially added to conserve resources, do you have any suggestions for this? thank you

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working build
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants