Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
9700026:
Ignore untracked files on Windows (Closed)
|
Created:
8 years, 9 months ago by gab Modified:
8 years, 9 months ago CC:
chromium-reviews, robertshield, erikwright (departed), scottmg Base URL:
svn://svn.chromium.org/chrome/trunk/src Visibility:
Public. |
DescriptionIgnore untracked files on Windows
Some untracked files were created in a clean repo build on Windows.
NOTRY=true
BUG=
TEST=No untracked files left after building all.sln on Windows 7 (VS20120)
Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=126724
Patch Set 1 #
Total comments: 4
Patch Set 2 : move comment back and remove .ico #
Total comments: 4
Patch Set 3 : added /chrome/installer/Release/ #
Total comments: 3
Patch Set 4 : +x64 in comment #Patch Set 5 : ...and ipch/ #
Total comments: 1
Messages
Total messages: 20 (0 generated)
PTAL maruel: Do I need some OWNER approval in src/?
https://chromiumcodereview.appspot.com/9700026/diff/1/.gitignore File .gitignore (right): https://chromiumcodereview.appspot.com/9700026/diff/1/.gitignore#newcode44 .gitignore:44: /c # The Chrome OS build creates a /c symlink due to http://crbug.com/54866. This works? https://chromiumcodereview.appspot.com/9700026/diff/1/.gitignore#newcode214 .gitignore:214: /ui/gfx/test/data/temp_test_icon.ico Don't add this one, it looks like a bug in the testing code.
Fixed. https://chromiumcodereview.appspot.com/9700026/diff/1/.gitignore File .gitignore (right): https://chromiumcodereview.appspot.com/9700026/diff/1/.gitignore#newcode44 .gitignore:44: /c # The Chrome OS build creates a /c symlink due to http://crbug.com/54866. On 2012/03/14 17:56:11, Marc-Antoine Ruel wrote: > This works? Oh I assumed it would (made it easier for sorting), but actually it doesn't, I just don't happen to have c/ in my build.... tested it now and it doesn't work. Good catch. https://chromiumcodereview.appspot.com/9700026/diff/1/.gitignore#newcode214 .gitignore:214: /ui/gfx/test/data/temp_test_icon.ico On 2012/03/14 17:56:11, Marc-Antoine Ruel wrote: > Don't add this one, it looks like a bug in the testing code. Ok, I'll file a bug about this.
lgtm https://chromiumcodereview.appspot.com/9700026/diff/1003/.gitignore File .gitignore (right): https://chromiumcodereview.appspot.com/9700026/diff/1003/.gitignore#newcode64 .gitignore:64: /chrome/installer/Debug/ This is a bug BTW, and you forgot Release. But that's outside the scope of this CL.
https://chromiumcodereview.appspot.com/9700026/diff/1003/.gitignore File .gitignore (right): https://chromiumcodereview.appspot.com/9700026/diff/1003/.gitignore#newcode64 .gitignore:64: /chrome/installer/Debug/ On 2012/03/14 18:17:51, Marc-Antoine Ruel wrote: > This is a bug BTW, and you forgot Release. But that's outside the scope of this > CL. Ok, is this a known bug? Do you want me to add Release / comment on that being a bug?
https://chromiumcodereview.appspot.com/9700026/diff/1003/.gitignore File .gitignore (right): https://chromiumcodereview.appspot.com/9700026/diff/1003/.gitignore#newcode64 .gitignore:64: /chrome/installer/Debug/ On 2012/03/14 18:28:11, gab wrote: > Ok, is this a known bug? Do you want me to add Release / comment on that being a > bug? There is no bug filed AFAIK. I guess it's not using the common gyp because it's using quite different settings. Add Release and a note about that should be inside the global output directory instead.
https://chromiumcodereview.appspot.com/9700026/diff/1003/.gitignore File .gitignore (right): https://chromiumcodereview.appspot.com/9700026/diff/1003/.gitignore#newcode64 .gitignore:64: /chrome/installer/Debug/ On 2012/03/14 18:34:10, Marc-Antoine Ruel wrote: > On 2012/03/14 18:28:11, gab wrote: > > Ok, is this a known bug? Do you want me to add Release / comment on that being > a > > bug? > > There is no bug filed AFAIK. I guess it's not using the common gyp because it's > using quite different settings. Add Release and a note about that should be > inside the global output directory instead. Ok, done.
https://chromiumcodereview.appspot.com/9700026/diff/1006/.gitignore File .gitignore (right): https://chromiumcodereview.appspot.com/9700026/diff/1006/.gitignore#newcode74 .gitignore:74: /chrome/installer/x64/ this one too btw.
https://chromiumcodereview.appspot.com/9700026/diff/1006/.gitignore File .gitignore (right): https://chromiumcodereview.appspot.com/9700026/diff/1006/.gitignore#newcode74 .gitignore:74: /chrome/installer/x64/ On 2012/03/14 20:20:52, Marc-Antoine Ruel wrote: > this one too btw. I think you mean x64 should also go in the comment? I wasn't sure if there was an 80 column limit for .gitignore, but I made it fit anyway by removing the '.' (which I felt was nicer than a multi-line comment...). Let me know should you want me to proceed otherwise.
https://chromiumcodereview.appspot.com/9700026/diff/1006/.gitignore File .gitignore (right): https://chromiumcodereview.appspot.com/9700026/diff/1006/.gitignore#newcode74 .gitignore:74: /chrome/installer/x64/ On 2012/03/14 20:27:25, gab wrote: > On 2012/03/14 20:20:52, Marc-Antoine Ruel wrote: > > this one too btw. > > I think you mean x64 should also go in the comment? In which case ipch/ probably also belong in that group?
On 2012/03/14 20:29:38, gab wrote: > https://chromiumcodereview.appspot.com/9700026/diff/1006/.gitignore > File .gitignore (right): > > https://chromiumcodereview.appspot.com/9700026/diff/1006/.gitignore#newcode74 > .gitignore:74: /chrome/installer/x64/ > On 2012/03/14 20:27:25, gab wrote: > > On 2012/03/14 20:20:52, Marc-Antoine Ruel wrote: > > > this one too btw. > > > > I think you mean x64 should also go in the comment? > > In which case ipch/ probably also belong in that group? You are right, add it to the comment too.
On 2012/03/14 20:30:18, Marc-Antoine Ruel wrote: > On 2012/03/14 20:29:38, gab wrote: > > https://chromiumcodereview.appspot.com/9700026/diff/1006/.gitignore > > File .gitignore (right): > > > > https://chromiumcodereview.appspot.com/9700026/diff/1006/.gitignore#newcode74 > > .gitignore:74: /chrome/installer/x64/ > > On 2012/03/14 20:27:25, gab wrote: > > > On 2012/03/14 20:20:52, Marc-Antoine Ruel wrote: > > > > this one too btw. > > > > > > I think you mean x64 should also go in the comment? > > > > In which case ipch/ probably also belong in that group? > > You are right, add it to the comment too. Done.
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/gab@chromium.org/9700026/4
Change committed as 126724
https://chromiumcodereview.appspot.com/9700026/diff/4/.gitignore File .gitignore (right): https://chromiumcodereview.appspot.com/9700026/diff/4/.gitignore#newcode64 .gitignore:64: # /chrome/installer/(Debug|Release|x64|ipch) should be in the global output directory. So, I was going to fix this, but it seems chrome.sln does the same thing and outputs to chrome\Debug (see (1) above). There are many other solutions that seem to output to their own \Debug (just looking through the current .gitignore, net/ and webkit/ also do this). This seems wrong as I feel like building all.sln should make it so that the other solutions are trivial to rebuild, but as it is right now, opening say chrome.sln with all.sln fully built still implies a full rebuild of chrome.sln... I'll look into this more, but any guidance as to how to specify the output directory in gyp is appreciated!
On 2012/03/16 15:20:46, gab wrote: > https://chromiumcodereview.appspot.com/9700026/diff/4/.gitignore > File .gitignore (right): > > https://chromiumcodereview.appspot.com/9700026/diff/4/.gitignore#newcode64 > .gitignore:64: # /chrome/installer/(Debug|Release|x64|ipch) should be in the > global output directory. > So, I was going to fix this, but it seems chrome.sln does the same thing and > outputs to chrome\Debug (see (1) above). > > There are many other solutions that seem to output to their own \Debug (just > looking through the current .gitignore, net/ and webkit/ also do this). > > This seems wrong as I feel like building all.sln should make it so that the > other solutions are trivial to rebuild, but as it is right now, opening say > chrome.sln with all.sln fully built still implies a full rebuild of > chrome.sln... > > I'll look into this more, but any guidance as to how to specify the output > directory in gyp is appreciated! Seems to be a VS2010 bug. Investigating.
On 2012/03/16 15:57:55, gab wrote: > On 2012/03/16 15:20:46, gab wrote: > > https://chromiumcodereview.appspot.com/9700026/diff/4/.gitignore > > File .gitignore (right): > > > > https://chromiumcodereview.appspot.com/9700026/diff/4/.gitignore#newcode64 > > .gitignore:64: # /chrome/installer/(Debug|Release|x64|ipch) should be in the > > global output directory. > > So, I was going to fix this, but it seems chrome.sln does the same thing and > > outputs to chrome\Debug (see (1) above). > > > > There are many other solutions that seem to output to their own \Debug (just > > looking through the current .gitignore, net/ and webkit/ also do this). > > > > This seems wrong as I feel like building all.sln should make it so that the > > other solutions are trivial to rebuild, but as it is right now, opening say > > chrome.sln with all.sln fully built still implies a full rebuild of > > chrome.sln... > > > > I'll look into this more, but any guidance as to how to specify the output > > directory in gyp is appreciated! > > Seems to be a VS2010 bug. Investigating. Looping in scottmg.
On 2012/03/16 15:57:55, gab wrote: > On 2012/03/16 15:20:46, gab wrote: > > https://chromiumcodereview.appspot.com/9700026/diff/4/.gitignore > > File .gitignore (right): > > > > https://chromiumcodereview.appspot.com/9700026/diff/4/.gitignore#newcode64 > > .gitignore:64: # /chrome/installer/(Debug|Release|x64|ipch) should be in the > > global output directory. > > So, I was going to fix this, but it seems chrome.sln does the same thing and > > outputs to chrome\Debug (see (1) above). > > > > There are many other solutions that seem to output to their own \Debug (just > > looking through the current .gitignore, net/ and webkit/ also do this). > > > > This seems wrong as I feel like building all.sln should make it so that the > > other solutions are trivial to rebuild, but as it is right now, opening say > > chrome.sln with all.sln fully built still implies a full rebuild of > > chrome.sln... > > > > I'll look into this more, but any guidance as to how to specify the output > > directory in gyp is appreciated! > > Seems to be a VS2010 bug. Investigating. I'm not sure if we really want all.sln and chrome.sln overlapping or not. Seems like it might be confusing if different sln's build the same obj/exes. I noticed build_dir_prefix in build/common.gypi, does that handle what you're trying to do?
Le 16 mars 2012 13:59, <scottmg@google.com> a écrit : > On 2012/03/16 15:57:55, gab wrote: > >> On 2012/03/16 15:20:46, gab wrote: >> > https://chromiumcodereview.**appspot.com/9700026/diff/4/.**gitignore<https://... >> > File .gitignore (right): >> > >> > https://chromiumcodereview.**appspot.com/9700026/diff/4/.** >> gitignore#newcode64<https://chromiumcodereview.appspot.com/9700026/diff/4/.gitignore#newcode64> >> > .gitignore:64: # /chrome/installer/(Debug|**Release|x64|ipch) should >> be in the >> > global output directory. >> > So, I was going to fix this, but it seems chrome.sln does the same >> thing and >> > outputs to chrome\Debug (see (1) above). >> > >> > There are many other solutions that seem to output to their own \Debug >> (just >> > looking through the current .gitignore, net/ and webkit/ also do this). >> > >> > This seems wrong as I feel like building all.sln should make it so that >> the >> > other solutions are trivial to rebuild, but as it is right now, opening >> say >> > chrome.sln with all.sln fully built still implies a full rebuild of >> > chrome.sln... >> > >> > I'll look into this more, but any guidance as to how to specify the >> output >> > directory in gyp is appreciated! >> > > Seems to be a VS2010 bug. Investigating. >> > > I'm not sure if we really want all.sln and chrome.sln overlapping or not. > Seems > like it might be confusing if different sln's build the same obj/exes. > That's what was done in VS2008. > I noticed build_dir_prefix in build/common.gypi, does that handle what > you're > trying to do? > > https://chromiumcodereview.**appspot.com/9700026/<https://chromiumcodereview.... >
On Fri, Mar 16, 2012 at 2:02 PM, Marc-Antoine Ruel <maruel@chromium.org>wrote: > Le 16 mars 2012 13:59, <scottmg@google.com> a écrit : > > On 2012/03/16 15:57:55, gab wrote: >> >>> On 2012/03/16 15:20:46, gab wrote: >>> > https://chromiumcodereview.**appspot.com/9700026/diff/4/.**gitignore<https://... >>> > File .gitignore (right): >>> > >>> > https://chromiumcodereview.**appspot.com/9700026/diff/4/.** >>> gitignore#newcode64<https://chromiumcodereview.appspot.com/9700026/diff/4/.gitignore#newcode64> >>> > .gitignore:64: # /chrome/installer/(Debug|**Release|x64|ipch) should >>> be in the >>> > global output directory. >>> > So, I was going to fix this, but it seems chrome.sln does the same >>> thing and >>> > outputs to chrome\Debug (see (1) above). >>> > >>> > There are many other solutions that seem to output to their own \Debug >>> (just >>> > looking through the current .gitignore, net/ and webkit/ also do this). >>> > >>> > This seems wrong as I feel like building all.sln should make it so >>> that the >>> > other solutions are trivial to rebuild, but as it is right now, >>> opening say >>> > chrome.sln with all.sln fully built still implies a full rebuild of >>> > chrome.sln... >>> > >>> > I'll look into this more, but any guidance as to how to specify the >>> output >>> > directory in gyp is appreciated! >>> >> >> Seems to be a VS2010 bug. Investigating. >>> >> >> I'm not sure if we really want all.sln and chrome.sln overlapping or not. >> Seems >> like it might be confusing if different sln's build the same obj/exes. >> > > That's what was done in VS2008. > It seems to be the right thing, the objects are built out of the same source. So why would we want different objects for different solutions? The gyp files are already doing what I think is correct; it simply doesn't reflect in VS2010's behaviour. > > > >> I noticed build_dir_prefix in build/common.gypi, does that handle what >> you're >> trying to do? >> > Yes, that's exactly what we want, but I'm not sure how to patch this so that it works in VS2010. > >> https://chromiumcodereview.**appspot.com/9700026/<https://chromiumcodereview.... >> > > |
