Fossil Forum

Disable creation of conflict files foo-baseline/-merge/-original
Login

Disable creation of conflict files foo-baseline/-merge/-original

(1) By geburashka on 2020-02-20 00:09:19 [source]

As per title, how can I stop fossil from generating such files when a conflict is created? I'm happy with just the markers inside the file.

(2) By Stephan Beal (stephan) on 2020-02-20 00:15:03 [link] [source] in reply to 1

There isn't currently a way to do that, but that sounds like a good idea for a new setting. If no one has implemented this by next Monday i will take a look at doing so (but unfortunately have other priorities until then).

(3) By geburashka on 2020-02-20 00:35:01 [link] [source] in reply to 2

Would very much appreciate it!

Manually deleting those files every time is annoying - made worse by forgetting about them and accidentally adding them in WIP commits, and then dealing with that.

(4) By Warren Young (wyoung) on 2020-02-20 13:20:01 [link] [source] in reply to 3

In the meantime, and for those who sometimes find these files useful but then want them gone once their purpose has been served, here is my cleanup-after-bad-patch script:

#!/bin/bash
find . '(' \
	-name \*-baseline -o \
	-name \*-baseline-\[0-9\] -o \
	-name \*-merge -o \
	-name \*-merge-\[0-9\] -o \
	-name \*-original -o \
	-name \*-original-\[0-9\] -o \
	-name \*.orig -o \
	-name \*.rej \
')' -delete

The script's name comes from the fact that it originally fixed the related patch(1) problem, but I've extended it for Fossil.

(5) By Stephan Beal (stephan) on 2020-02-20 13:29:20 [link] [source] in reply to 3

Manually deleting those files every time is annoying - made worse by forgetting about them and accidentally adding them in WIP commits, and then dealing with that.

Don't forget that you can add wildcards for them to your ignore-glob setting. That won't eliminate the files but will keep them from being implicitly added to the repo by certain commands.

(6) By geburashka on 2020-02-20 22:55:34 [link] [source] in reply to 5

Ah! Completely forgot, cheers! One part of the problem dominated my mind to the exclusion of all else.

(7) By Florian Balmer (florian.balmer) on 2020-02-21 13:44:10 [link] [source] in reply to 2

... that sounds like a good idea for a new setting.

Without wanting to dampen the motivation to implement feature requests, I'm not sure if this "deserves a new setting":

  1. There's only one (implicit) request, and no replies from other Fossil users also lacking such a feature, so far.
  2. There's two solutions that require only minimal changes to existing workflows (the posted cleanup-after-bad-patch script, and the ignore-glob).
  3. The third solution is to use fossil clean --temp.
  4. A setting to omit backup files on merge conflicts may never be used by 99.9% of Fossil users.
  5. With more and more settings, it's getting more and more complicated and confusing to configure new repositories (this may be relieved a bit if the settings are added to the config sets, so they work with fossil init --template ..., for example).
  6. The same way users may forget about the work-arounds, they may also forget about the setting itself, ending up with the same situation for new repositories.

If such a feature is really necessary, I believe it's better implemented as a command-line option for the merge command, similar to the --no-backup-if-mismatch option from patch(1), so users are more likely to find it through fossil help merge.

(8) By Richard Hipp (drh) on 2020-02-21 13:54:05 [link] [source] in reply to 7

I believe it's better implemented as a command-line option

I was thinking of omitting the merge-conflict temp files by default and only retaining them if a new command-line option is used. I agree that we don't need yet another setting for this.

In the 12.5 years that I've been actively using Fossil, I don't think I have ever once used or even looked at those merge-conflict temp files. I would not miss them if they went away completely.

(9) By Andy Bradford (andybradford) on 2020-02-21 14:57:18 [link] in reply to 8

I can honestly  say I've never looked  at them at all  except in passing
while looking at "ls" output---wondering why they were there.

Andy

(10) By Florian Balmer (florian.balmer) on 2020-02-21 15:30:18 [link] [source] in reply to 8

I was thinking of omitting the merge-conflict temp files by default and only retaining them if a new command-line option is used.

That's an even better idea!

(12) By Stephan Beal (stephan) on 2020-02-21 16:09:11 [link] [source] in reply to 10

That's an even better idea!

Agreed 100%.

(15) By Stephan Beal (stephan) on 2020-02-25 19:12:29 [link] [source] in reply to 8

I was thinking of omitting the merge-conflict temp files by default and only retaining them if a new command-line option is used. I agree that we don't need yet another setting for this.

Good evening, Richard!

i'm implementing this now and have a couple questions about the implementation:

  1. This lives in merge_3way(), which is used by the stash, update, and merge commands, but update and merge are the only ones which currently pass any flags to merge_3way(). Presumably we'll certainly want a --keep-merge-files flag (or something less verbose) for merge and update, but do we also want it for stash?

  2. --keep-merge-files seems rather verbose, but a better name has yet to come to mind. Suggestions are welcomed.

The implementation would seem to be trivial, but if you notice a glaring hole here, please let us know:

Index: src/merge3.c
==================================================================
--- src/merge3.c
+++ src/merge3.c
@@ -436,10 +436,16 @@
 #if INTERFACE
 /*
 ** Flags to the 3-way merger
 */
 #define MERGE_DRYRUN  0x0001
+/*
+** The MERGE_KEEP_FILES flag specifies that merge_3way() should retain
+** its temporary files on error. By default they are removed after the
+** merge, regardless of success or failure.
+*/
+#define MERGE_KEEP_FILES 0x0002
 #endif
 
 
 /*
 ** This routine is a wrapper around blob_merge() with the following
@@ -487,21 +493,24 @@
       zGMerge = db_get("gmerge-command", 0);
       if( zGMerge && zGMerge[0] ){
         char *zOut;     /* Temporary output file */
         char *zCmd;     /* Command to invoke */
         const char *azSubst[8];  /* Strings to be substituted */
-
+        i64 nZOut;      /* Size of generated zOut. */
         zOut = file_newname(zV1, "output", 1);
         azSubst[0] = "%baseline";  azSubst[1] = zPivot;
         azSubst[2] = "%original";  azSubst[3] = zOrig;
         azSubst[4] = "%merge";     azSubst[5] = zOther;
         azSubst[6] = "%output";    azSubst[7] = zOut;
         zCmd = string_subst(zGMerge, 8, azSubst);
         printf("%s\n", zCmd); fflush(stdout);
         fossil_system(zCmd);
-        if( file_size(zOut, RepoFILE)>=0 ){
+        nZOut = file_size(zOut, RepoFILE);
+        if( nZOut>=0 ){
           blob_read_from_file(pOut, zOut, ExtFILE);
+        }
+        if( nZOut>=0 || (mergeFlags & MERGE_KEEP_FILES)==0 ){
           file_delete(zPivot);
           file_delete(zOrig);
           file_delete(zOther);
           file_delete(zOut);
         }

And while looking for the origin of these files i stumbled across a typo in sqlite3:

sqlite3.c:** This function is called after an incrmental-merge operation has run to

(16) By Richard Hipp (drh) on 2020-02-25 19:44:35 [link] [source] in reply to 15

I'd be happy to see this appearing on trunk.

(17) By Stephan Beal (stephan) on 2020-02-25 19:50:36 [link] [source] in reply to 16

The local implementation is done, just waiting on confirmation of:

  1. The name of the flag. It's currently --keep-merge-files a.k.a. -K and causes those files only to be retained on a merge error. On success they're always removed (as before).

  2. Whether or not stash should also support that flag. Without it, those files would always be deleted for stash conflicts.

And if you can recommend an easy way to simulate a conflict, for testing purposes, that would be helpful.

(18) By Stephan Beal (stephan) on 2020-02-25 21:03:29 [link] [source] in reply to 16

I'd be happy to see this appearing on trunk.

This is now implemented and tested, but is in a branch because i'm off to bed soon and am not sure how suitable the flag's name is:

https://fossil-scm.org/fossil/timeline?r=merge-remove-temp-files

That applies to merge/update, but stash was left untouched because implementing it would have required a bit more internal API surgery to pass along the merge flags.

Full disclosure: testing requiring merging and conflicting, and i merged/undid the new admin-user flag branch about a dozen times while testing... and then committed this feature with that branch unintentionally merged into it. Luckily, you've already merged that feature into trunk, so there's no harm done other than to my pride.

(19) By Florian Balmer (florian.balmer) on 2020-02-26 13:20:30 [link] [source] in reply to 18

  • I can't test this right now, but doesn't stash pop clear the stash entry, making backups of the "original" even more important?
  • Should the flag name include "temp" or "backup", since "merge files" could also refer to the files resulting from the merge operation?

(20) By Stephan Beal (stephan) on 2020-02-26 13:38:05 [link] [source] in reply to 19

but doesn't stash pop clear the stash entry, making backups of the "original" even more important?

Good point. They'll still be lost unless the user uses the new flag, though. Maybe stash should always create them on error (same as historical behaviour), because of the potential for loss in a pop conflict? That would require only a 1-line change. Having to pass the flags through multiple layers of calls is the reason i didn't touch stash during the initial implementation.

Should the flag name include "temp" or "backup", since "merge files" could also refer to the files resulting from the merge operation?

Very possibly. i'm not really happy with the current name - suggestions are welcomed.

(21) By Stephan Beal (stephan) on 2020-02-26 14:08:42 [link] [source] in reply to 20

Maybe stash should always create them on error (same as historical behaviour), because of the potential for loss in a pop conflict? That would require only a 1-line change.

i'm currently convinced that that's the right way to handle the stash, and have checked that in to the feature branch. Feel free to convince me otherwise if there are objections.

(22) By Florian Balmer (florian.balmer) on 2020-02-26 17:29:27 [link] [source] in reply to 21

Prevention of data loss is most important. But for consistency, the stash commands that perform merges should have an inverse option -- if implementation complexity allows it?

(23) By Stephan Beal (stephan) on 2020-02-26 17:35:31 [link] [source] in reply to 22

But for consistency, the stash commands that perform merges should have an inverse option -- if implementation complexity allows it?

i considered that but it really doesn't seem worth the effort :/. It would likely cause more confusion than it solves, since it's the opposite of how merge/update's flags work, and i doubt anyone would actually remember to use that flag in that one corner case (they'd have to know about it and remember to use it before popping the stash). i suggest we leave that option out until/unless someone complains about having to clean up those files after a failed stash pop/apply. (That said - feel free to add it if you feel strongly about it.)

(24) By Florian Balmer (florian.balmer) on 2020-02-27 13:50:42 [link] [source] in reply to 23

I've now had the time to do some tests, and everything seems to work fine, in particular:

  • fossil undo after fossil merge│update restores changes to edited files, so the -original backups seem redundant, here. (But only as long a no other command has replaced the undo entry, so the backups still provide a certain level of safety.)
  • fossil stash pop removes the stash entry, so the only way to recover the original file is to copy from the -original backups. (However, fossil undo seems to restore the popped stash entry, something I haven't been aware of.)

The different behavior for fossil merge│update vs. fossil stash pop│apply bugs me a littleº, but not enough to add the inverse command-line option to the stash commands, which nobody might ever use, as stephan already pointed out.

º: The now less-predictably (i.e. only for the stash commands) appearing -baseline│-merge│-original files might still bug the people who submitted the initial feature request? But in the end, a workflow that allows these files to unintentionally slip into a check-in looks flawed, since adding new files requires explicit user action, and it seems quite common for source code projects to accumulate other temporary files inside the source code tree.

(25) By geburashka on 2020-02-27 23:13:53 [link] [source] in reply to 24

since adding new files requires explicit user action, and it seems quite common for source code projects to accumulate other temporary files inside the source code tree.

While I agree with you mostly there are two small points against that:

  1. the temp files generated in projects are generally expected ahead of time and generated by known tools for known purposes - instead, these conflict files aren't generated by any other vcs I know of, and there's no message to inform the user they've been created either. Also, their purpose is debatable (given how many fossil veterans here have never even looked at them), and at best is transient (to feed into a merge tool for eg., in which case they can reside in /tmp instead).

  2. in rapid prototyping phase I don't want to explicitly specify each source file I added as part of my changes - instead I just do fossil add * && fossil commit -m as an alias and move on with life.

Point 2 is solved by having a global ignore glob - but then it's only global for one user on one machine (unless you (remember to) sync your dotfiles). There's still a lot of room there for accidental checkins for the unsuspecting fossil user.

An alternative solution might be that the fossil binary ignores such files - I don't see why anyone would ever want to track them... problem is, that naming convention can easily be an intentional file and not a generated one, so placing them in /tmp (using mktemp) instead makes a lot more sense (which is more or less how other vcs feed merge conflicts to other tools).

(26) By Stephan Beal (stephan) on 2020-02-28 00:11:12 [link] [source] in reply to 25

in rapid prototyping phase I don't want to explicitly specify each source file I added as part of my changes - instead I just do fossil add * && fossil commit -m as an alias and move on with life.

Fossil doing exactly what you've told it to do (add *) is a feature, not a bug. It's the user's responsibility to understand what * resolves to before passing it to fossil, otherwise Garbage In Garbage Out happens.

(FWIW, i've always considered fossil addremove to be a misfeature. Developers can and should specify precisely what they want in SCM. IMHO. Obviously, opinions differ.)

Point 2 is solved by having a global ignore glob - but then it's only global for one user on one machine

That setting is versioned. Create a file named .fossil-settings/ignore-glob and check it into fossil, then it's treated as SCM'd content and its globs apply to all commands which honor the ignore-glob setting. e.g.:

[stephan@lapdog:~/fossil/cwal]$ cat .fossil-settings/ignore-glob 
*~
*.o
*.out

An alternative solution might be that the fossil binary ignores such files - I don't see why anyone would ever want to track them... problem is, that naming convention can easily be an intentional file and not a generated one, so placing them in /tmp (using mktemp) instead makes a lot more sense (which is more or less how other vcs feed merge conflicts to other tools).

Just FYI: from the man page of mktemp(3):

DESCRIPTION
       Never use this function; see BUGS.

CONFORMING TO
       4.3BSD, POSIX.1-2001.  POSIX.1-2008 removes the specification of mktemp().

BUGS
       Never use mktemp().  Some implementations follow 4.3BSD and replace XXXXXX by the current process ID  and
       a single letter, so that at most 26 different names can be returned.  Since on the one hand the names are
       easy to guess, and on the other hand there is a race between testing whether the name exists and  opening
       the file, every use of mktemp() is a security risk.  The race is avoided by mkstemp(3) and mkdtemp(3).

Placing them in /tmp also does not account for files in different parts of a source tree having the same name. The temp filenames would need to be random to avoid potential collisions in such cases, reducing their utility from very near 0 to even closer to 0. Rather than the files being easily accessible (if somewhat annoying) to the user, they'd be off in la-la land somewhere with names the user won't know once those names disappear from the terminal they were hypothetically output to when fossil generated them.

(27) By Warren Young (wyoung) on 2020-02-28 00:49:59 [link] [source] in reply to 26

Developers can and should specify precisely what they want in SCM. IMHO

Which they currently can, via ignore rules.

In other words, if addremove is grabbing stuff you don't want, you've misconfigured your ignore rules.

Just FYI: from the man page of mktemp(3):

Right, so use the mkstemp(3) family instead.

(28) By Stephan Beal (stephan) on 2020-02-28 02:06:29 [link] [source] in reply to 27

Which they currently can, via ignore rules.

In other words, if addremove is grabbing stuff you don't want, you've misconfigured your ignore rules.

ignore-glob doesn't protect against files you might not know exist, like the merge conflict files (until they've bitten you once) or leftover crud from automation or whatever. More precise, in terms of telling fossil what to include, would be a hypothetical include-glob (not that i advocate such a thing - i advocate only that developers specify precisely what they want to add/rm/ci).

(30) By geburashka on 2020-02-28 06:10:56 and edited on 2020-02-28 06:11:27 [link] [source] in reply to 26

Fossil doing exactly what you've told it to do (add *) is a feature, not a bug.

I didn't say it was. I was discussing a workflow in the presence of unexpected generated files vs files you do expect might be generated by a non-fossil command like make.

I believe this is exactly why no other vcs pollutes your repo - nobody expects a version control system to add project files to your project tree which were not explicitly added to the project tree or committed to the repo by a human.

That setting is versioned.

Yes, for one person. Or all people but in one project - unless you remember to add that glob every time you create a new repo; which circles back to the idea that fossil should ignore those files by default (which you can then override if you have a legitimate file called foo-original in that one unique case out of a thousand (million?)).

Just FYI: from the man page of mktemp(3):

Good to know, thanks. However my man page says nothing like that as I'm using the GNU version. In any case, I was using mktemp as a euphemism for the concept, not the implementation detail.

Placing them in /tmp also does not account for files in different parts of a source tree having the same name. The temp filenames would need to be random to avoid potential collisions in such cases,

Different parts of the tree with the same file name will still get unique temporary conflict files when using mktemp. The second sentence is exactly the promise of a mktemp family of commands (bar any broken ones), otherwise it's literally useless.

From the top of my mktemp man page:

mktemp

Create a temporary file or directory, safely...

Rather than the files being easily accessible...

Nobody wants to access them, that's the point. The general consensus here is everyone would be (just as) happy if they were never accessible. In that light, being difficult to access is a fantastic option, to which I also say...

they'd be off in la-la land somewhere with names the user won't know once those names disappear from the terminal

Not if they have sensible names with the dir structure in them eg. /tmp/my-deeply-nested-project-folder-foo-baseline.XXXXXX.

If you really don't like the /tmp idea (though so far I don't see a compelling reason, but I'm not attached to it, was just a thought) then it can be done the git/mercurial/etc way - those files are put next to their originals, as per current fossil behaviour, but only exist for the life of the invoked merge tool; no merge tool? no files.

Git etc can also be instructed to leave those files in the filesystem for manual interaction - when you actually want that.

(32) By Andy Bradford (andybradford) on 2020-02-28 14:18:38 [link] in reply to 30

> I believe this is exactly why no other vcs pollutes your repo - nobody
> expects a version control system to  add project files to your project
> tree which were not explicitly added  to the project tree or committed
> to the repo by a human.

Is not the presence of the .git directory in every clone an example of a
VCS adding files in the project  tree which were not explicitly added or
committed by a human?

$ git init
Initialized empty Git repository in /tmp/masterproject/.git/
$ cd /tmp
$ git clone /tmp/masterproject masterclone
Cloning into 'masterclone'...
warning: You appear to have cloned an empty repository.
done.
$ ls -lan masterclone 
total 12
drwxr-xr-x   3 1000  0  512 Feb 28 07:07 ./
drwxrwxrwt  13 0     0  512 Feb 28 07:16 ../
drwxr-xr-x   7 1000  0  512 Feb 28 07:07 .git/

If one is going to claim that no  VCS puts files in my project tree that
I didn't explicitly  add, then what is that .git  directory doing there?
If one concedes  that the presence of that directory  is a direct result
of expressing  my desire to  clone and that  it necessarily needs  to be
there for  proper functioning of git,  then one can just  as easily make
similar arguments about Fossil's use of merge conflict files.

In all the years  I've been using Fossil, I've never  once had a problem
with the presence  of the conflict files; though, I  wouldn't be opposed
to a change wherein they are made ephemeral.

Personally, I don't  think it's a good idea to  write the merge conflict
files in  /tmp; one  because it  may be on  a different  filesystem with
different permissions, and even different security profiles; two because
it's simpler to implement in the working checkout tree across platforms.

Andy

(31) By Florian Balmer (florian.balmer) on 2020-02-28 13:28:07 [link] [source] in reply to 25

... purpose is debatable (given how many fossil veterans here have never even looked at them), and at best is transient ...

Not looking at the files is not the same as being bugged by them, especially given the various options to get rid of them (ignore-glob, fossil clean --temp, ...).

I see these files as an extra protection layer against data loss, and value that higher than the risk of accidental check-ins of unwanted files. In case of a failed merge, one has to be careful not to issue commands that override the undo history, or reconstructing the underlying file will only be possible from the merge marks in the merge file, and I think this can cause way more grief.

Anyway, the files are now gone for (regular and update) merges. Still, I wonder if the inconsistent behavior between merge│update and stash apply│pop is okay -- or should it after all be a setting, valid for both merge│update and stash apply│pop, so people who really don't want the safety can get consistent behavior?

(33) By Andy Bradford (andybradford) on 2020-02-28 14:26:47 [link] in reply to 31

> > ... purpose is  debatable (given how many fossil  veterans here have
> > never even looked at them), and at best is transient ...
>
> Not looking  at the  files is not  the same as  being bugged  by them,
> especially given the various options  to get rid of them (ignore-glob,
> fossil clean --temp, ...).

That no veteran  fossil user has ever had a  problem with their presence
is made self-evident by the fact that no code has ever been committed to
remove them. :-)

In fact, this  is the first time  in all the years that  I've been using
Fossil---which I can recall---that this has ever come up.

It doesn't mean the OP is wrong, just that he does't like their presence
because they interrupt his workflow.


> I see these files as an  extra protection layer against data loss, and
> value that  higher than the  risk of accidental check-ins  of unwanted
> files.

To the extent that there is  actually some ability to recover in certain
scenarios, I agree with this. I  would much rather have an unwanted file
(committed or otherwise) in my repository than data loss.


Andy

(13) By Stephan Beal (stephan) on 2020-02-21 16:07:55 and edited on 2020-02-21 16:10:35 [link] [source] in reply to 7

There's 2 requests, actually - i would certainly use this (just never thought of it before). The clean command is never an option for me: i don't trust cleanup rules i didn't write into a makefile myself. Having it as a merge option certainly sounds reasonable, but it would probably always be forgotten until after a merge conflict and the annoying extra files were generated.

Edit: nevermind. Replied before seeing Richard's response, which offers a better solution.

(14) By Florian Balmer (florian.balmer) on 2020-02-21 16:53:39 [link] [source] in reply to 13

I also had some distrusts, but then found fossil clean has --dry-run and can be undone. As with most aspects of Fossil, they are usually more intelligent than my own solutions ...