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

Add to new source format and the mandatory signed-by #239390

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

bhack
Copy link

@bhack bhack commented Feb 1, 2025

fixes #238697

@Nichebiche
Copy link

fixes #238697

@deepak1556 deepak1556 assigned rzhao271 and unassigned deepak1556 Feb 3, 2025
@deepak1556 deepak1556 added this to the February 2025 milestone Feb 3, 2025
@Nichebiche
Copy link

@bhack bhack requested a review from 818Nawaf February 3, 2025 19:42
@wcbing
Copy link

wcbing commented Feb 4, 2025

Your method uses the modernize-sources subcommand to check, but several versions will prompt without this subcommand. So, I think it's better to determine it by the apt version number.

In fact, even Debian and Ubuntu versions that are not EOL now support DEB822 sources, so perhaps there's no need for the check.


You also need to modify the if-elif blocks on here to facilitate migration from old repository, or else they will conflict.

@wcbing
Copy link

wcbing commented Feb 4, 2025

A digression: If possible, squash your formatting changes into the earlier commits. The current commit history doesn't look very clean. 👀

@bhack
Copy link
Author

bhack commented Feb 4, 2025

but several versions will prompt without this subcommand

I have not bisected this what is the starting point of this notification?

In my experience It seems to me very connected to modernize introduction.

@wcbing
Copy link

wcbing commented Feb 4, 2025

The missing Signed-By warning and migration consideration started with version 2.9.24, while the modernize-sources subcommand was added in 2.9.26.

In fact, even Debian and Ubuntu versions that are not EOL now support DEB822 sources, so perhaps there's no need for the check.

Do you think this is right?

@bhack
Copy link
Author

bhack commented Feb 4, 2025

I suppose it was introduced since APT 1.1

@bhack
Copy link
Author

bhack commented Feb 4, 2025

I will wait for the vscode team's official feedback about what kind of behaviour we want to adopt.

@wcbing
Copy link

wcbing commented Feb 4, 2025

You did not change the gpg key location or embed it in the sources file as I mentioned.

@bhack
Copy link
Author

bhack commented Feb 4, 2025

You did not change the gpg key location or embed it in the sources file as I mentioned.

Cause I think this will require to handle also the removal case of the existing key.

So I think this could be introduced eventually in another PR.

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.

VS Code APT sources.list: add signed-by field and remove unnecessary architectures
6 participants