Lossy configuration on release builds is a bad default

[emilazy]

Current behavior 😯

gitoxide/gix/src/config/cache/util.rs

Lines 19 to 25 in d0ef276

	pub(crate) fn base_options(lossy: Option<bool>, lenient: bool) -> gix_config::file::init::Options<'static> {
	gix_config::file::init::Options {
	lossy: lossy.unwrap_or(!cfg!(debug_assertions)),
	ignore_io_errors: lenient,
	..Default::default()
	}
	}

We just ran into our tests failing in Jujutsu only on release builds, because our snapshots of the generated configuration suddenly lost all their whitespace. I was briefly wondering if some kind of bizarre compiler bug might be involved.

Expected behavior 🤔

Please reconsider this behaviour!

Release builds should not affect functionality, only performance and the presence of debug checks. Any behavioural change should be handled through feature flags. (From a quick search,

gitoxide/gix-transport/src/client/blocking_io/http/mod.rs

Lines 300 to 305 in d0ef276

	#[cfg(not(debug_assertions))]
	if self.url.starts_with("http://") {
	return Err(client::Error::AuthenticationRefused(
	"Will not send credentials in clear text over http",
	));
	}

also looks questionable from this perspective.)

But really, I think modifying repository configuration with gix and writing it back out again should not silently throw away user data in general, even with a feature flag. If it affects lookup performance I think using a different data structure is the correct approach, not stripping out whitespace and comments.

Git behavior

Git doesn’t clobber configuration in this way.

Steps to reproduce 🕹

https://gist.github.com/demize/8a468be11b3b19069efca37323c352a2#file-minimum-reproducible-example-rs is not a minimal reproducer (just taking a snapshot and writing it back out suffices), but it works to demonstrate the problem.

[Byron]

Thanks for bringing this to my attention. It's one of these late-night ideas that somehow stuck and survived till now.

Would you want to remove this special case or get its removal started in a PR? If it turns out to be more work, I can take it from there.

Please beware of slow(er) responses these days, I am looking at a massive pile of email + backlog from before that after 7 days "off".

[emilazy]

I’ve opened #1882. Thanks for being open to a change here and sorry for my somewhat provocative report; I was very surprised when we tracked down this failure and felt the need to put some of my strength of feeling into it :)

[Byron]

No worries at all, I kind of failed to recognise any provocation, or else I would have been more sympathetic in my reply. After all, I am quite aware of the pain merely by observing some of it first hand when threading on new grounds as a user of the crate. Some portions are a delight, many are not, unfortunately.

And now I hope there isn't any more gems like this hidden in the codebase 😅.