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

tree-wide: Remove (almost) all remaining rojig bits #2842

Merged
merged 8 commits into from
May 25, 2021

Conversation

cgwalters
Copy link
Member

@cgwalters cgwalters commented May 19, 2021

importer: Remove rojig bits


core: Remove rojig bits

Nothing uses this code now.


daemon/sysroot: Replace some rojig code with g_assert_not_reached()

Prep for removing the APIs entirely.


origin: Remove rojig bits

Nothing calls these now.


rust/origin: Remove rojig bits

Has never been used.


util: Remove rojig cache branch mapping functions

Not used.


rust/treefile: Remove rojig spec writing

One thing I realized is we need to keep the rojig: bit
in the treefile because we're (ab)using it in coreos-assembler
for image naming.

But we don't need the spec file generation bits, so that can go.


tree-wide: Remove (almost) all remaining rojig bits

The only part left that we will need to keep ~forever is
the treefile parsing rojig: because it's used by coreos-assembler.
But all we need is to propagate it into the JSON treefile.


@cgwalters
Copy link
Member Author

I initially started by doing a search for everything referencing RPMOSTREE_REFSPEC_TYPE_ROJIG and deleting code left and right but that ended up feeling like swinging around a chainsaw while surrounded by hungry zombies.

This series uses g_assert_not_reached() where necessary instead, and removes things a bit more bottom up.

@cgwalters cgwalters force-pushed the remove-more-rojig-bits branch from 67b5d08 to 673b320 Compare May 19, 2021 18:58
@cgwalters
Copy link
Member Author

OK actually I went to try removing REFSPEC_TYPE_ROJIG as a followup and in fact on top of this it was quite easy so I went ahead and pushed the last commit here.

@cgwalters cgwalters changed the title Remove more rojig bits tree-wide: Remove (almost) all remaining rojig bits May 19, 2021
@cgwalters cgwalters force-pushed the remove-more-rojig-bits branch from 6c639b7 to 4eef894 Compare May 19, 2021 19:36
jlebon
jlebon previously approved these changes May 19, 2021
Copy link
Member

@jlebon jlebon left a comment

Choose a reason for hiding this comment

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

LGTM!
Needs a rebase.

cgwalters added 8 commits May 19, 2021 17:28
Nothing uses this code now.
Nothing calls these now.
Has never been used.
One thing I realized is we need to keep the `rojig:` bit
in the treefile because we're (ab)using it in coreos-assembler
for image naming.

But we don't need the spec file generation bits, so that can go.
The only part left that we will need to keep ~forever is
the treefile parsing `rojig:` because it's used by coreos-assembler.
But all we need is to propagate it into the JSON treefile.
@cgwalters cgwalters force-pushed the remove-more-rojig-bits branch from c784a86 to 7082822 Compare May 19, 2021 21:29
@cgwalters
Copy link
Member Author

Ah ok, fixed a logic error in the origin code removal.

cgwalters added a commit to cgwalters/coreos-assembler that referenced this pull request May 21, 2021
Conceptually `image.yaml` is the configuration for coreos-assembler
specifically, distinct from `manifest` and `overlay` etc. which
end up being fed into rpm-ostree.

First, this is part of addressing
coreos/rpm-ostree#2842 (comment)
so that e.g. we can migrate the `name:` into this file instead
of the rpm-ostree manifest.

But I also want to hook other things off here, such as whether
to use the new ostree-container format instead of our tar-of-archive-repo.
cgwalters added a commit to cgwalters/coreos-assembler that referenced this pull request May 21, 2021
Conceptually `image.yaml` is the configuration for coreos-assembler
specifically, distinct from `manifest` and `overlay` etc. which
end up being fed into rpm-ostree.

First, this is part of addressing
coreos/rpm-ostree#2842 (comment)
so that e.g. we can migrate the `name:` into this file instead
of the rpm-ostree manifest.

But I also want to hook other things off here, such as whether
to use the new ostree-container format instead of our tar-of-archive-repo.
jlebon pushed a commit to coreos/coreos-assembler that referenced this pull request May 21, 2021
Conceptually `image.yaml` is the configuration for coreos-assembler
specifically, distinct from `manifest` and `overlay` etc. which
end up being fed into rpm-ostree.

First, this is part of addressing
coreos/rpm-ostree#2842 (comment)
so that e.g. we can migrate the `name:` into this file instead
of the rpm-ostree manifest.

But I also want to hook other things off here, such as whether
to use the new ostree-container format instead of our tar-of-archive-repo.
@cgwalters
Copy link
Member Author

Ping on this one, any objections to landing it? Would like to clear some dead wood here for more construction.

Copy link
Member

@jlebon jlebon left a comment

Choose a reason for hiding this comment

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

Whoops sorry, missed your update here!

@jlebon jlebon merged commit f3ccb97 into coreos:main May 25, 2021
kelvinfan001 added a commit to kelvinfan001/rpm-ostree that referenced this pull request Jun 30, 2021
This mainly undoes
coreos@2c270a6,
and follows up on coreos#2842.
Also serves as some cleanup to make way for the introduction of a
new `container-image-reference` refspec type.

We no longer have any use for `ostree://` or `rojig://`-style
prefixes in the refspecs. There had been efforts to "canonicalize"
refspecs to always include such prefixes into the code internally,
but that hasn't gotten too far. Crucially, since such prefixes were
never introduced to libostree, the refspecs in the origin file
were never meant to contain such prefixes anyway.

Following some discussion in
coreos#2940 (comment),
in the future we would disambiguate "refspec type" via the name of
the key in the origin file instead.
kelvinfan001 added a commit to kelvinfan001/rpm-ostree that referenced this pull request Jul 5, 2021
This mainly undoes
coreos@2c270a6,
and follows up on coreos#2842.
Also serves as some cleanup to make way for the introduction of a
new `container-image-reference` refspec type.

We no longer have any use for `ostree://` or `rojig://`-style
prefixes in the refspecs. There had been efforts to "canonicalize"
refspecs to always include such prefixes into the code internally,
but that hasn't gotten too far. Crucially, since such prefixes were
never introduced to libostree, the refspecs in the origin file
were never meant to contain such prefixes anyway.

Following some discussion in
coreos#2940 (comment),
in the future we would disambiguate "refspec type" via the name of
the key in the origin file instead.
kelvinfan001 added a commit to kelvinfan001/rpm-ostree that referenced this pull request Jul 6, 2021
This mainly undoes
coreos@2c270a6,
and follows up on coreos#2842.
Also serves as some cleanup to make way for the introduction of a
new `container-image-reference` refspec type.

We no longer have any use for `ostree://` or `rojig://`-style
prefixes in the refspecs. There had been efforts to "canonicalize"
refspecs to always include such prefixes into the code internally,
but that hasn't gotten too far. Crucially, since such prefixes were
never introduced to libostree, the refspecs in the origin file
were never meant to contain such prefixes anyway.

Following some discussion in
coreos#2940 (comment),
in the future we would disambiguate "refspec type" via the name of
the key in the origin file instead.
kelvinfan001 added a commit to kelvinfan001/rpm-ostree that referenced this pull request Jul 6, 2021
This mainly undoes
coreos@2c270a6,
and follows up on coreos#2842.
Also serves as some cleanup to make way for the introduction of a
new `container-image-reference` refspec type.

We no longer have any use for `ostree://` or `rojig://`-style
prefixes in the refspecs. There had been efforts to "canonicalize"
refspecs to always include such prefixes into the code internally,
but that hasn't gotten too far. Crucially, since such prefixes were
never introduced to libostree, the refspecs in the origin file
were never meant to contain such prefixes anyway.

Following some discussion in
coreos#2940 (comment),
in the future we would disambiguate "refspec type" via the name of
the key in the origin file instead.
cgwalters pushed a commit that referenced this pull request Jul 10, 2021
This mainly undoes
2c270a6,
and follows up on #2842.
Also serves as some cleanup to make way for the introduction of a
new `container-image-reference` refspec type.

We no longer have any use for `ostree://` or `rojig://`-style
prefixes in the refspecs. There had been efforts to "canonicalize"
refspecs to always include such prefixes into the code internally,
but that hasn't gotten too far. Crucially, since such prefixes were
never introduced to libostree, the refspecs in the origin file
were never meant to contain such prefixes anyway.

Following some discussion in
#2940 (comment),
in the future we would disambiguate "refspec type" via the name of
the key in the origin file instead.
ravanelli pushed a commit to ravanelli/coreos-assembler that referenced this pull request Aug 25, 2021
Conceptually `image.yaml` is the configuration for coreos-assembler
specifically, distinct from `manifest` and `overlay` etc. which
end up being fed into rpm-ostree.

First, this is part of addressing
coreos/rpm-ostree#2842 (comment)
so that e.g. we can migrate the `name:` into this file instead
of the rpm-ostree manifest.

But I also want to hook other things off here, such as whether
to use the new ostree-container format instead of our tar-of-archive-repo.
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