zabbly / incus

Incus package repository
208 stars 16 forks source link

Effects of sed commands should be moved to patches for easier updating #28

Closed mkg20001 closed 7 months ago

mkg20001 commented 7 months ago

We've had issues in nixos with iso uploads: https://github.com/NixOS/nixpkgs/issues/294376

The solution was to sed over the files: https://github.com/NixOS/nixpkgs/pull/294497/files

This was because we missed some sed commands.

stgraber commented 7 months ago

I'm confused, aren't the files you're sedding the exact same ones that we are? https://github.com/zabbly/incus/blob/daily/.github/workflows/builds.yml#L330

cmspam commented 7 months ago

I am the person who submitted the issue and the pull request to sed over the files.

Just to clarify, the sed commands were taken directly from this repository, as stated in the pull request. To clarify further, the Nix implementation was applying all the patches from zabbly repository, but was not running the sed commands you can see in the builds.yml file linked above by @stgraber which caused broken functionality in the Nix implementation. My fix was to add those sed commands.

Is what @mkg20001 refers to, is that the changes made with the sed commands, should be implemented in patch files instead, or something like that?

mkg20001 commented 7 months ago

The idea was to move the sed commands to the patch files. I didn't know they were from here, but also having them in patch files makes things better for us (package people), since those commands in the github workflow slipped under my radar.

One idea would be to just have a script that generates an additional patch file that's put into git.

mkg20001 commented 7 months ago

Is what @mkg20001 refers to, is that the changes made with the sed commands, should be implemented in patch files instead, or something like that?

Yes

stgraber commented 7 months ago

Ah right, so no, we'd rather stick to sed as 99% of the time, the same patterns will work on newer versions of the UI.

Whereas patches basically need manual refreshing every time a new version comes out as simple things like indentation changes will break them.

mkg20001 commented 7 months ago

How about putting the commands into a script, that the github workflow and our packaging can call (preferrably one using set -e)? In case something would need to be changed, since that is easier than analyzing the diff each time.

Stéphane Graber @.***> schrieb am Sa., 9. März 2024, 19:12:

Ah right, so no, we'd rather stick to sed as 99% of the time, the same patterns will work on newer versions of the UI.

Whereas patches basically need manual refreshing every time a new version comes out as simple things like indentation changes will break them.

— Reply to this email directly, view it on GitHub https://github.com/zabbly/incus/issues/28#issuecomment-1986937940, or unsubscribe https://github.com/notifications/unsubscribe-auth/AB3AO2MCVTKWSHX3NRYOQ7TYXNGKBAVCNFSM6AAAAABEOITU2KVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTSOBWHEZTOOJUGA . You are receiving this because you were mentioned.Message ID: @.***>

stgraber commented 7 months ago

Yeah, we can move those into a separate sed script.