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

directory is the job directory #8762

Merged
merged 2 commits into from
Jan 11, 2024
Merged

Conversation

jakecoffman
Copy link
Member

@jakecoffman jakecoffman commented Jan 11, 2024

In #8405 I was under the impression that DependencyFile's directory was the directory of the file, and name was just the basename, but that's not the case. A DependencyFile's directory should always be the job (source) directory, and the name contains the relative path from the directory and also the filename.

Not sure what problem I was running into before, but removing job_directory and filtering on directory alone is passing the smoke tests, so this is confirmed!

@jakecoffman jakecoffman marked this pull request as ready for review January 11, 2024 15:28
@jakecoffman jakecoffman requested a review from a team as a code owner January 11, 2024 15:28
Copy link
Contributor

@deivid-rodriguez deivid-rodriguez left a comment

Choose a reason for hiding this comment

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

So if I'm understanding correctly, even in a multidir group update, each dependency file will have the directory attribute set to the proper directory (from the ones included in the job).

@jakecoffman
Copy link
Member Author

@deivid-rodriguez correct! For multi-directory fetches it loops over the directories:

# Fetch dependency files for multiple directories
def dependency_files_for_multi_directories
@dependency_files_for_multi_directories ||= job.source.directories.flat_map do |dir|
ff = with_retries { file_fetcher_for_directory(dir) }
files = ff.files
post_ecosystem_versions(ff) if should_record_ecosystem_versions?
files
end
end

For each directory it sets the job source as that directory:

source: job.source.clone.tap { |s| s.directory = directory_to_use },

@Nishnha
Copy link
Member

Nishnha commented Jan 11, 2024

I see, so the directory wasn't being set on DependencyFiles during grouped updates because we were referencing the job_directory instead? Glad you spotted this!

@jakecoffman jakecoffman merged commit bc4de14 into main Jan 11, 2024
136 checks passed
@jakecoffman jakecoffman deleted the jakecoffman/remove-job-directory branch January 11, 2024 16:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants