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

base/v0_6_exp: add parent directory sugar #508

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

prestist
Copy link
Collaborator

@prestist prestist commented Feb 9, 2024

Add a field called 'Parent' which is used to specify a file's parent directory. When a parent is specified, all directories from the parent to the file will be created, with the 'mode' supplied in the parent directory.

@prestist
Copy link
Collaborator Author

prestist commented Feb 9, 2024

still adding unit tests and cleaning up code.

@prestist prestist force-pushed the directories-sugar branch 2 times, most recently from d532226 to 9f25bc1 Compare February 9, 2024 21:34
@prestist
Copy link
Collaborator Author

prestist commented Feb 9, 2024

Ooof, ok

Looking at other tests they correctly have a storage translation set

you can see this here with this TestTranslateMountUnit but for some reason, this one test is .. not? it has all of the translations minus "$.storage" I will need to look into this.

@prestist prestist force-pushed the directories-sugar branch 2 times, most recently from 1451196 to 3125dcd Compare February 16, 2024 23:25
@prestist prestist marked this pull request as ready for review February 16, 2024 23:26
@prestist prestist force-pushed the directories-sugar branch 3 times, most recently from eb5115d to 07d381f Compare February 20, 2024 20:51
@prestist prestist requested a review from jlebon February 21, 2024 21:41
@@ -67,6 +67,12 @@ type File struct {
Append []Resource `yaml:"append"`
Contents Resource `yaml:"contents"`
Mode *int `yaml:"mode"`
Parent Parent `yaml:"parent"`
Copy link
Member

Choose a reason for hiding this comment

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

I think we also need to add this to Directory and Link.

@@ -134,6 +184,22 @@ func translateFile(from File, options common.TranslateOptions) (to types.File, t
return
}

func translateLuks(from Luks, options common.TranslateOptions) (to types.Luks, tm translate.TranslationSet, r report.Report) {
Copy link
Member

Choose a reason for hiding this comment

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

It feels weird to me that we need to rewrite this since IIUC nothing in what we do is related to LUKS (right?).

Comment on lines +128 to +133
translate.MergeP(tr, tm, &r, "disks", &from.Disks, &to.Disks)
translate.MergeP(tr, tm, &r, "files", &from.Files, &to.Files)
translate.MergeP(tr, tm, &r, "filesystems", &from.Filesystems, &to.Filesystems)
translate.MergeP(tr, tm, &r, "links", &from.Links, &to.Links)
translate.MergeP(tr, tm, &r, "luks", &from.Luks, &to.Luks)
translate.MergeP(tr, tm, &r, "raid", &from.Raid, &to.Raid)
Copy link
Member

Choose a reason for hiding this comment

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

Is there precedence for other sugar we have in Butane which ends up having to relist every other section at the level it's operating at?

@yasminvalim yasminvalim force-pushed the directories-sugar branch 6 times, most recently from 63c3086 to da97cc0 Compare June 26, 2024 14:33
@yasminvalim yasminvalim force-pushed the directories-sugar branch 12 times, most recently from 3ea71ea to cba27e1 Compare July 5, 2024 22:13
@prestist prestist force-pushed the directories-sugar branch 2 times, most recently from 63ed6c2 to aa476a4 Compare July 18, 2024 14:18
Add a field called 'Parent' which is used to specify a file's parent
directory. When a parent is specified, all directories from the parent to
the file will be created, with the 'mode' supplied in the parent directory.

resolves: coreos#380

Co-authored-by: Yasmin Valim  <[email protected]>
Co-authored-by: Joseph Corchado <[email protected]>
Co-authored-by: Adam Piasecki <[email protected]>
Co-authored-by: Luke Yang <[email protected]>
Co-authored-by: Renata Ravanelli <[email protected]>
Co-authored-by: Huijing Hei <[email protected]>
@prestist
Copy link
Collaborator Author

prestist commented Jul 19, 2024

@yasminvalim windows weird ci has been fixed, now we can start addressing issues brought up by @jlebon.

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