Skip to content

pkglib: put real pkg-build-arg-hash into pkg hash#4176

Draft
christoph-zededa wants to merge 1 commit intolinuxkit:masterfrom
christoph-zededa:fix_rel_hash_dependency
Draft

pkglib: put real pkg-build-arg-hash into pkg hash#4176
christoph-zededa wants to merge 1 commit intolinuxkit:masterfrom
christoph-zededa:fix_rel_hash_dependency

Conversation

@christoph-zededa
Copy link
Contributor

when using

buildArgs:
  - REL_HASH_%=@lkt:pkgs:../*

then exactly this string is put into the pkg hash, but if one of these packages are updated, the pkg hash is not updated, as the string is static.

Instead put in the real pkg-build-arg-hash so that even if a pkg is updated, it's descendants end up with a new hash.

- What I did

Include "real" build-args hashes in to pkg hash.

- How I did it

- How to verify it

Change a pkg and then build a pkg (with REL_HASH_%=@lkt:pkgs:../*); check that it really gets built again.

- Description for the changelog
Include "real" build-args hashes in to pkg hash.

- A picture of a cute animal (not mandatory but encouraged)
ChatGPT Image Sep 22, 2025, 02_05_46 PM

@christoph-zededa christoph-zededa marked this pull request as draft September 22, 2025 12:06
when using
```
buildArgs:
  - REL_HASH_%=@lkt:pkgs:../*
```
then exactly this string is put into the pkg hash, but
if one of these packages are updated, the pkg hash is
not updated, as the string is static.

Instead put in the real pkg-build-arg-hash so that even if
a pkg is updated, it's descendants end up with a new hash.

Signed-off-by: Christoph Ostarek <christoph@zededa.com>

buildArgKey := fmt.Sprintf("buildArg=%s", buildArg)
pkgHash += fmt.Sprintf("%x", sha1.Sum([]byte(buildArgKey)))
pkgHash = fmt.Sprintf("%x", sha1.Sum([]byte(pkgHash)))
Copy link
Contributor Author

Choose a reason for hiding this comment

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

IMO this should be optimized, f.e. filter out duplicates if needed and sort alphabetically.

@deitch
Copy link
Collaborator

deitch commented Sep 25, 2025

Can you explain the logic here a bit, using words (as opposed to code)? The way it works now, the logic for determining the tag is here, where it basically gets the tree hash. If the files are dirty, then it appends dirty and also calculates the content hash. The builds args themselves are not included directly, only by virtue of being in a file in the directory.

What does this change in that process?

@christoph-zededa
Copy link
Contributor Author

What does this change in that process?

Ideally I would also have put these changes around https://github.com/linuxkit/linuxkit/blob/master/src/cmd/linuxkit/pkglib/pkglib.go#L243-L265, but ProcessBuildArgs() needs pkg, so I run it after pkg has been created.
Once I have the build args, I iterate over these and do a similar hashing as linuxkit already does in https://github.com/linuxkit/linuxkit/blob/master/src/cmd/linuxkit/pkglib/pkglib.go#L249 and the following line.

Finally it does this template thing that was originally here: https://github.com/linuxkit/linuxkit/blob/master/src/cmd/linuxkit/pkglib/pkglib.go#L243-L265
and then it sets pkg.tag = tag

@deitch
Copy link
Collaborator

deitch commented Sep 25, 2025

The implementation is a bit off (it only runs the template if buildArgs are not nil), but it looks like you are changing the pkgHash on the Pkg struct rather than in the code?

More fundamentally, you are including the build args as part of the hash, whereas currently it is just the git ls-tree output. Sure, if it is dirty we add -dirty- and then the git contents hashed, but we don't modify that first, primary digest in any way.

This whol REL_* is turning out to create new issues. At heart, you no longer can look at the directory and know if it has changed!

@christoph-zededa
Copy link
Contributor Author

it only runs the template if buildArgs are not nil

this is the code:

           if pkg.buildArgs != nil {                                                                                                                                          
               for _, buildArg := range *pkg.buildArgs {                                                                                                                      
                   fmt.Printf("add buildArg '%s' to hash\n", buildArg)                                                                                                        
                                                                                                                                                                              
                   buildArgKey := fmt.Sprintf("buildArg=%s", buildArg)                                                                                                        
                   pkgHash += fmt.Sprintf("%x", sha1.Sum([]byte(buildArgKey)))                                                                                                
                   pkgHash = fmt.Sprintf("%x", sha1.Sum([]byte(pkgHash)))                                                                                                     
               }                                                                                                                                                              
           }                                                                                                                                                                  
           // calculate the tag to use based on the template and the pkgHash                                                                                                  
           tmpl, err := template.New("tag").Parse(tagTmpl)                                                                                                                    
           if err != nil {                                                                                                                                                    
               return nil, fmt.Errorf("invalid tag template: %v", err)                                                                                                        
           }                                                                                                                                                                  
           var buf bytes.Buffer                                                                                                                                               
           if err := tmpl.Execute(&buf, map[string]string{"Hash": pkgHash}); err != nil {                                                                                     
               return nil, fmt.Errorf("failed to execute tag template: %v", err)                                                                                              
           }                                                                                                                                                                  
           tag := buf.String()                                                                                                                                                
           pkg.tag = tag                                          

if the buildArgs are nil, then only

           if pkg.buildArgs != nil {                                                                                                                                          
               for _, buildArg := range *pkg.buildArgs {                                                                                                                      
                   fmt.Printf("add buildArg '%s' to hash\n", buildArg)                                                                                                        
                                                                                                                                                                              
                   buildArgKey := fmt.Sprintf("buildArg=%s", buildArg)                                                                                                        
                   pkgHash += fmt.Sprintf("%x", sha1.Sum([]byte(buildArgKey)))                                                                                                
                   pkgHash = fmt.Sprintf("%x", sha1.Sum([]byte(pkgHash)))                                                                                                     
               }                                                                                                                                                              
           }                                                                                                                                                                  
            

is not running, the rest, i.e.:

           // calculate the tag to use based on the template and the pkgHash                                                                                                  
           tmpl, err := template.New("tag").Parse(tagTmpl)                                                                                                                    
           if err != nil {                                                                                                                                                    
               return nil, fmt.Errorf("invalid tag template: %v", err)                                                                                                        
           }                                                                                                                                                                  
           var buf bytes.Buffer                                                                                                                                               
           if err := tmpl.Execute(&buf, map[string]string{"Hash": pkgHash}); err != nil {                                                                                     
               return nil, fmt.Errorf("failed to execute tag template: %v", err)                                                                                              
           }                                                                                                                                                                  
           tag := buf.String()                                                                                                                                                
           pkg.tag = tag                        

still is executed.

@christoph-zededa
Copy link
Contributor Author

christoph-zededa commented Sep 25, 2025

it looks like you are changing the pkgHash on the Pkg struct rather than in the code?

the struct is code - I do not understand, can you elaborate a bit more?
You want ProcessBuildArgs to be refactored so that it does not need Pkg?

@christoph-zededa
Copy link
Contributor Author

christoph-zededa commented Sep 25, 2025

This whol REL_* is turning out to create new issues.

This might be the last one ;-)

Does it help if I ask ChatGPT to create an even cuter animal image?

At heart, you no longer can look at the directory and know if it has changed!

Even in the past you could not, because if there is an ADD https://github.com/golang/go.git /usr/src/go-latest then it depends on things outside of the git checkout.

@deitch
Copy link
Collaborator

deitch commented Sep 25, 2025

You want ProcessBuildArgs to be refactored so that it does not need Pkg?

No, I am commenting on how it was done, not on whether or not it was good.

@deitch
Copy link
Collaborator

deitch commented Sep 25, 2025

Does it help if I ask ChatGPT to create an even cuter animal image?

Nah, make it something interesting and not believable. "ChatGPT" give me an image of an animal that might appear in the Halo video game franchise.

@deitch
Copy link
Collaborator

deitch commented Sep 25, 2025

Even in the past you could not, because if there is an ADD https://github.com/golang/go.git /usr/src/go-latest then it depends on things outside of the git checkout

I would argue that you should use ADD https://github.com/golang/go.git#12345abcd and get the commit, but I agree, the tool would not (and could not) enforce that. But there, we just accepted that it would not change. Hence --force to cover cases where you just had to build it anyways.

I need to think about this for a while. We are changing the definition of the hash, which is not something to be taken lightly. Sure, the tools all will get it, like pkg show-tag and build etc., but it really does change it.

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.

2 participants