On 10/02/2011 04:22 PM, Brian Harring wrote:
> On Sun, Oct 02, 2011 at 02:10:09PM -0700, Zac Medico wrote:
>> On 10/02/2011 01:54 PM, Robin H. Johnson wrote:
>>> On Sun, Oct 02, 2011 at 01:39:41PM -0700, Zac Medico wrote:
>>>> On 10/02/2011 05:46 AM, Robin H. Johnson wrote:
>>>>> On Sat, Oct 01, 2011 at 09:40:13PM -0700, Zac Medico wrote:
>>>>>> If we control these hashes via metadata/layout.conf, then we can toggle
>>>>>> it atomically for all commiters. Otherwise, we'll have an annoying
>>>>>> period of time where different committers are committing different sets
>>>>>> of hashes, depending on their portage version.
>>>>> How do you suggest doing it via layout.conf? I've kept SHA256 in both
>>>>> sets for now, but if you could enforce new signatures including both
>>>>> WHIRLPOOL and SHA256, that would be great.
>>>> How about if we put something like this in
>>>> gentoo-x86/metadata/layout.conf now:
>>> Did you mean profiles/layout.conf? I just want to make sure no scripts
>>> that pull from CVS and expect that dir to not exist don't break.
>>
>> No, it's metadata/layout.conf. I didn't choose the location. We actually
>> inherited it from paludis about 1.5 years ago:
>>
>>
>> http://git.overlays.gentoo.org/gitweb/?p=proj/portage.git;a=commit;h=f16aee82cefa95e9903fa46f448 d30f6d4350f64
>>
>> We're also using it to control thin-manifest support, among other things
>> now:
>>
>> https://bugs.gentoo.org/show_bug.cgi?id=333691
>>
>>>> manifest2-sha1 = true
>>>> manifest2-whirlpool = false
>>> Bikeshedding slightly, but can we figure something like a list or dict
>>> instead? (Also gives us a chance to make the required hashes a list).
>>> manifest2-hashes = ['SHA1', 'SHA256', 'RMD160']
>>
>> Well, booleans are simpler. Also, note that I designed them to be
>> removed from layout.conf eventually, which means that we will accumulate
>> less bloat in layout.conf over time.

I've implemented it with booleans in this commit:

http://git.overlays.gentoo.org/gitweb/?p=proj/portage.git;a=commit;h=c8cd3a985cc529299411d7343a1 1004b7d1330ef

> Should use a space delimited list instead named hashes instead; those
> being the hashes that should be generated, and that can be /used/.
> Not in the list, not an acceptable hash (even if a manifest2 carries
> that data).

Why? Boolean flags are simpler and they work.

> If it's not set, then the pm defaults in a list; that default list
> should be tracked somewhere (rather than just whatever the PM author
> decides) also, although that's a seperate discussion.

Sure, it could be added to PMS or something.

> Breaking it out into individual booleans isn't particularly great; we
> use lists for masters, a tristate for use-manifest, etc. Having each
> CHF controlled by a seperate boolean adds more toggles than is worth

You can group them into a dictionary, like I did.

> it imo, and having the manifest2- prefix makes the parsing slightly
> more complex while also making the key name a bit daft if we ever
> switch to a manifest3.

I made it manifest- instead.
--
Thanks,
Zac

On 10/02/2011 05:21 PM, Zac Medico wrote:
> On 10/02/2011 04:22 PM, Brian Harring wrote:
>> On Sun, Oct 02, 2011 at 02:10:09PM -0700, Zac Medico wrote:
> I've implemented it with booleans in this commit:
>
> http://git.overlays.gentoo.org/gitweb/?p=proj/portage.git;a=commit;h=c8cd3a985cc529299411d7343a1 1004b7d1330ef
>
>> Should use a space delimited list instead named hashes instead; those
>> being the hashes that should be generated, and that can be /used/.
>> Not in the list, not an acceptable hash (even if a manifest2 carries
>> that data).
>
> Why? Boolean flags are simpler and they work.

After some thought, I like the space delimited approach better. Here's
the patch, which retains the ability to remove the manifest hash
settings from layout.conf after they become redundant:

http://git.overlays.gentoo.org/gitweb/?p=proj/portage.git;a=commit;h=d9d0606fe01618cc81fb0b862ad a91149dad3746
--
Thanks,
Zac

On Mon, Oct 03, 2011 at 02:48:55AM -0700, Zac Medico wrote:
> On 10/02/2011 05:21 PM, Zac Medico wrote:
> > On 10/02/2011 04:22 PM, Brian Harring wrote:
> >> On Sun, Oct 02, 2011 at 02:10:09PM -0700, Zac Medico wrote:
> > I've implemented it with booleans in this commit:
> >
> > http://git.overlays.gentoo.org/gitweb/?p=proj/portage.git;a=commit;h=c8cd3a985cc529299411d7343a1 1004b7d1330ef
> >
> >> Should use a space delimited list instead named hashes instead; those
> >> being the hashes that should be generated, and that can be /used/.
> >> Not in the list, not an acceptable hash (even if a manifest2 carries
> >> that data).
> >
> > Why? Boolean flags are simpler and they work.
>
> After some thought, I like the space delimited approach better. Here's
> the patch, which retains the ability to remove the manifest hash
> settings from layout.conf after they become redundant:
>
> http://git.overlays.gentoo.org/gitweb/?p=proj/portage.git;a=commit;h=d9d0606fe01618cc81fb0b862ad a91149dad3746

Suggest you go through the implementation a bit closer; quick look, if
the repo no longer uses what portage considers a required hash (atm,
sha1), it still will force it in; while that's rather annoying for
manifest creation, the validation logic there strikes me as probably
being buggy for that case.

~harring

On 10/03/2011 04:43 AM, Brian Harring wrote:
> On Mon, Oct 03, 2011 at 02:48:55AM -0700, Zac Medico wrote:
>> After some thought, I like the space delimited approach better. Here's
>> the patch, which retains the ability to remove the manifest hash
>> settings from layout.conf after they become redundant:
>>
>> http://git.overlays.gentoo.org/gitweb/?p=proj/portage.git;a=commit;h=d9d0606fe01618cc81fb0b862ad a91149dad3746
>
> Suggest you go through the implementation a bit closer; quick look, if
> the repo no longer uses what portage considers a required hash (atm,
> sha1), it still will force it in; while that's rather annoying for
> manifest creation, the validation logic there strikes me as probably
> being buggy for that case.

This case is only supposed to come up if the user is generating
manifests with a version of portage that has become obsolete for this
type of operation on the repository. I suppose that we could simply make
Manifest.write() raise an exception in this case, and make digestgen()
return unsuccessfully with a suitable error message.

We could also add a layout.conf setting to override
MANIFEST2_REQUIRED_HASH, but that seems unnecessary as long as we stick
to the plan:

After WHIRLPOOL is supported in stable portage:
- Add SHA256 and WHIRLPOOL to MANIFEST2_HASH_DEFAULTS.
- Remove SHA1 and RMD160 from MANIFEST2_HASH_*.
- Set manifest-hashes in gentoo-x86/metadata/layout.conf as follows:
manifest-hashes = SHA256 SHA512 WHIRLPOOL

After WHIRLPOOL is supported in stable portage for at least 1 year:
- Change MANIFEST2_REQUIRED_HASH to WHIRLPOOL.
- Remove SHA256 from MANIFEST2_HASH_*.
- Set manifest-hashes in gentoo-x86/metadata/layout.conf as follows:
manifest-hashes = SHA512 WHIRLPOOL

After SHA-3 is approved:
- Add new hashes to MANIFEST2_HASH_*.

After SHA-3 is supported in stable portage:
- Set manifest-hashes in gentoo-x86/metadata/layout.conf as follows:
manifest-hashes = SHA3 SHA512 WHIRLPOOL

After layout.conf settings correspond to defaults in stable portage:
- Remove redundant settings from gentoo-x86/metadata/layout.conf.
--
Thanks,
Zac