`mach clobber` shouldn't clobber the generated Visual Studio project files. It takes too long to reindex after a clobber and most of the time it isn't necessary. The generated project should live outside the objdir. I've been using Visual Studio with the solution outside the objdir and the solution never being clobbered for 3 years until now and it seems to work very well this way.

1. This causes "mach clobber" to fail when you have the solution open in MSVS2013. This causes trouble. 2. When you *do* run "mach clobber && mach build," which is something that is required multiple times per week (sometimes multiple times per day) you cannot do useful work within Visual Studio because the project is not there yet.

3. Clobbering the generated project and then re-generating it causes indexing to reset from zero. But, indexing from zero is too slow.

This simple change appears to work for me, without ant issues. Changed the directory name to .msvc, so it's more obvious that it is not part of the tracked files. Added to .hgignore and .gitignore. I've never used either of these before, so hopefully the syntax is correct.

Comment on attachment 8628220 [details] [diff] [review] Move the generated Visual Studio solution out of the obj directory. Review of attachment 8628220 [details] [diff] [review]: ----------------------------------------------------------------- Ted and I talked about this at Whistler after this question was asked to me and we came up with a different solution but I was unable to follow up because I didn't know who asked the question. Anyway, the visual studio project files are tied to a specific build configuration. e.g. the project files for debug builds will vary from non-debug builds. If we put the project files in the source directory, this works for workflows that have a single object directory per source directory. However, some people have multiple object directories per source directory and this change would make Visual Studio unusable to them. The relatively simple solution we came up with was to change the default behavior of `mach clobber` to ignore certain directories in the objdir, starting with "msvc." i.e. `mach clobber` would remove everything except "objdir/msvc." We'd likely want to introduce a `mach clobber --full` or some such to force a delete of the entire objdir. You can find the `mach clobber` code in python/mozbuild/mozbuild/mach_commands.py. If this is beyond your Python skill level, let me know - I could probably whip it up in a few minutes.

(In reply to Gregory Szorc [:gps] from comment #4) > Ted and I talked about this at Whistler after this question was asked to me > and we came up with a different solution but I was unable to follow up > because I didn't know who asked the question. It was Jim Mathies that asked about this. > The relatively simple solution we came up with was to change the default > behavior of `mach clobber` to ignore certain directories in the objdir, > starting with "msvc." i.e. `mach clobber` would remove everything except > "objdir/msvc." We'd likely want to introduce a `mach clobber --full` or some > such to force a delete of the entire objdir. How about using?: self._out_dir = os.path.join(self.environment.topsrcdir, '.msvc', os.path.relpath(self.environment.topobjdir, self.environment.topsrcdir)) Then you wouldn't need to change clobber. > You can find the `mach clobber` code in > python/mozbuild/mozbuild/mach_commands.py. If this is beyond your Python > skill level, let me know - I could probably whip it up in a few minutes. I'm no python expert, but I could give it a go, it would be next week though (at least). It would definitely take me more than a few minutes.

I'd prefer we not store more state in the source directory. We want to be trending towards the ability to build from a read-only source directory. Also, if you add .msvc, someone else will want to add .eclipse. And we have a slippery slope. This is all build-specific state and IMO should be stored inside the objdir because the objdir is our container for build-specific state.

Too many other things are piling up, so de-assigning for the moment.

Here's my patch with the solution moved up a level, but with individual directories to allow for different build configs. I spoke to Jim about this in passing in Orlando and he felt that it shouldn't go in the obj directory, I think mainly because people use other methods than mach clobber to clear their obj directory. I'll let Jim chime in with anything else.

(In reply to Gregory Szorc [:gps] from comment #4) > The relatively simple solution we came up with was to change the default > behavior of `mach clobber` to ignore certain directories in the objdir, > starting with "msvc." i.e. `mach clobber` would remove everything except > "objdir/msvc." We'd likely want to introduce a `mach clobber --full` or some > such to force a delete of the entire objdir. FWIW, I don't use mach clobber because it is very slow. I use a batch script that renames obj folders to backup names and then invokes a series of Windows command line tools to purge the folders quickly. Two advantages to this, the Windows remove directory tools I invoke are substantially faster than anything else available, and I can start building immediately without waiting for delete operations to complete. Because I use this method, the auto-generated visual studio files are unusuable. If I do use the auto-generated files, every time I clobber (once a week or so on average) I have to shut down vs and lose my project and environmental settings. As a Windows dev who uses vs exclusively I invest a substantial amount of time in setting vs up the way I like it, so this is a deal killer for me. I've stopped using the auto-generated feature and have gone back to maintaining my own Mozilla visual studio project. I'd like to propose we land Bob's work here. It far exceeds the current solution in terms of usability, and devs shouldn't have much trouble working around the corner case idiosyncrasies.

We talked about this at a build system work week and decided that we're going to change the behavior of `mach clobber` to clobber everything except a whitelist of directories and the visual studio files will be in that whitelist. There's a risk that changing to N invocations of rm.exe will slow things down. If that happens, we'll cross that bridge if we come to it. Speaking of rm.exe, the latest MozillaBuild ships a special rm executable that uses Win32 APIs that delete directories efficiently. When you complain about `mach clobber` being slow, that is surprising to me.

(In reply to Gregory Szorc [:gps] from comment #10) > Speaking of rm.exe, the latest MozillaBuild ships a special rm executable > that uses Win32 APIs that delete directories efficiently. When you complain > about `mach clobber` being slow, that is surprising to me. Maybe the remove operation isn't, I haven't compared the timing. The scripts I use don't care about that operation though, they rename the folder and then delete it. These scripts are also batch files so I run and minimize them and start a build immediately. here's the batch script for a mc clobber on my system: move F:\Mozilla\DBG F:\Mozilla\DBG_deltmp move F:\Mozilla\REL F:\Mozilla\REL_deltmp del F:\Mozilla\DBG_deltmp /F /Q > nul del F:\Mozilla\REL_deltmp /F /Q > nul rmdir F:\Mozilla\DBG_deltmp /S /Q rmdir F:\Mozilla\REL_deltmp /S /Q

`mach clobber` only got fixed to use winrm ~8 months ago: bug 1162519. It would be interesting if you could test if it's still slow for you.

I'll let you know. It's not going to as fast as a background delete of a renamed obj dir though. :) But if I need to live with waiting for clobbers to use the project files it's a pretty minor productivity sacrifice for a big productivity win.

I'll try to hack on this this week.

My Python editor told me rmtree has been marked as deprecated. Use mozfile.remove instead. Review commit: https://reviewboard.mozilla.org/r/35971/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/35971/

The "remove_objdir" method has been rewritten to support partial clobber. It still defaults to full clobber. But the "full" argument can be passed as False to limit to a partial clobber where currently the "msvc" directory (contains Visual Studio files) is not clobbered. On Windows, there might be a regression with this change because we'll be invoking N winrm.exe processes and new processes have a non-trivial overhead on Windows. However, it is hopefully unlikely that new processes are more overhead than deleting hundreds of thousands of files. Review commit: https://reviewboard.mozilla.org/r/35973/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/35973/

(In reply to Ted Mielczarek [:ted.mielczarek] from comment #12) > `mach clobber` only got fixed to use winrm ~8 months ago: bug 1162519. It > would be interesting if you could test if it's still slow for you. eh, not great, not awful. mach clobber of a release obj dir took about 2 min. 15 seconds. My scripts blew away a debug obj dir in about 1 minute 15 seconds.

Comment on attachment 8722291 [details] Bug 974281 - Use mozfile.remove since rmtree is deprecated; https://reviewboard.mozilla.org/r/35971/#review35419

Comment on attachment 8722292 [details] Bug 974281 - Do not clobber msvc directory by default; https://reviewboard.mozilla.org/r/35973/#review35425 Did you compare timings of this vs. the existing single-invocation? It would be good to at least sanity check that it's not a huge regression. ::: python/mozbuild/mozbuild/base.py:353 (Diff revision 1) > + subprocess.check_call(['winrm', '-rf', path]) One fairly simple optimization here would be to run the processes using `subprocess.Popen` instead of `check_call`, and then if we have psutil, use `psutil.wait_procs` to wait for the whole set of them (if we don't have psutil you could just loop over the list and call `wait` on each in turn). This way we'd be running all the rm processes in parallel.

Comment on attachment 8722291 [details] Bug 974281 - Use mozfile.remove since rmtree is deprecated; Review request updated; see interdiff: https://reviewboard.mozilla.org/r/35971/diff/1-2/

Comment on attachment 8722292 [details] Bug 974281 - Do not clobber msvc directory by default; Review request updated; see interdiff: https://reviewboard.mozilla.org/r/35973/diff/1-2/

Hey gps, can we get these landed?

Comment on attachment 8722291 [details] Bug 974281 - Use mozfile.remove since rmtree is deprecated; Review request updated; see interdiff: https://reviewboard.mozilla.org/r/35971/diff/2-3/

Comment on attachment 8722292 [details] Bug 974281 - Do not clobber msvc directory by default; Review request updated; see interdiff: https://reviewboard.mozilla.org/r/35973/diff/2-3/

Comment on attachment 8722292 [details] Bug 974281 - Do not clobber msvc directory by default; ted: can you please look at this again since the rebase changed things considerably?

Comment on attachment 8722292 [details] Bug 974281 - Do not clobber msvc directory by default; https://reviewboard.mozilla.org/r/35973/#review70358

Pushed by gszorc@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/a09ef1eb1d63 Use mozfile.remove since rmtree is deprecated; r=ted https://hg.mozilla.org/integration/autoland/rev/c4c26c0af171 Do not clobber msvc directory by default; r=ted

https://hg.mozilla.org/mozilla-central/rev/a09ef1eb1d63 https://hg.mozilla.org/mozilla-central/rev/c4c26c0af171

This landed. Cancelling needinfo.