Message ID | d75780e0567b5f765816ab7522afe550ebaa3521.1674849963.git.gitgitgadget@gmail.com (mailing list archive) |
---|---|
State | Accepted |
Commit | dea63088928cde2fd264a852a9b14c05178e0838 |
Headers | show |
Series | Allow scalar to succeed despite maintenance failures | expand |
"Derrick Stolee via GitGitGadget" <gitgitgadget@gmail.com> writes: > From: Derrick Stolee <derrickstolee@github.com> > > A user reported issues with 'scalar clone' and 'scalar register' when > working in an environment that had locked down the ability to run > 'crontab' or 'systemctl' in that those commands registered as _failures_ > instead of opportunistically reporting a success with just a warning > about background maintenance. > > As a workaround, they can use GIT_TEST_MAINT_SCHEDULER to fake a > successful background maintenance, but this is not a viable strategy for > long-term. > > Update 'scalar register' and 'scalar clone' to no longer fail by > modifying register_dir() to only warn when toggle_maintenance(1) fails. > > Since background maintenance is a "nice to have" and not a requirement > for a working repository, it is best to move this from hard error to > gentle warning. Wasn't the main selling point of scalar that users do not have to worry about various minute details of configuration settings to maintain their clone of projects on the larger side? The "maintain their clone" certainly should include running periodic maintenance tasks without them having to worry about it. It feels like this is calling for an explicit "disable periodic maintenance tasks in this repository" option to help these esoteric environments that disable cron-like system services, while keeping the default safer, i.e. fail loudly when the periodic maintenance tasks that the users expect to happen cannot be enabled, or something. Perhaps I am not the primary audience, but hmph, I have a feeling that this is not exactly going into a healthy direction. Other two steps that led to this step looked quite sensible, though. Thanks.
Derrick Stolee via GitGitGadget wrote: > From: Derrick Stolee <derrickstolee@github.com> > > A user reported issues with 'scalar clone' and 'scalar register' when > working in an environment that had locked down the ability to run > 'crontab' or 'systemctl' in that those commands registered as _failures_ > instead of opportunistically reporting a success with just a warning > about background maintenance. > > As a workaround, they can use GIT_TEST_MAINT_SCHEDULER to fake a > successful background maintenance, but this is not a viable strategy for > long-term. > > Update 'scalar register' and 'scalar clone' to no longer fail by > modifying register_dir() to only warn when toggle_maintenance(1) fails. > > Since background maintenance is a "nice to have" and not a requirement > for a working repository, it is best to move this from hard error to > gentle warning. > > Signed-off-by: Derrick Stolee <derrickstolee@github.com> > --- > scalar.c | 2 +- > t/t9210-scalar.sh | 4 ++-- > t/t9211-scalar-clone.sh | 4 ++-- > 3 files changed, 5 insertions(+), 5 deletions(-) > > diff --git a/scalar.c b/scalar.c > index f25d5f1d0ef..ca19b95ce46 100644 > --- a/scalar.c > +++ b/scalar.c > @@ -262,7 +262,7 @@ static int register_dir(void) > return error(_("could not set recommended config")); > > if (toggle_maintenance(1)) > - return error(_("could not turn on maintenance")); > + warning(_("could not turn on maintenance")); Should we do the same thing for 'unregister_dir()'? Unlike 'register_dir()', it doesn't break immediately (and finishes removing the enlistment), but it still returns a nonzero error code from 'scalar unregister'. > > if (have_fsmonitor_support() && start_fsmonitor_daemon()) { > return error(_("could not start the FSMonitor daemon")); > diff --git a/t/t9210-scalar.sh b/t/t9210-scalar.sh > index 13a4f6dbcf4..4432a30d10b 100755 > --- a/t/t9210-scalar.sh > +++ b/t/t9210-scalar.sh > @@ -104,10 +104,10 @@ test_expect_success FSMONITOR_DAEMON 'scalar register starts fsmon daemon' ' > test_cmp_config -C test/src true core.fsmonitor > ' > > -test_expect_success 'scalar register fails when background maintenance fails' ' > +test_expect_success 'scalar register warns when background maintenance fails' ' > git init register-repo && > GIT_TEST_MAINT_SCHEDULER="crontab:false,launchctl:false,schtasks:false" \ > - test_must_fail scalar register register-repo 2>err && > + scalar register register-repo 2>err && > grep "could not turn on maintenance" err > ' > > diff --git a/t/t9211-scalar-clone.sh b/t/t9211-scalar-clone.sh > index a6156da29ac..872ad1c9c2b 100755 > --- a/t/t9211-scalar-clone.sh > +++ b/t/t9211-scalar-clone.sh > @@ -174,9 +174,9 @@ test_expect_success 'progress without tty' ' > cleanup_clone $enlistment > ' > > -test_expect_success 'scalar clone fails when background maintenance fails' ' > +test_expect_success 'scalar clone warns when background maintenance fails' ' > GIT_TEST_MAINT_SCHEDULER="crontab:false,launchctl:false,schtasks:false" \ > - test_must_fail scalar clone "file://$(pwd)/to-clone" maint-fail 2>err && > + scalar clone "file://$(pwd)/to-clone" maint-fail 2>err && > grep "could not turn on maintenance" err > ' Similarly, it might be nice to show how 'scalar unregister' behaves when maintenance fails in the tests.
On 1/27/2023 5:06 PM, Victoria Dye wrote: > Derrick Stolee via GitGitGadget wrote: >> if (toggle_maintenance(1)) >> - return error(_("could not turn on maintenance")); >> + warning(_("could not turn on maintenance")); > > Should we do the same thing for 'unregister_dir()'? Unlike 'register_dir()', > it doesn't break immediately (and finishes removing the enlistment), but it > still returns a nonzero error code from 'scalar unregister'. The interesting thing about unregister_dir() is that toggle_maintenance(0) "turns of maintenance" by removing the maintenance.repo config value pointing to this repository, not by removing the maintenance schedule. Thus, we don't get the failure in the same way. >> -test_expect_success 'scalar clone fails when background maintenance fails' ' >> +test_expect_success 'scalar clone warns when background maintenance fails' ' >> GIT_TEST_MAINT_SCHEDULER="crontab:false,launchctl:false,schtasks:false" \ >> - test_must_fail scalar clone "file://$(pwd)/to-clone" maint-fail 2>err && >> + scalar clone "file://$(pwd)/to-clone" maint-fail 2>err && >> grep "could not turn on maintenance" err >> ' > > Similarly, it might be nice to show how 'scalar unregister' behaves when > maintenance fails in the tests. And the way I found out was by making the same tests, but since they actually never failed or listed a warning (for this reason) I left it out. Thanks, -Stolee
On 1/27/2023 3:36 PM, Junio C Hamano wrote: > "Derrick Stolee via GitGitGadget" <gitgitgadget@gmail.com> writes: > >> From: Derrick Stolee <derrickstolee@github.com> >> >> A user reported issues with 'scalar clone' and 'scalar register' when >> working in an environment that had locked down the ability to run >> 'crontab' or 'systemctl' in that those commands registered as _failures_ >> instead of opportunistically reporting a success with just a warning >> about background maintenance. >> >> As a workaround, they can use GIT_TEST_MAINT_SCHEDULER to fake a >> successful background maintenance, but this is not a viable strategy for >> long-term. >> >> Update 'scalar register' and 'scalar clone' to no longer fail by >> modifying register_dir() to only warn when toggle_maintenance(1) fails. >> >> Since background maintenance is a "nice to have" and not a requirement >> for a working repository, it is best to move this from hard error to >> gentle warning. > > Wasn't the main selling point of scalar that users do not have to > worry about various minute details of configuration settings to > maintain their clone of projects on the larger side? Yes, and that includes things like partial clone, sparse-checkout, and FS Monitor which are independent of this maintenance change. > The "maintain > their clone" certainly should include running periodic maintenance > tasks without them having to worry about it. It feels like this is > calling for an explicit "disable periodic maintenance tasks in this > repository" option to help these esoteric environments that disable > cron-like system services, while keeping the default safer, > i.e. fail loudly when the periodic maintenance tasks that the users > expect to happen cannot be enabled, or something. > > Perhaps I am not the primary audience, but hmph, I have a feeling > that this is not exactly going into a healthy direction. Here, we are in an environment where background maintenance is unavailable in an unexpected way. If that feature is not available to the user, should they not get the benefits of the others? Not having background maintenance is definitely a downside, but it's different from failing to connect to the server or being unable to complete setting up the working directory. The warning communicates the issue, so users can realize the problem exists and work to resolve it in their own way. Thanks, -Stolee
Junio C Hamano wrote: > "Derrick Stolee via GitGitGadget" <gitgitgadget@gmail.com> writes: > >> From: Derrick Stolee <derrickstolee@github.com> >> >> A user reported issues with 'scalar clone' and 'scalar register' when >> working in an environment that had locked down the ability to run >> 'crontab' or 'systemctl' in that those commands registered as _failures_ >> instead of opportunistically reporting a success with just a warning >> about background maintenance. >> >> As a workaround, they can use GIT_TEST_MAINT_SCHEDULER to fake a >> successful background maintenance, but this is not a viable strategy for >> long-term. >> >> Update 'scalar register' and 'scalar clone' to no longer fail by >> modifying register_dir() to only warn when toggle_maintenance(1) fails. >> >> Since background maintenance is a "nice to have" and not a requirement >> for a working repository, it is best to move this from hard error to >> gentle warning. > > Wasn't the main selling point of scalar that users do not have to > worry about various minute details of configuration settings to > maintain their clone of projects on the larger side? The "maintain > their clone" certainly should include running periodic maintenance > tasks without them having to worry about it. It feels like this is > calling for an explicit "disable periodic maintenance tasks in this > repository" option to help these esoteric environments that disable > cron-like system services, while keeping the default safer, > i.e. fail loudly when the periodic maintenance tasks that the users > expect to happen cannot be enabled, or something. I see Stolee's approach as more in line with how FSMonitor is treated by 'scalar', which is "only turn it on if it's supported, but otherwise do nothing" (the main difference here being that a warning is displayed if maintenance can't be turned on). That still fits the stated goal of 'scalar' ("configure all the niche large-repo settings for me when I clone/register"), but it makes 'scalar' more forgiving of system configurations that don't support maintenance. That said, this doesn't distinguish between "maintenance couldn't be turned on because the system doesn't support it" and "maintenance couldn't be turned on because of an internal error". The latter might still be worth failing on, so maybe something like this would be more palatable? --------8<--------8<--------8<-------- diff --git a/scalar.c b/scalar.c index 6c52243cdf1..138780abe5f 100644 --- a/scalar.c +++ b/scalar.c @@ -252,6 +252,10 @@ static int stop_fsmonitor_daemon(void) return 0; } +static int have_maintenance_support(void) { + /* check whether at least one scheduler is supported on the system */ +} + static int register_dir(void) { if (add_or_remove_enlistment(1)) @@ -260,7 +264,7 @@ static int register_dir(void) if (set_recommended_config(0)) return error(_("could not set recommended config")); - if (toggle_maintenance(1)) + if (have_maintenance_support() && toggle_maintenance(1)) return error(_("could not turn on maintenance")); if (have_fsmonitor_support() && start_fsmonitor_daemon()) { -------->8-------->8-------->8-------- > > Perhaps I am not the primary audience, but hmph, I have a feeling > that this is not exactly going into a healthy direction. I don't think this change would be the start of a larger pattern, since most Scalar-related settings aren't system-dependent (only FSMonitor and maintenance are, AFAIK). What would be worse, I think, is setting the maintenance behavior with a new command option - I could very easily see *that* leading to a slippery slope of endless options to toggle 'scalar's behaviors, completely defeating it's whole "set everything up for me" purpose. > > Other two steps that led to this step looked quite sensible, though. > > Thanks.
Derrick Stolee <derrickstolee@github.com> writes: >> The "maintain >> their clone" certainly should include running periodic maintenance >> tasks without them having to worry about it. It feels like this is >> calling for an explicit "disable periodic maintenance tasks in this >> repository" option to help these esoteric environments that disable >> cron-like system services, while keeping the default safer, >> i.e. fail loudly when the periodic maintenance tasks that the users >> expect to happen cannot be enabled, or something. >> >> Perhaps I am not the primary audience, but hmph, I have a feeling >> that this is not exactly going into a healthy direction. > > Here, we are in an environment where background maintenance is > unavailable in an unexpected way. If that feature is not available > to the user, should they not get the benefits of the others? That is not what I was saying. I just have expected to see a way for the user to give scalar an explicit "I understand that periodic maintenance does not happen in this repository" consent, instead of demoting an error detection for everybody to a warning that users will just ignore.
On 1/27/2023 7:32 PM, Junio C Hamano wrote: > Derrick Stolee <derrickstolee@github.com> writes: > >>> The "maintain >>> their clone" certainly should include running periodic maintenance >>> tasks without them having to worry about it. It feels like this is >>> calling for an explicit "disable periodic maintenance tasks in this >>> repository" option to help these esoteric environments that disable >>> cron-like system services, while keeping the default safer, >>> i.e. fail loudly when the periodic maintenance tasks that the users >>> expect to happen cannot be enabled, or something. >>> >>> Perhaps I am not the primary audience, but hmph, I have a feeling >>> that this is not exactly going into a healthy direction. >> >> Here, we are in an environment where background maintenance is >> unavailable in an unexpected way. If that feature is not available >> to the user, should they not get the benefits of the others? > > That is not what I was saying. I just have expected to see a way > for the user to give scalar an explicit "I understand that periodic > maintenance does not happen in this repository" consent, instead of > demoting an error detection for everybody to a warning that users > will just ignore. Ah, so you'd prefer a --no-maintenance option for users who have this problem instead of just a warning. I'll do that in v2. This could be a good time for me to upstream the --no-src option while I'm messing with arguments in 'scalar clone'. Thanks, -Stolee
Derrick Stolee <derrickstolee@github.com> writes: >>> Here, we are in an environment where background maintenance is >>> unavailable in an unexpected way. If that feature is not available >>> to the user, should they not get the benefits of the others? >> >> That is not what I was saying. I just have expected to see a way >> for the user to give scalar an explicit "I understand that periodic >> maintenance does not happen in this repository" consent, instead of >> demoting an error detection for everybody to a warning that users >> will just ignore. > > Ah, so you'd prefer a --no-maintenance option for users who have > this problem instead of just a warning. I'll do that in v2. Or a repository-local configuration to declare "no need to do the maintenance stuff here", probably? The expected use case you gave does not match per-invocation command line option very well, right? > This could be a good time for me to upstream the --no-src option > while I'm messing with arguments in 'scalar clone'. OK. Thanks.
Derrick Stolee wrote: > On 1/27/2023 7:32 PM, Junio C Hamano wrote: >> Derrick Stolee <derrickstolee@github.com> writes: >> >>>> The "maintain >>>> their clone" certainly should include running periodic maintenance >>>> tasks without them having to worry about it. It feels like this is >>>> calling for an explicit "disable periodic maintenance tasks in this >>>> repository" option to help these esoteric environments that disable >>>> cron-like system services, while keeping the default safer, >>>> i.e. fail loudly when the periodic maintenance tasks that the users >>>> expect to happen cannot be enabled, or something. >>>> >>>> Perhaps I am not the primary audience, but hmph, I have a feeling >>>> that this is not exactly going into a healthy direction. >>> >>> Here, we are in an environment where background maintenance is >>> unavailable in an unexpected way. If that feature is not available >>> to the user, should they not get the benefits of the others? >> >> That is not what I was saying. I just have expected to see a way >> for the user to give scalar an explicit "I understand that periodic >> maintenance does not happen in this repository" consent, instead of >> demoting an error detection for everybody to a warning that users >> will just ignore. > > Ah, so you'd prefer a --no-maintenance option for users who have > this problem instead of just a warning. I'll do that in v2. I mentioned this earlier [1], but I want to reiterate that I really don't think a dedicated '--no-maintenance' option is a good approach to this problem. I understand wanting more active user acknowledgement that "I understand that periodic maintenance does not happen in this repository"; without that, users may (rightfully) be confused when they find their scalar-cloned repository full of loose objects. But, in the use case you've presented (where no scheduler is available), the user would need to - somewhat redundantly, I feel - acknowledge that for *every* repository they clone. I'm also still worried about cluttering scalar's UX with options that toggle use of its internally-configured options and features. One of the big selling points for including scalar in the upstream project ([2], [3]) was its ability to "intelligently" configure all of the settings a user would need to optimize a large repository *without* a user needing to know what any of those options are/what they mean. These settings are inherently subject to change (due to use of experimental features); exposing a feature toggle entrenches that setting permanently within scalar and makes a user aware of implementation details that were intended to be hidden. At a high level, it pushes scalar towards simply being an "opinionated" 'git config'-configurator, which was a model I explicitly tried to move away from while upstreaming last year. I still believe treating maintenance like FSMonitor - pre-determining whether the feature is available and only enabling it if possible - is the most consistent and user-friendly solution to the given problem within the context of scalar. But, if you feel that user acknowledgement is absolutely critical, I'd strongly prefer a config setting like 'maintenance.enabled'; it could be set globally (the appropriate scope in the case of a system that has no scheduler), or with '-c' with Scalar clone if it really needs to be per-repo. [1] https://lore.kernel.org/git/3ade6d9f-8477-40c2-d683-d629e863c6ab@github.com/ [2] https://lore.kernel.org/git/pull.1005.git.1630359290.gitgitgadget@gmail.com/ [3] https://lore.kernel.org/git/pull.1275.git.1656521925.gitgitgadget@gmail.com/ > > This could be a good time for me to upstream the --no-src option > while I'm messing with arguments in 'scalar clone'. For what it's worth, my concerns about option clutter don't really apply to '--no-src' (cloning directly into a given directory, rather than '<directory>/src'). Unlike features like FSMonitor and maintenance, the 'src/' directory is a user-facing Scalar design decision. It's also something that seems to exist primarily for backward-compatibility reasons (if I'm interpreting your earlier comments [4] correctly). This could be a step on a deprecation path to make '--no-src' the default and remove the legacy enlistment structure? At the very least, it's sufficiently outside scalar's "configure for a large repo" scope for me to not worry about it setting a bad precedent. [4] https://lore.kernel.org/git/82716e5b-3522-68f5-7479-1b39811e0cb2@github.com/ > > Thanks, > -Stolee
Victoria Dye <vdye@github.com> writes: > I'm also still worried about cluttering scalar's UX with options that toggle > use of its internally-configured options and features. One of the big > selling points for including scalar in the upstream project ([2], [3]) was > its ability to "intelligently" configure all of the settings a user would > need to optimize a large repository *without* a user needing to know what > any of those options are/what they mean. These settings are inherently > subject to change (due to use of experimental features); exposing a feature > toggle entrenches that setting permanently within scalar and makes a user > aware of implementation details that were intended to be hidden. At a high > level, it pushes scalar towards simply being an "opinionated" 'git > config'-configurator, which was a model I explicitly tried to move away from > while upstreaming last year. I personally do not think "opinionated configurator" is a bad model at all. And "this does not seem to work here, so let's silently disable it, as the user does not want to hear about minute details" is a valid opinion to have for such a tool. I too share the aversion to command line option for this one. Disabled periodic task support is most likely system-wide, and passing --no-whatever every time you touch a new repository on the same system does not make much sense. Thanks.
On 1/30/2023 1:58 PM, Junio C Hamano wrote: > Victoria Dye <vdye@github.com> writes: > >> I'm also still worried about cluttering scalar's UX with options that toggle >> use of its internally-configured options and features. One of the big >> selling points for including scalar in the upstream project ([2], [3]) was >> its ability to "intelligently" configure all of the settings a user would >> need to optimize a large repository *without* a user needing to know what >> any of those options are/what they mean. These settings are inherently >> subject to change (due to use of experimental features); exposing a feature >> toggle entrenches that setting permanently within scalar and makes a user >> aware of implementation details that were intended to be hidden. At a high >> level, it pushes scalar towards simply being an "opinionated" 'git >> config'-configurator, which was a model I explicitly tried to move away from >> while upstreaming last year. > > I personally do not think "opinionated configurator" is a bad model > at all. And "this does not seem to work here, so let's silently > disable it, as the user does not want to hear about minute details" > is a valid opinion to have for such a tool. > > I too share the aversion to command line option for this one. > Disabled periodic task support is most likely system-wide, and > passing --no-whatever every time you touch a new repository on the > same system does not make much sense. Thanks, both. v2 will include --no-src, but not --no-maintenance. -Stolee
On 1/27/2023 5:18 PM, Victoria Dye wrote: > Junio C Hamano wrote: >> "Derrick Stolee via GitGitGadget" <gitgitgadget@gmail.com> writes: This reply almost got lost in the shuffle, but mostly because it wasn't super-relevant if we were going with the --no-maintenance option. It's relevant again, so I wanted to point something out. > I see Stolee's approach as more in line with how FSMonitor is treated by > 'scalar', which is "only turn it on if it's supported, but otherwise do > nothing" (the main difference here being that a warning is displayed if > maintenance can't be turned on). That still fits the stated goal of 'scalar' > ("configure all the niche large-repo settings for me when I > clone/register"), but it makes 'scalar' more forgiving of system > configurations that don't support maintenance. > > That said, this doesn't distinguish between "maintenance couldn't be turned > on because the system doesn't support it" and "maintenance couldn't be > turned on because of an internal error". The latter might still be worth > failing on, so maybe something like this would be more palatable? > > --------8<--------8<--------8<-------- > diff --git a/scalar.c b/scalar.c > index 6c52243cdf1..138780abe5f 100644 > --- a/scalar.c > +++ b/scalar.c > @@ -252,6 +252,10 @@ static int stop_fsmonitor_daemon(void) > return 0; > } > > +static int have_maintenance_support(void) { > + /* check whether at least one scheduler is supported on the system */ > +} > + > static int register_dir(void) > { > if (add_or_remove_enlistment(1)) > @@ -260,7 +264,7 @@ static int register_dir(void) > if (set_recommended_config(0)) > return error(_("could not set recommended config")); > > - if (toggle_maintenance(1)) > + if (have_maintenance_support() && toggle_maintenance(1)) > return error(_("could not turn on maintenance")); > > if (have_fsmonitor_support() && start_fsmonitor_daemon()) { The tricky thing about this is that have_fsmonitor_support() is something we can detect at compile time, while have_maintenance_support() would not (unless we start building for a new platform). The case that brought this up is a platform that has both 'crontab' and 'systemctl' on the PATH, but when executing those commands an error occurs due to permissions. So, if we wanted to distinguish between permissions issues and/or other unrelated failures, we would need to be able to parse the output of those commands. That seems fraught with potential errors, so it seems like we should _attempt_ to enable maintenance and warn with whatever failure is presented. The only thing I could think is that we could define a custom exit code within 'git maintenance start' that means "we were able to start the scheduler process, but it failed" to differentiate from other kinds of failures, such as failing to write to global config. Thanks, -Stolee
diff --git a/scalar.c b/scalar.c index f25d5f1d0ef..ca19b95ce46 100644 --- a/scalar.c +++ b/scalar.c @@ -262,7 +262,7 @@ static int register_dir(void) return error(_("could not set recommended config")); if (toggle_maintenance(1)) - return error(_("could not turn on maintenance")); + warning(_("could not turn on maintenance")); if (have_fsmonitor_support() && start_fsmonitor_daemon()) { return error(_("could not start the FSMonitor daemon")); diff --git a/t/t9210-scalar.sh b/t/t9210-scalar.sh index 13a4f6dbcf4..4432a30d10b 100755 --- a/t/t9210-scalar.sh +++ b/t/t9210-scalar.sh @@ -104,10 +104,10 @@ test_expect_success FSMONITOR_DAEMON 'scalar register starts fsmon daemon' ' test_cmp_config -C test/src true core.fsmonitor ' -test_expect_success 'scalar register fails when background maintenance fails' ' +test_expect_success 'scalar register warns when background maintenance fails' ' git init register-repo && GIT_TEST_MAINT_SCHEDULER="crontab:false,launchctl:false,schtasks:false" \ - test_must_fail scalar register register-repo 2>err && + scalar register register-repo 2>err && grep "could not turn on maintenance" err ' diff --git a/t/t9211-scalar-clone.sh b/t/t9211-scalar-clone.sh index a6156da29ac..872ad1c9c2b 100755 --- a/t/t9211-scalar-clone.sh +++ b/t/t9211-scalar-clone.sh @@ -174,9 +174,9 @@ test_expect_success 'progress without tty' ' cleanup_clone $enlistment ' -test_expect_success 'scalar clone fails when background maintenance fails' ' +test_expect_success 'scalar clone warns when background maintenance fails' ' GIT_TEST_MAINT_SCHEDULER="crontab:false,launchctl:false,schtasks:false" \ - test_must_fail scalar clone "file://$(pwd)/to-clone" maint-fail 2>err && + scalar clone "file://$(pwd)/to-clone" maint-fail 2>err && grep "could not turn on maintenance" err '