DragonFlyBSD

The "off" switch used to be there, but it was removed. It can be brought back.
The copy is made with "-pRP". It looks like the first switch is responsible for the flags that must be changed before deletion. The issue is that these flags need to be preserved in case the "restore world" option is used. I'm not sure what to do other than tell people that use NFS that they must turn off backups.

There's also a problem with those that build as non-root and install as root. The next build fails because the non-root user can't remove the root-owned timestamp file. I want to solve this by making this relatively unimportant file use 777 permission so any user can delete it. Matt didn't like this idea but personally I see nothing wrong with it.

The original incarnation of this is similar to your patch, but dillon asked for it to be reworked and simplified. Now it's blown up in complexity again.

On the "makefile" patch, IIRC, you're only supposed to document targets that you can actually run. You can't "make backupworld-auto-clean" for instance, so it's would only appear on Makefile.inc

On quick glance, it seems that by removing the timestamp to solve one problem, you reintroduced another problem: The auto-backup (if enabled) will occur every time you "make installworld". In the case where you buildworld once and "make installworld" many times, this is undesired.

The use of a tarball is probably an improvement especially if it allows backup of make, cp, etc, and I suspect it's a key in dealing with the chflags/nfs issue.

I would disagree (also based on Dillon's direction) that the AUTO_BACKUP is off by default.
I'm also not sure about the redefinition of the AUTO_BACKUP default. That seems to favor an NFS implementation. Wouldn't the standard case be a single machine with local backups? In that case, the default is a bit obnoxious. Not something to fall on a sword for..

I think it's a big step in the right direction.

On 04/08/13 11:52, John Marino via Redmine wrote:

I would disagree (also based on Dillon's direction) that the AUTO_BACKUP is off by default.

as long as it can be disabled is fine IMHO

I'm also not sure about the redefinition of the AUTO_BACKUP default. That seems to favor an NFS implementation. Wouldn't the standard case be a single machine with local backups? In that case, the default is a bit obnoxious. Not something to fall on a sword for..

Not sure how you mean 'favoring an NFS implementation'

A local backup wouldn't need several levels of directories filled with "hostname" and "arch" since neither will ever change. It implies that it would be backed up in a common area with multiple hostnames and archs. (and do we need to protect for a case where the there are two arches with the same hostname?). Seems like overkill for the default.

Well, I guess hostname could change, but do we want to lose the backup when this happens?

marino wrote:

The original incarnation of this is similar to your patch, but dillon asked for it to be reworked
and simplified. Now it's blown up in complexity again.

I don't agree; supplied patch simplifies (& fixes bugs): fewer lines with simple logic & better doc.

On the "makefile" patch, IIRC, you're only supposed to document targets that you can actually run.
You can't "make backupworld-auto-clean" for instance, so it's would only appear on Makefile.inc

The backupworld-auto-clean* targets are meant to be manual only (GC).

On quick glance, it seems that by removing the timestamp to solve one problem, you reintroduced >another problem: The auto-backup (if enabled) will occur every time you "make installworld".
In the case where you buildworld once and "make installworld" many times, this is undesired.

It is not perfect, but your solution was worse & doc/commit message didn't indicate what purpose was;
we need to find a better way if it is important.

I would disagree (also based on Dillon's direction) that the AUTO_BACKUP is off by default.
I'm also not sure about the redefinition of the AUTO_BACKUP default. That seems to favor an NFS
implementation. Wouldn't the standard case be a single machine with local backups? In that case,
the default is a bit obnoxious. Not something to fall on a sword for..

There is no standard case, we need to make it work for all supported setups,
especially if auto-backup will be on by default.

I think it's a big step in the right direction.

Thanks

A local backup wouldn't need several levels of directories filled with "hostname" and "arch"
since neither will ever change. It implies that it would be backed up in a common area with
multiple hostnames and archs.
(and do we need to protect for a case where the there are two arches with the same hostname?).
Seems like overkill for the default.

We need a simple scheme which works in all supported setups: NFS or local MAKEOBJDIRPREFIX;
there is no mechanism to change auto-backup dir, so we don't have a default.

IMO the scheme I suggest in patch matches what we need,
but if you have other suggestion, please show it.

We could introduce mechanism to change auto-backup dir, if it is needed.

Also, even for local MAKEOBJDIRPREFIX we do need TARGET_ARCH to influence auto-backup dir,
as we support cross builds.

Hostname isn't a perfect match, for what we need, but it is close, as we have no system UUID.
If hostname is changed, user can easily find backup to restore from: it is all documented.

We could turn on auto-backup on by default, when we have a worked out implementation;
I'm not sure we will get there for the release.

My intention is to commit something like the supplied patch before release,
if we don't agree on other fix; alternatively we could backout backupworld for now &
re-introduce a reworked version at a later time.

-thomas

We will do something before release, don't worry about that.

I don't know what you mean by, there is no mechanism to change auto-backup dir, so we don't have a default.". That is simply not true IMO. The operator "?=" means define it as this if it's not already defined. You can predefine auto-backup dir in make.conf. So those that use NFS could define it anyway they think works. We don't need to protect for lowest common denominator, it should be defaulted to the most common case.

Regardless if the current implementation isn't the best, you've introduced a regression. In the end, we may accept that based on the full pros and cons but in general regressions should be avoided. In the other hand, I haven't figured out how to make it better yet either.

The autobackup has basically been working in most cases, it's a couple of edge cases that's the problem (NFS + build as non-root). The later can be "fixed" with your regression and the former is fixed with your patch. Since that covers the known problems, I see no problem with the release.

Hi,

The comment about the NFS users "those that use NFS could define it anyway they think works" seems a way of avoiding or bypassing the problem. In my opinion you cannot propose different setup just to leave an issue unhandled.

I also think this should be "off" by default with an "on" switch.

Best regards,
Antonio Huete

On 04/12/13 06:28, John Marino via Redmine wrote:

I don't know what you mean by, there is no mechanism to change auto-backup dir, so we don't have a default.". That is simply not true IMO. The operator "?=" means define it as this if it's not already defined. You can predefine auto-backup dir in make.conf. So those that use NFS could define it anyway they think works. We don't need to protect for lowest common denominator, it should be defaulted to the most common case.

Agreed that using '?=' provides a mechanism for cfg overrides -
however, in the current patch, auto_backup is '=''ed and not '?='d..
so maybe this is the issue?

To note - my personal typical NFS usage is:

- run a build on machine1
- if the build works, install to another 'second-tier'
machine (machine2)
- reboot machine2 and 'burn in test' or resolve any upgrade issues
- if machine2 works, then upgrade any other second-tier machines
and finally machine1

If I'm heavily tracking tip or doing local dev, the 'machine1 build
/ machine2 install' cycle might occur multiple times before installing
to the other machines, however, each individual machine1-machine2
cycle in effect performs a mini-verification of the changes between
builds, so 'fast forwarding' the other machines after a few builds
without having the backup is not very risky.

So really, for most cases, I'd only need the backup on machine2,
since it is the 'build test' machine, and wouldn't need host specific
ones - This is somewhat symmetric with a 'single obj' tree shared
over NFS - e.g. I don't build N times for N machines, but 1 time
for N machines - similarly, I only need 1 backup for N machines.

But, if this is configurable, defaulting to multiple ${HOSTNAME}
backups is not really an issue since I manage /etc/make.conf on all
nodes, and might be handy for other use cases e.g. where you have
various 'sets' of machines hooked into a build node upgraded at
different times and want to keep emergency backups during upgrades.

In my scenario, I'd probably only have the auto-backup setup on
'machine2', with the rest configured as 'off'.

Put another way, using the 'NFS centric' hostname directory path
is not necessary in my particular local NFS use case, and if this
setting is configurable, could always be configured by a user
to support multiple backups, thereby making it 'not NFS centric'
by Marinos definition of 'NFS centric' and my NFS use case :D

Anyhow, some thoughts.

Cheers,

- Chris

Yeah, given NFS scenario, AUTO_BACKUP should be redefinable. It was defined as "=" before when it was assumed to be a local folder.

Tuxillo, the default "works". There's a 100 ways to set up an NFS mount, nobody can guess them all. The point is if somebody's particular setup works better with a different definition, they can set it.

Having an auto backup off when you don't even know it exists pretty much defeats the point. By turning it off, you clearly know it exists and have made an informed decision. It's erroring on the side of safety, aka it is the most conservative.

On 04/16/13 03:06, John Marino via Redmine wrote:

I have slightly reworked Thomas' patch (attached).

Works (in theory) for me - +1

Cheers,

- Chris