When using "arc patch" command the ".orig" file is created for every patched file. Since changed files are under VCS control anyway, then creating ".orig" files only creates problems, because I need to go and delete each one manually before doing a commit.
I suppose it's "-b" argument of patch command that creates there. Can you please not create these?
Can you provide a list of steps to reproduce the issue? Using arc patch on git, I have not been able to repro this.
I'm using SVN 1.6 working copy.
{
"phabricator.uri" : "http://qa.in-portal.org/"
}You'll see, that not only core/kernel/db/db_tag_processor.php file is changed (as it should), but also core/kernel/db/db_tag_processor.php.orig file is created.
I think this isn't something specific to particular repo, but to SVN repository handling in general.
I've debugged arc patch command and defietely after https://github.com/phacility/arcanist/blob/master/src/workflow/ArcanistPatchWorkflow.php#L582-L587 lines are invoked I got my files patched and .orig files present.
I've tried to download patch via wget http://qa.in-portal.org/D13?download=true and then did patch -p0 < D13.diff and also got .orig files created.
From the http://linux.die.net/man/1/patch I've found out, that --backup-if-mismatch option is default for patch command and results in .orig files to be created because files to be patched doesn't at 100% match ones from which patch file was created. Doesn't sound like great idea, because usually you can't guarantee that patches will be applied in same order as they were created and line numbers will exactly match.
I can confirm, that adding --no-backup-if-mismatch option to patch command solved the problem.
This is working as intended on our side, and glad you've found a workaround for yourself.
That's no workaround, because actual patch command is invoked from within the Arcanist. That argument is needed to be added in it.
@chad, so you're saying that following my instructions you don't get .orig files created? Which patch version is on your side? Mine is:
GNU patch 2.7 Copyright (C) 2003, 2009-2012 Free Software Foundation, Inc. Copyright (C) 1988 Larry Wall License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html>. This is free software: you are free to change and redistribute it. There is NO WARRANTY, to the extent permitted by law. Written by Larry Wall and Paul Eggert
Maybe that behavior was changed in your version.
That's the thing: there are no conflict. You can see that lines are shifted a bit in a file and with fuzz factor of 1 hunk was applied. You can see that clearly on my scenario.
Also same patch file failed to apply with following message, when patch program version is 2.5.4 (default on CentOS):
Hunk #1 succeeded at 1156 (offset 1 line). patch: util.c:426: savebuf: Assertion `s && size' failed. Aborted
In http://stackoverflow.com/questions/11519230/apply-svn-patch-to-git-repository article I've found that this in fact was a bug in older patch command versions when trying to apply patches that have No new line at the end of a file text.
Can you cleanly arc patch from secure.phabricator.com? I'm running 2.5.8. You could also download your diff and try to manually pass the file to patch.
Can you cleanly arc patch from secure.phabricator.com?
Yes I can. No .orig files are created. But what I've noticed, that Arcanist have created branch for me (because it's a Git repo) from the commit, that was last at the moment of patch creation. So of course applying was 100% accurate without .orig files. With SVN this is not the case. In SVN you always are on latest commit in current branch and of course some issues with moving lines during patch apply can happen.
You could also download your diff and try to manually pass the file to patch.
As I said before I'm getting same .orig files created in this case as well.
So it's the git repo workflow (with branch creation) that is preventing .orig files from being created in your case.
Due the inability to split changes by branches (like in Git) my SVN workflow is to have multiple files changed related to several issues at same time. I group them together using SVN changelist feature. Unfortunately arc diff doesn't take SVN changelists into account and creating revision based on all changes files in working copy, which is not what I need. Because of this I create patch from changeset manually and upload it to Differential via Web Interface. In this scenario Differential can see the SVN revisions in patch file, but it doesn't know the repository they are associated with and therefore has empty context.
If patch itself is failing and we can detect a bad binary of patch, we'd look into doing that. If it's just a workflow issue that you have a preference, we're unlikely to change the defaults of Phabricator as many other developers already rely on existing behaviour.
Do you have any Phabricator user, who use SVN repositories? I wonder what workflow they are using.
Without the --no-backup-if-mismatch option (this is current Arcanist behavior) the patch is applied successfully and Arcanist showing green OKAY word. So fact, that .orig files are created doesn't mean any problem. If there was a problem during patch apply presence of --no-backup-if-mismatch won't change the outcome.
So adding --no-backup-if-mismatch to patch command, when working with SVN repositories won't really harm anybody. It's safe to try. If you'll have any bug reports after that you can easily rollback the change. Like one that was with quoted printable encoding in PhpMailer few months back.
does arc patch under SVN checks that the workspace is clean? In other words, can there be local, uncommitted changes when it calls patch? In there are, --no-backup can potentially lose them.
You can also try setting envvar POSIXLY_CORRECT (to anything), which should make --no-backup-of-mismatch the default.
does arc patch under SVN checks that the workspace is clean?
Sort of. It does tell me that some files are not under VCS control and if I want to add them. But it doesn't really complain about fact, that I have some locally modified files. I don't know if Arcanist does check every file mentioned in patch for local modifications before invoking patch command though.
You can also try setting envvar POSIXLY_CORRECT (to anything), which should make --no-backup-of-mismatch the default.
I'm not sure what side effects of this will be, because description of this environment variable on Wikipedia is very complex and doesn't specifically mention, that it affects only patch command.
Care to share why? Also what about SVN users of Phabricator? Do they have the same issue?
Have you tried the workflow I've explained in https://secure.phabricator.com/T6424#81369 to see that it caused .orig files to be created on your side as well?
We have no other reports or complaints on the issue you state. Please fork Phabricator or maintain local patches if you desire different behaviour.
But what is really preventing you from adding that argument for SVN workflow? It won't hurt anybody. Not adding this just because it's an answer. As I said before you can always revert if something will come up.
We're not writing Flight Control system here, errors can happen, nobody gets killed.
P.S.
Local fork is never an option because of amount of code changes are made to Phabricator every day.
Do you ever directly answer to asked questions instead universal: do this in your local fork & we're not interested answers :(
Local fork is never an option because of amount of code changes are made to Phabricator every day.
This doesn't seem accurate. Why is it OK to waste hours of Phabricator developer time per day on personal requests rather than the 5 minutes maintaining a fork? Many, maybe even most installs, maintain a fork. It is not as hard as you make it out to be.
We're not writing Flight Control system here, errors can happen, nobody gets killed.
We don't subscribe to this philosophy of code development. Phabricator, specifically, is a tool to produce quality code. Disrupting the expected defaults diminishes the trust other companies and developers put into our tool.
But what is really preventing you from adding that argument for SVN workflow?
It is not the correct default for Vanilla Phabricator.
On a personal note, please take time and try to be respectful of our feedback and decisions. If other people who use Phabricator agree with you, they will speak up on these threads and we'll be happy to re-address as those comments come in. We're a team of 3 unpaid people who build this for millions of people for free.
Because it is free, open, and easy to fork, we expect people to modify the source as they see fit. This gives us more time building larger and newer features rather than chasing a long tail of minor nuances from different developers.
On a personal note, please take time and try to be respectful of our feedback and decisions.
I honestly try. But when 50% of reported issues (at least reported by me) gets never fixed making me more unhappier with each time. I try to keep positive thinking, but it's kind of hard.
We're a team of 3 unpaid people who build this for millions of people for free.
I recommend increasing team size. As my friend said: open source isn't equal to free. It's like Symfony: it's open source but company is paying employees to develop it. I guess same happens here in one way or another. There are always ways to bring $ into company if there is a need.
... they will speak up on these threads and we'll be happy to re-address as those comments come in.
Never seen this happening on issues reported by me, but this is valid point anyway.
Because it is free, open, and easy to fork, we expect people to modify the source as they see fit.
Having more documentation on how to fork Phabricator and what code to change to get desired effect would surely help.
We understand that we need to grow, which is why we've been shifting focus to launching Phacility on our short term radar.
D10764 goes into much more detail on Phabricator itself, requesting features, and explaining how the upstream works.
Maintaining a fork is fairly common and not really specific to Phabricator, I imagine there are many guides on the web. If your changes are small, this will be fairly easy.