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.

Sign up for GitHub

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

Jump to bottom

Bug: forge does not add tag information into gitmodules file on install #7225

Open
2 tasks done
sambacha opened this issue Feb 23, 2024 · 7 comments · Fixed by #9453 · May be fixed by #9522
Open
2 tasks done

Bug: forge does not add tag information into gitmodules file on install #7225

sambacha opened this issue Feb 23, 2024 · 7 comments · Fixed by #9453 · May be fixed by #9522
Assignees
@yash-atreya
Labels
Cmd-forge-pm Command: forge install/update/remove P-high Priority: high T-bug Type: bug
Milestone
v1.0.0

Comments

@sambacha
Copy link
Contributor

Component

Forge

Have you ensured that all of these are up to date?

  • Foundry
  • Foundryup

What version of Foundry are you on?

forge 0.2.0 (a436a0d 2024-02-20T00:25:57.253479000Z) && forge 0.2.0 (6d5de51 2024-02-23T00:19:38.512335000Z)

What command(s) is the bug in?

forge install

Operating System

macOS (Apple Silicon)

Describe the bug

Running this install, solidstate-network/[email protected]

$  forge install solidstate-network/[email protected]
Installing solidstate-solidity in /Users/janitor/rico/lib/solidstate-solidity (url: Some("https://github.com/solidstate-network/solidstate-solidity"), tag: Some("v0.0.59"))
Cloning into '/Users/janitor/rico/lib/solidstate-solidity'...
remote: Enumerating objects: 17261, done.
remote: Counting objects: 100% (3206/3206), done.
remote: Compressing objects: 100% (671/671), done.
remote: Total 17261 (delta 2572), reused 3007 (delta 2506), pack-reused 14055
Receiving objects: 100% (17261/17261), 4.06 MiB | 5.31 MiB/s, done.
Resolving deltas: 100% (12427/12427), done.
    Installed solidstate-solidity v0.0.59

After we inspect .gitmodules and find

[submodule "lib/solidstate-solidity"]
        path = lib/solidstate-solidity
        url = https://github.com/solidstate-network/solidstate-solidity

However we want the .gitmodules file to reflect this correct configuration

[submodule "lib/solidstate-solidity"]
        path = lib/solidstate-solidity
        url = https://github.com/solidstate-network/solidstate-solidity
+       branch = v0.0.59
@sambacha sambacha added the T-bug Type: bug label Feb 23, 2024
@sambacha
Copy link
Contributor Author

Confirmed just in case

$ forge install foundry-rs/[email protected]  
Installing forge-std in /Users/janitor/tstorish/lib/forge-std (url: Some("https://github.com/foundry-rs/forge-std"), tag: Some("v1.7.6"))  
Cloning into '/Users/janitor/tstorish/lib/forge-std'...  
remote: Enumerating objects: 2168, done.  
remote: Counting objects: 100% (2164/2164), done.  
remote: Compressing objects: 100% (722/722), done.  
remote: Total 2168 (delta 1436), reused 2070 (delta 1375), pack-reused 4  
Receiving objects: 100% (2168/2168), 610.57 KiB | 2.81 MiB/s, done.  
Resolving deltas: 100% (1436/1436), done.  
Submodule 'lib/ds-test' (https://github.com/dapphub/ds-test) registered for path 'lib/ds-test'  
Cloning into '/Users/janitor/tstorish/lib/forge-std/lib/ds-test'...  
remote: Enumerating objects: 313, done.  
remote: Counting objects: 100% (171/171), done.  
remote: Compressing objects: 100% (79/79), done.  
remote: Total 313 (delta 91), reused 132 (delta 83), pack-reused 142  
Receiving objects: 100% (313/313), 71.35 KiB | 1.32 MiB/s, done.  
Resolving deltas: 100% (130/130), done.  
   Installed forge-std v1.7.6  
01:59:55 Sat Feb 24 2024 janitor macbook /Users/janitor/tstorish feat/simple-tests
$ cat .gitmodules
[submodule "lib/forge-std"]
path = lib/forge-std
url = https://github.com/foundry-rs/forge-std

@klkvr
Copy link
Member

klkvr commented Feb 26, 2024

branch field can only contain a branch name in the specified repository, not a tag name.

Currently when installing submodule, forge tries to find a branch with the same name as the tag, and only if it's found, the branch field is populated

foundry/crates/forge/bin/cmd/install.rs

Lines 345 to 350 in 474591a

if std::io::stdout().is_terminal() {
if tag.is_empty() {
tag = self.match_tag(&tag, path)?;
} else if let Some(branch) = self.match_branch(&tag, path)? {
trace!(?tag, ?branch, "selecting branch for given tag");
tag = branch;

@sambacha
Copy link
Contributor Author

sambacha commented Feb 26, 2024 •
edited
Loading

branch field can only contain a branch name in the specified repository, not a tag name.

Currently when installing submodule, forge tries to find a branch with the same name as the tag, and only if it's found, the branch field is populated

foundry/crates/forge/bin/cmd/install.rs

Lines 345 to 350 in 474591a

if std::io::stdout().is_terminal() {
if tag.is_empty() {
tag = self.match_tag(&tag, path)?;
} else if let Some(branch) = self.match_branch(&tag, path)? {
trace!(?tag, ?branch, "selecting branch for given tag");
tag = branch; 

$ forge install -h  
Install one or multiple dependencies

Usage: forge install \[OPTIONS\] \[DEPENDENCIES\]...  
   forge install \[OPTIONS\] \<github username>/\<github project>@\<tag>...  
   forge install \[OPTIONS\] \<alias>=\<github username>/\<github project>@\<tag>...  
   forge install \[OPTIONS\] \<https:// git url>...

$ forge install /transmissions11/solmate@c892309933b25c03d32b1b0d674df7ae292ba925

Resolving deltas: 100% (130/130), done.  
   Installed solmate c892309933b25c03d32b1b0d674df7ae292ba925  
08:11:40 Mon Feb 26 2024 janitor macbook /Users/janitor/solmate-tmp master  
$ cat .gitmodules  
\[submodule "lib/forge-std"\]  
path = lib/forge-std  
url = https://github.com/foundry-rs/forge-std  
\[submodule "lib/solmate"\]  
path = lib/solmate  
url = [https://github.com//transmissions11/solmate](https://github.com//transmissions11/solmate)

Forge uses 'tag' to mean any git commit hash, the behaviour is true in that regard. As you mention git submodules does not explicitly support proper git tags, however this is just really an alias for some git commit hash really.

@zerosnacks
Copy link
Member

zerosnacks commented Jul 11, 2024 •
edited
Loading

Hi @sambacha, thanks for reporting this

Confirming this issue is still active (cc @klkvr)

I've created a minimal reproduction here: https://github.com/zerosnacks/foundry-bug-7225-repro that has a practical example people run into relating to the 0.8 branch of Uniswap V3 + forge update.

@zerosnacks zerosnacks added the Cmd-forge-pm Command: forge install/update/remove label Jul 11, 2024
@zerosnacks zerosnacks added this to the v1.0.0 milestone Jul 26, 2024
@zerosnacks zerosnacks mentioned this issue Oct 2, 2024
Merged
@zerosnacks zerosnacks added the P-high Priority: high label Oct 2, 2024
@zerosnacks zerosnacks self-assigned this Oct 2, 2024
@github-project-automation github-project-automation bot moved this to Todo in Foundry Oct 2, 2024
@zerosnacks zerosnacks mentioned this issue Oct 2, 2024
Closed
2 tasks
@zerosnacks
Copy link
Member

Ref: #3720

@zerosnacks zerosnacks removed their assignment Oct 21, 2024
@grandizzy
Copy link
Collaborator

we're going to add --branch/--tag args with #5996 and then use them to pin it to specific branch.

@grandizzy grandizzy added the T-blocked Type: blocked label Oct 24, 2024
@zerosnacks zerosnacks mentioned this issue Oct 28, 2024
Merged
@grandizzy grandizzy removed the T-blocked Type: blocked label Nov 14, 2024
@yash-atreya yash-atreya mentioned this issue Dec 2, 2024
Merged
@yash-atreya yash-atreya self-assigned this Dec 2, 2024
@yash-atreya yash-atreya moved this from Todo to Ready For Review in Foundry Dec 2, 2024
@github-project-automation github-project-automation bot moved this from Ready For Review to Done in Foundry Dec 3, 2024
@yash-atreya yash-atreya reopened this Dec 3, 2024
@yash-atreya
Copy link
Member

Reopened as #9453 doesn't address the tag/rev issue

@yash-atreya yash-atreya linked a pull request Dec 9, 2024 that will close this issue
Open
4 tasks
@yash-atreya yash-atreya moved this from Done to In Progress in Foundry Dec 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@yash-atreya yash-atreya

Labels
Cmd-forge-pm Command: forge install/update/remove P-high Priority: high T-bug Type: bug
Projects
Foundry
Status: In Progress
Milestone
v1.0.0
5 participants
@sambacha @grandizzy @yash-atreya @klkvr @zerosnacks