I can't reproduce this. Here's what I did:

I made a new repository with a submodule:

$ mkdir example
$ cd example
example/ $ git init
example/ $ git commit --allow-empty -m "Initial Commit"
example/ $ git submodule add https://github.com/phacility/libphutil.git
example/ $ git commit -m "Add Submodule"

I created an example commit in this repository (I want someone to review my changes to file.c):

example/ $ nano file.c
example/ $ git add file.c
example/ $ git commit -m "Add file.c"

Then I ran arc diff on it:

example/ $ arc diff HEAD^ --conduit-uri=http://local.phacility.com/
Linting...
No lint engine configured for this project.
Running unit tests...
No unit test engine is configured for this project.
 SKIP STAGING  Unable to determine repository for this change.
Updating commit message...
Created a new Differential revision:
        Revision URI: http://local.phacility.com/D73

Included changes:
  A       file.c

From your description, it sounds like this detects that the submodule has changes for you even when it does not. However, it didn't do this for me.

Do you have a different set of steps I can follow to reproduce this issue, or do you see what I did differently?

Uhg, it only shows up if there are changes in the submodule. So things like the .iml files will trigger it to show that and then cause an arc prompt for asking about it

Ah, okay, so the missing steps are something like this:

$ nano libphutil/README.md # make some change

$ git status
On branch master
Changes not staged for commit:
  (use "git add <file>..." to update what will be committed)
  (use "git checkout -- <file>..." to discard changes in working directory)
  (commit or discard the untracked or modified content in submodules)

	modified:   libphutil (modified content)

no changes added to commit (use "git add" and/or "git commit -a")

And we're conflating "(modified content)" in a submodule with this state (when the base commit of the submodule changes):

$ git status
On branch master
Changes not staged for commit:
  (use "git add <file>..." to update what will be committed)
  (use "git checkout -- <file>..." to discard changes in working directory)

	modified:   libphutil (new commits)

no changes added to commit (use "git add" and/or "git commit -a")

In the "modified content" state, we produce a meaningless prompt:

$ arc diff HEAD^ --conduit-uri=http://local.phacility.com/ --create
You have uncommitted changes in this working copy.

  Working copy: /Users/epriestley/dev/scratch/example/

  Unstaged changes in working copy:
    libphutil


    Do you want to create a new commit with this change? [y/N]

This would make sense from the "new commits" state, but should just say "ignore the submodule changes and continue" in the "modified content" state, I think.

To solve your specific problem, are .iml files ignorable (i.e., can you exclude them from version control)? I haven't used whatever generates them (IntelliJ IDEA?), but if it's possible to just add them to .gitignore that might solve this (or maybe there are ways to configure whatever IDE you're using to avoid this problem, since it seems like every project in the world that uses both the IDE and any VCS must be subject to it).

I think we can detect this state in a porcelain way by testing whether the old and new commit hashes are identical. That is, it looks like we get this for "(modified content)":

:160000 160000 880c0fb... 880c0fb... M  libphutil

...and this for "(new commits)" (which really means "base commit changed"; it could be moved backward in history):

:160000 160000 880c0fb... e838909... M  libphutil

From the Git source, it looks like there's some more state here:

	case WT_STATUS_CHANGED:
		if (d->new_submodule_commits || d->dirty_submodule) {
			strbuf_addstr(&extra, " (");
			if (d->new_submodule_commits)
				strbuf_addf(&extra, _("new commits, "));
			if (d->dirty_submodule & DIRTY_SUBMODULE_MODIFIED)
				strbuf_addf(&extra, _("modified content, "));
			if (d->dirty_submodule & DIRTY_SUBMODULE_UNTRACKED)
				strbuf_addf(&extra, _("untracked content, "));
			strbuf_setlen(&extra, extra.len - 2);
			strbuf_addch(&extra, ')');
		}

But we probably don't care about the distinction between DIRTY_SUBMODULE_MODIFIED and DIRTY_SUBMODULE_UNTRACKED in arc.

So I talked to some of the other people working on my project and we're ok with .gitignore'ing the .iml file in our submodule so it will get rid of the issue if you don't have any changes and if you have changes then you either have to live with the prompt or commit to the submodule first.

I'd say it is resolved for now, but I'll let you know if people take issue with it and want a different solution. Thanks for the help! :)

It would definitely be nice to not be prompted on updates to the diff if the commits are identical.

Our behavior here is strictly wrong. We ask you a question:

Do you want to create a new commit with this change? [y/N]

When you answer "y", we don't do what we said we would (since the question is meaningless in context). We'll fix this.

Awesome, thanks :)

Actually it looks like these states aren't distinguishable from git diff --raw:

• Submodule commit has changed, changed commit state has been staged.
• Submodule commit has changed, changed commit state has been staged, submodule also has local uncommitted or untracked changes.

From grep'ing the git source, this distinction (the DIRTY_SUBMODULE_MODIFIED flag) only appears to be exposed in non-porcelain UI (i.e., messages intended for human consumption, specifically the readable output of git status and the readable output of git diff --submodule, which is overridden by the --raw flag, at least at my version of Git).

Although it's possible to parse git status or git diff --submodule to figure this out, it isn't possible to do it in a language-agnostic way, and we've tackled this issue so far by avoiding English-dependent calls everywhere (see T5554) because forcing English is complicated. I'm loathe to introduce English-dependent calls here.

So I think we'll get this slightly wrong sometimes (if you've changed the submodule's base commit and then made additional changes to the files inside it, we may fail to detect the uncommitted or untracked changes), and err on the side of missing changes, which is sort of bad, but seems not completely unreasonable.

Git itself apparently figures this out by running git status --porcelain in the submodule, so we could do that too, but that's a fairly large amount of work.

Darn :( Introducing language specific calls sounds like a very complicated and fragile change for something like this. I don't think it will be too big of a deal :) Thanks for looking into it.

D14137 replaces the prompt with this one:

$ arc diff HEAD^ --create --conduit-uri=http://local.phacility.com
A submodule has uncommitted or untracked changes:

      - libphutil

    Ignore the changes to this submodule and continue? [Y/n]

This doesn't actually solve your original problem, per se: you'll still get the prompt every time. But the prompt is no longer wrong/misleading, and it sounds like you have a .gitignore-based solution for the original problem.

That sounds great to me :)

D14137 (with the new prompt) is now in master. It will promote to stable in about a week. You can grab it with arc upgrade.

D14136 included a fairly aggressive rewrite of some aspects of how we do console rendering. One goal of that change is to eventually resolve T8243. Let us know if you spot console rendering issues, most likely in the form of ANSI color sequences or whitespace characters being incorrectly escaped in interfaces where they are application-controlled and safe.