Masatoshi Kimura [:emk] Assignee
	Description • 7 years ago

Steps to reproduce:
1. Set WIN_UCRT_REDIST_DIR in .mozconfig.
2. ./mach clobber && ./mach build && ./mach build
   (Run ./mach build twice)

Actual result:
$ mach build
 0:01.01 d:\mozilla-build\mozmake\mozmake.EXE -f client.mk -s
 0:03.73 Adding client.mk options from e:/m/mozilla-unified/.mozconfig:
 0:03.73     AUTOCLOBBER=1
 0:03.73     CONFIG_GUESS=x86_64-pc-mingw32
 0:03.73     MOZ_OBJDIR=e:/m/mozilla-unified/obj-x86_64-pc-mingw32
 0:03.73     OBJDIR=e:/m/mozilla-unified/obj-x86_64-pc-mingw32
 0:03.73     FOUND_MOZCONFIG=e:/m/mozilla-unified/.mozconfig
 0:04.70 Build configuration changed. Regenerating backend.
 1:10.93 Reticulating splines...
 1:10.94 e:\m\mozilla-unified\python\mozbuild\mozbuild\frontend\gyp_reader.py:385: UserWarning: MSVS_VERSION being set to 2015 to appease GYP
 1:10.94   warnings.warn('MSVS_VERSION being set to 2015 to appease GYP')
 1:10.94 Finished reading 1339 moz.build files in 31.32s
 1:10.94 Read 119 gyp files in parallel contributing 0.00s to total wall time
 1:10.94 Processed into 10971 build config descriptors in 10.14s
 1:10.94 RecursiveMake backend executed in 11.92s
 1:10.94   3234 total backend files; 0 created; 3 updated; 3231 unchanged; 0 deleted; 56 -> 1197 Makefile
 1:10.94 FasterMake backend executed in 0.44s
 1:10.94   18 total backend files; 0 created; 0 updated; 18 unchanged; 0 deleted
 1:10.94 VisualStudio backend executed in 8.28s
 1:10.94 Generated Visual Studio solution at e:/m/mozilla-unified/obj-x86_64-pc-mingw32\msvc\mozilla.sln
 1:10.94 Total wall time: 64.93s; CPU time: 64.93s; Efficiency: 100%; Untracked: 2.83s
 1:12.46 Elapsed: 0.01s; From dist/public: Kept 0 existing; Added/updated 0; Removed 0 files and 0 directories.
 1:12.47 Elapsed: 0.00s; From dist/private: Kept 0 existing; Added/updated 0; Removed 0 files and 0 directories.
 1:12.47 Elapsed: 0.06s; From dist/branding: Kept 12 existing; Added/updated 0; Removed 0 files and 0 directories.
 1:12.53 Elapsed: 0.04s; From dist/xpi-stage: Kept 14 existing; Added/updated 0; Removed 0 files and 0 directories.
 1:13.61 Elapsed: 1.10s; From _tests: Kept 0 existing; Added/updated 1045; Removed 0 files and 0 directories.
 1:13.77 Elapsed: 1.36s; From dist/idl: Kept 0 existing; Added/updated 1034; Removed 0 files and 0 directories.
 1:14.94 Traceback (most recent call last):
 1:14.94   File "d:\mozilla-build\python\Lib\runpy.py", line 174, in _run_module_as_main
 1:14.94     "__main__", fname, loader, pkg_name)
 1:14.94   File "d:\mozilla-build\python\Lib\runpy.py", line 72, in _run_code
 1:14.94     exec code in run_globals
 1:14.94   File "e:\m\mozilla-unified\python\mozbuild\mozbuild\action\process_install_manifest.py", line 111, in <module>
 1:14.94     main(sys.argv[1:])
 1:14.94   File "e:\m\mozilla-unified\python\mozbuild\mozbuild\action\process_install_manifest.py", line 98, in main
 1:14.95     defines=args.defines)
 1:14.95   File "e:\m\mozilla-unified\python\mozbuild\mozbuild\action\process_install_manifest.py", line 69, in process_manifest
 1:14.95     remove_empty_directories=remove_empty_directories)
 1:14.95   File "e:\m\mozilla-unified\python\mozbuild\mozpack\copier.py", line 449, in copy
 1:14.95     os.remove(f)
 1:14.95 WindowsError: [Error 5] アクセスが拒否されました。: u'd:Program Files (x86)\\Windows Kits\\10\\Redist\\ucrt\\DLLs\\x64\\api-ms-win-crt-time-l1-1-0.dll'
 1:14.95 mozmake.EXE[3]: *** [Makefile:178: install-dist/bin] Error 1
 1:14.95 mozmake.EXE[3]: *** Waiting for unfinished jobs....
 1:16.94 Elapsed: 4.51s; From dist/include: Kept 0 existing; Added/updated 5544; Removed 0 files and 0 directories.
 1:16.95 mozmake.EXE[2]: *** [e:/m/mozilla-unified/config/recurse.mk:33: pre-export] Error 2
 1:16.95 mozmake.EXE[1]: *** [e:/m/mozilla-unified/config/rules.mk:432: default] Error 2
 1:16.96 mozmake.EXE: *** [client.mk:269: build] Error 2
 1:16.99 134 compiler warnings present.
2

Expected result:
Build should succeed.

I had to modify NFS acl of the Redist\ucrt\DLLs\x64 folder to prevent ./mach from deleting the folder content.

Reverting https://hg.mozilla.org/mozilla-central/rev/3a181cd6052b fixed the issue.

	Masatoshi Kimura [:emk] Assignee
	Comment 1 • 7 years ago

I found the following line in obj-x86_64-pc-mingw32\_build_manifests\install\dist_bin which looks very wrong to me:
5d:/Program Files (x86)/Windows Kits/10/Redist/ucrt/DLLs/x64/api-ms-win-*.dll/d:/Program Files (x86)/Windows Kits/10/Redist/ucrt/DLLs/x64api-ms-win-*.dll

	Masatoshi Kimura [:emk] Assignee
	Comment 3 • 7 years ago

(In reply to Masatoshi Kimura [:emk] from comment #0)
> I had to modify NFS acl

NTFS acl

	Michael Shal [:mshal]
	Comment 4 • 7 years ago
mozreview-review

Comment on attachment 8927627 [details]
Bug 1416465 - Expand pattern when track file is created rather than read.

https://reviewboard.mozilla.org/r/198936/#review204610

	Pulsebot
	Comment 5 • 7 years ago

Pushed by VYV03354@nifty.ne.jp:
https://hg.mozilla.org/integration/autoland/rev/6f0e650b0e53
Fix PATTERN_LINK and PATTERN_COPY entries of install manifests. r=mshal

	Noemi Erli[:noemi_erli]
	Comment 6 • 7 years ago
bugherder

https://hg.mozilla.org/mozilla-central/rev/6f0e650b0e53

	:Grisha Kruglov
	Comment 7 • 7 years ago

This patch seems to cause Bug 1417626 (android build failures on Linux). Reversing it gets things to work again.

	:Grisha Kruglov
	Comment 8 • 7 years ago

Added some logging for paths being passed into `_add_entry` before (working) and after (broken) this patch:

 0:06.31 DEBUGRI link WORKING /home/grisha/Code/mozilla-central/testing/mozbase/manifestparser/**/mozbase
 0:06.31 DEBUGRI link BROKEN mozbase/manifestparser/**
 0:06.31 DEBUGRI link WORKING /home/grisha/Code/mozilla-central/testing/mozbase/mozcrash/**/mozbase
 0:06.31 DEBUGRI link BROKEN mozbase/mozcrash/**
 0:06.31 DEBUGRI link WORKING /home/grisha/Code/mozilla-central/testing/mozbase/mozdebug/**/mozbase
 0:06.31 DEBUGRI link BROKEN mozbase/mozdebug/**
 0:06.31 DEBUGRI link WORKING /home/grisha/Code/mozilla-central/testing/mozbase/mozdevice/**/mozbase
 0:06.31 DEBUGRI link BROKEN mozbase/mozdevice/**
 0:06.31 DEBUGRI link WORKING /home/grisha/Code/mozilla-central/testing/mozbase/mozfile/**/mozbase
 0:06.31 DEBUGRI link BROKEN mozbase/mozfile/**
....
 0:07.47 DEBUGRI link WORKING /home/grisha/Code/mozilla-central/security/manager/ssl/tests/unit/test_cert_embedded_null/**/xpcshell/./security/manager/ssl/tests/unit
 0:07.47 DEBUGRI link BROKEN xpcshell/./security/manager/ssl/tests/unit/test_cert_embedded_null/**
 0:07.47 DEBUGRI link WORKING /home/grisha/Code/mozilla-central/security/manager/ssl/tests/unit/test_cert_isBuiltInRoot_reload/**/xpcshell/./security/manager/ssl/tests/unit
 0:07.47 DEBUGRI link BROKEN xpcshell/./security/manager/ssl/tests/unit/test_cert_isBuiltInRoot_reload/**
 0:07.47 DEBUGRI link WORKING /home/grisha/Code/mozilla-central/security/manager/ssl/tests/unit/test_cert_keyUsage/**/xpcshell/./security/manager/ssl/tests/unit
 0:07.47 DEBUGRI link BROKEN xpcshell/./security/manager/ssl/tests/unit/test_cert_keyUsage/**

	Ed Lee :Mardak
	Comment 9 • 7 years ago

Activity Stream is hitting this and reverting the change gets rid of the following errors:
 0:01.17 mozpack.errors.ErrorMessage: Error: chrome/content/prerendered/static/activity-stream-initial-state.js already added
 0:01.43 mozpack.errors.ErrorMessage: Error: chrome/content/lib/UITour-lib.js already added

$ curl https://hg.mozilla.org/mozilla-central/raw-rev/6f0e650b0e53 | patch -Rp1
 0:18.59 Your build was successful!

	Ed Lee :Mardak
	Comment 10 • 7 years ago

With a clean checkout of mozilla-central https://hg.mozilla.org/mozilla-central/rev/f41930a869a8 artifact build:

$ ./mach build
…
 0:45.86 Your build was successful!

$ ./mach build faster
…
0:01.18 Traceback (most recent call last):
 0:01.18   File "/usr/local/Cellar/python/2.7.14/Frameworks/Python.framework/Versions/2.7/lib/python2.7/runpy.py", line 174, in _run_module_as_main
 0:01.18     "__main__", fname, loader, pkg_name)
 0:01.18   File "/usr/local/Cellar/python/2.7.14/Frameworks/Python.framework/Versions/2.7/lib/python2.7/runpy.py", line 72, in _run_code
 0:01.18     exec code in run_globals
 0:01.18   File "/Users/Ed/Development/mozilla-central/python/mozbuild/mozbuild/action/process_install_manifest.py", line 111, in <module>
 0:01.18     main(sys.argv[1:])
 0:01.18   File "/Users/Ed/Development/mozilla-central/python/mozbuild/mozbuild/action/process_install_manifest.py", line 98, in main
 0:01.18     defines=args.defines)
 0:01.18   File "/Users/Ed/Development/mozilla-central/python/mozbuild/mozbuild/action/process_install_manifest.py", line 45, in process_manifest
 0:01.19     remove_unaccounted.add(p, dummy_file)
 0:01.19   File "/Users/Ed/Development/mozilla-central/python/mozbuild/mozpack/copier.py", line 69, in add
 0:01.19     return errors.error("%s already added" % path)
 0:01.19   File "/Users/Ed/Development/mozilla-central/python/mozbuild/mozpack/errors.py", line 106, in error
 0:01.19     self._handle(self.ERROR, msg)
 0:01.19   File "/Users/Ed/Development/mozilla-central/python/mozbuild/mozpack/errors.py", line 98, in _handle
 0:01.19     raise ErrorMessage(msg)
 0:01.19 mozpack.errors.ErrorMessage: Error: chrome/content/prerendered/static/activity-stream-initial-state.js already added
 0:01.20 make: *** [install-dist/bin/browser/features/activity-stream@mozilla.org] Error 1
 0:01.20 make: *** Waiting for unfinished jobs....
 0:01.38 Traceback (most recent call last):
 0:01.38   File "/usr/local/Cellar/python/2.7.14/Frameworks/Python.framework/Versions/2.7/lib/python2.7/runpy.py", line 174, in _run_module_as_main
 0:01.38     "__main__", fname, loader, pkg_name)
 0:01.38   File "/usr/local/Cellar/python/2.7.14/Frameworks/Python.framework/Versions/2.7/lib/python2.7/runpy.py", line 72, in _run_code
 0:01.38     exec code in run_globals
 0:01.38   File "/Users/Ed/Development/mozilla-central/python/mozbuild/mozbuild/action/process_install_manifest.py", line 111, in <module>
 0:01.38     main(sys.argv[1:])
 0:01.38   File "/Users/Ed/Development/mozilla-central/python/mozbuild/mozbuild/action/process_install_manifest.py", line 98, in main
 0:01.38     defines=args.defines)
 0:01.38   File "/Users/Ed/Development/mozilla-central/python/mozbuild/mozbuild/action/process_install_manifest.py", line 45, in process_manifest
 0:01.38     remove_unaccounted.add(p, dummy_file)
 0:01.38   File "/Users/Ed/Development/mozilla-central/python/mozbuild/mozpack/copier.py", line 69, in add
 0:01.38     return errors.error("%s already added" % path)
 0:01.39   File "/Users/Ed/Development/mozilla-central/python/mozbuild/mozpack/errors.py", line 106, in error
 0:01.39     self._handle(self.ERROR, msg)
 0:01.39   File "/Users/Ed/Development/mozilla-central/python/mozbuild/mozpack/errors.py", line 98, in _handle
 0:01.39     raise ErrorMessage(msg)
 0:01.39 mozpack.errors.ErrorMessage: Error: chrome/content/lib/UITour-lib.js already added
 0:01.40 make: *** [install-dist/bin/browser/features/onboarding@mozilla.org] Error 1

	Masatoshi Kimura [:emk] Assignee
	Comment 11 • 7 years ago

> 0:06.31 DEBUGRI link WORKING /home/grisha/Code/mozilla-central/testing/mozbase/manifestparser/**/mozbase

The "working" path is obviously bogus (no "mozbase" under "testing/mozbase/manifestparser/"). So it was ignored and it happened to work. According to bug 1417605 comment #5, the error implies a potential race condition.

	Masatoshi Kimura [:emk] Assignee
	Comment 12 • 7 years ago

If we need a quick hack, `mozpath.join(dest, pattern, base),` will work (changing a bogus path to another bogus but unique path). It will not at least wipe out files outside objdir.

	Masatoshi Kimura [:emk] Assignee
	Comment 13 • 7 years ago

> `mozpath.join(dest, pattern, base),`

Ah, this will not work if base is an absolute path.

	Masatoshi Kimura [:emk] Assignee
	Comment 14 • 7 years ago

FileFinder matches "chrome/content/prerendered/*" (not "chrome/content/prerendered/**") with
chrome/content/prerendered/ach/activity-stream-prerendered.html
chrome/content/prerendered/ach/activity-stream-strings.js
chrome/content/prerendered/ach/activity-stream.html
chrome/content/prerendered/ar/activity-stream-prerendered.html
chrome/content/prerendered/ar/activity-stream-strings.js
chrome/content/prerendered/ar/activity-stream.html
...
Is this expected?

	Ed Lee :Mardak
	Comment 15 • 7 years ago

Is which "part" is expected or not? We do want it to package those 3 ach/* files and 3 ar/* files under chrome/content/prerendered/ach/*, etc.

I'm guessing there's the "potential race" from comment 11 via:

browser/extensions/activity-stream/jar.mn:
  content/prerendered/static/activity-stream-initial-state.js (./prerendered/static/activity-stream-initial-state.js)
  content/prerendered/ (./prerendered/locales/*)

and

browser/extensions/onboarding/jar.mn:
  content/ (content/*)
  content/lib/UITour-lib.js (/browser/components/uitour/UITour-lib.js)
  content/modules/ (*.jsm)

Where for activity stream, there could be a "static" directory at ./prerendered/locales/* (but doesn't)

And for onboarding, content/lib could exist (but doesn't)

	Nick Alexander :nalexander [he/him] (PTO until Montreallhands August 21, 2023)
	Comment 16 • 7 years ago

This is completely busted for many people and needs to be backed out.  I'll find a sheriff.

	Nick Alexander :nalexander [he/him] (PTO until Montreallhands August 21, 2023)
	Comment 17 • 7 years ago

Aryx kindly backed this out (and pushed to central), so we should be back to building shortly.

emk: sorry to take action without involving you.  Can you pick this up and figure out what should really be done here?  Thanks!

	Pulsebot
	Comment 18 • 7 years ago

Backout by archaeopteryx@coole-files.de:
https://hg.mozilla.org/mozilla-central/rev/a3f183201f7f
Backed out changeset 6f0e650b0e53 on request from nalexander for many busted build environments. r=backout a=backout

	Michael Shal [:mshal]
	Comment 19 • 7 years ago

Given that there's a lot of weird cases that clearly aren't covered by a try push or autoland, I think we'll want some python tests to verify:
 1) This bug (ie: things don't get clobbered outside of the objdir due to incorrectly formed entries)
 2) Re-running a manifest after it's already installed (I think this is what is triggering the 'mach build faster' errors after this patch landed)

Another way to reproduce the 'mach build faster' error is to do:

./mach configure
make -C obj-x86_64-pc-linux-gnu/faster install-dist/bin/browser/features/onboarding@mozilla.org
make -C obj-x86_64-pc-linux-gnu/faster install-dist/bin/browser/features/onboarding@mozilla.org

The second time processing the install manifest fails, which isn't something that will happen in automation without a test case to check it.

	Masatoshi Kimura [:emk] Assignee
	Comment 22 • 7 years ago

Can anyone test this patch? I can not test it myself due to bug 1283444. `mach build faster` has never worked for me since VS2015 migration.

	Ed Lee :Mardak
	Comment 23 • 7 years ago

Applying both attachment 8927627 [details] and attachment 8929051 [details] has the build succeeding.

	Michael Shal [:mshal]
	Comment 24 • 7 years ago
mozreview-review

Comment on attachment 8929051 [details]
Bug 1416465 - Remove files that are actually linked or copied.

https://reviewboard.mozilla.org/r/200356/#review205572

::: commit-message-b421b:3
(Diff revision 1)
> +Bug 1416465 - Remove files that are actually linked or copied. r?Build
> +
> +If multiple source direcotries share the same destination, process_manifest

Can we add a python test to verify this behavior per #c19? And ideally a test for the original busted behavior of removing files outside of the objdir.

	Masatoshi Kimura [:emk] Assignee
	Comment 26 • 7 years ago

Oddly enough, I can no longer reproduce the original issue. I am adding a regression test for now.

	Masatoshi Kimura [:emk] Assignee
	Comment 27 • 7 years ago

Why the review request was canceled? This is a test-only patch. Do you want a next bustage just like bug 1414060? OK, good luck.

	(not currently active) Ted Mielczarek
	Comment 28 • 7 years ago

My apologies, that was simply user error.

	Michael Shal [:mshal]
	Comment 29 • 7 years ago
mozreview-review

Comment on attachment 8927627 [details]
Bug 1416465 - Expand pattern when track file is created rather than read.

https://reviewboard.mozilla.org/r/198936/#review207044

This looks great! Thanks for adding test coverage here. Sorry for the confusion earlier - we're still getting used to the new shared queue workflow.

::: python/mozbuild/mozbuild/test/action/test_process_install_manifest.py:60
(Diff revision 3)
> +            process_install_manifest.process_manifest(dest, [p], track)
> +
> +            self.assertTrue(os.path.exists(self.tmppath('dest/foo/file1')))
> +            self.assertTrue(os.path.exists(self.tmppath('dest/foo/file2')))
> +            self.assertTrue(os.path.exists(self.tmppath('dest/foo/file3')))
> +

So one thing I noticed that is broken currently but was fixed with your earlier patch is if you add these steps here:

+        m = InstallManifest()
+        m.write(path=p)
+
+        for i in range(2):
+            process_install_manifest.process_manifest(dest, [p], track)
+
+            self.assertFalse(os.path.exists(self.tmppath('dest/foo/file1')))
+            self.assertFalse(os.path.exists(self.tmppath('dest/foo/file2')))
+            self.assertFalse(os.path.exists(self.tmppath('dest/foo/file3')))

IOW: Remove the manifest entries and re-run the manifest. Due to the track file, the dest files should be removed, but the two pattern_link created entries are not removed. I think this may be a sufficient addition to test the original broken behavior you were seeing, but I'm not sure if it's the exact problem. Obviously we can't add that now though since it doesn't work, but it may help further diagnosing the issue.

	Masatoshi Kimura [:emk] Assignee
	Comment 31 • 7 years ago
mozreview-review-reply

Comment on attachment 8927627 [details]
Bug 1416465 - Expand pattern when track file is created rather than read.

https://reviewboard.mozilla.org/r/198936/#review207044

> So one thing I noticed that is broken currently but was fixed with your earlier patch is if you add these steps here:
> 
> +        m = InstallManifest()
> +        m.write(path=p)
> +
> +        for i in range(2):
> +            process_install_manifest.process_manifest(dest, [p], track)
> +
> +            self.assertFalse(os.path.exists(self.tmppath('dest/foo/file1')))
> +            self.assertFalse(os.path.exists(self.tmppath('dest/foo/file2')))
> +            self.assertFalse(os.path.exists(self.tmppath('dest/foo/file3')))
> 
> IOW: Remove the manifest entries and re-run the manifest. Due to the track file, the dest files should be removed, but the two pattern_link created entries are not removed. I think this may be a sufficient addition to test the original broken behavior you were seeing, but I'm not sure if it's the exact problem. Obviously we can't add that now though since it doesn't work, but it may help further diagnosing the issue.

I added it as an @expectedFailure test.

	Pulsebot
	Comment 32 • 7 years ago

Pushed by VYV03354@nifty.ne.jp:
https://hg.mozilla.org/integration/autoland/rev/366c4c091bb3
Regression tests for process_install_manifest.py. r=mshal

	Stefan Hindli [:stefan_hindli]
	Comment 33 • 7 years ago
bugherder

https://hg.mozilla.org/mozilla-central/rev/366c4c091bb3

	Masatoshi Kimura [:emk] Assignee
	Comment 34 • 7 years ago

Today I hit this again.

	Masatoshi Kimura [:emk] Assignee
	Comment 36 • 7 years ago

The latest patch fixed bug 1283444 as well as this bug.

	Michael Shal [:mshal]
	Comment 37 • 7 years ago
mozreview-review

Comment on attachment 8927627 [details]
Bug 1416465 - Expand pattern when track file is created rather than read.

https://reviewboard.mozilla.org/r/198936/#review211514

::: python/mozbuild/mozbuild/action/process_install_manifest.py:72
(Diff revision 5)
>          remove_unaccounted=remove_unaccounted,
>          remove_all_directory_symlinks=remove_all_directory_symlinks,
>          remove_empty_directories=remove_empty_directories)
>  
>      if track:
> -        manifest.write(path=track)
> +        manifest.write(path=track, expand_pattern=True)

I think it might help to have a comment here explaining why we need the wildcard patterns expanded in the track file.

	Masatoshi Kimura [:emk] Assignee
	Comment 39 • 7 years ago
mozreview-review-reply

Comment on attachment 8927627 [details]
Bug 1416465 - Expand pattern when track file is created rather than read.

https://reviewboard.mozilla.org/r/198936/#review211514

> I think it might help to have a comment here explaining why we need the wildcard patterns expanded in the track file.

Added a comment.

	Pulsebot
	Comment 40 • 7 years ago

Pushed by VYV03354@nifty.ne.jp:
https://hg.mozilla.org/integration/autoland/rev/f6b4d0711868
Expand pattern when track file is created rather than read. r=mshal

	Bogdan Tara[:bogdan_tara | bogdant]
	Comment 41 • 7 years ago
bugherder

https://hg.mozilla.org/mozilla-central/rev/f6b4d0711868