Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

#

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Jump to bottom

Update to the dependency name to avoid download collisions in forc_build. #611

Merged
merged 1 commit into from
Jan 11, 2022
Merged

Update to the dependency name to avoid download collisions in forc_build. #611

merged 1 commit into from
Jan 11, 2022

Conversation

eureka-cpu
Copy link
Contributor

@eureka-cpu eureka-cpu commented Jan 10, 2022 •
edited
Loading

Updated the file that a git dependency is downloaded to to avoid collisions where dependencies have the same name, but different URLs.

@eureka-cpu eureka-cpu added bug Something isn't working dependencies labels Jan 10, 2022
@eureka-cpu eureka-cpu requested review from adlerjohn and sezna January 10, 2022 16:51
@eureka-cpu eureka-cpu self-assigned this Jan 10, 2022
@eureka-cpu eureka-cpu changed the title Update to the dependency name: Issue 521 Update to the dependency name to avoid download collisions in forc_build. Jan 10, 2022
@eureka-cpu eureka-cpu linked an issue Jan 10, 2022 that may be closed by this pull request
forc uses manifest file name as a cache key for dependencies, but we should use the github URL to avoid collisions. #521
Closed
@eureka-cpu eureka-cpu requested review from emilyaherbert, otrho and canndrew and removed request for emilyaherbert January 11, 2022 02:59
adlerjohn
adlerjohn approved these changes Jan 11, 2022
View reviewed changes
Copy link
Contributor

@adlerjohn adlerjohn left a comment

Choose a reason for hiding this comment

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

I can see the possibility of a malicious dependency making a collision with another by including dashes in the name, but it's probably okay for now. Long-term I would prefer using domain-separated hashing to avoid this possibility.

@eureka-cpu eureka-cpu merged commit 78ad6ea into master Jan 11, 2022
@eureka-cpu eureka-cpu deleted the eureka-cpu/update-download-dep branch January 11, 2022 16:22
# for free to join this conversation on GitHub. Already have an account? # to comment
Reviewers

@adlerjohn adlerjohn adlerjohn approved these changes

@sezna sezna Awaiting requested review from sezna

@otrho otrho Awaiting requested review from otrho

@canndrew canndrew Awaiting requested review from canndrew

Assignees

@eureka-cpu eureka-cpu

Labels
bug Something isn't working
Projects
Fuel Network
Archived in project
Milestone
No milestone
3 participants
@eureka-cpu @adlerjohn @sezna