New issue
Advanced search Search tips

Issue 799827 link

Starred by 1 user

Issue metadata

Status: Assigned
Owner:
Cc:
EstimatedDays: ----
NextAction: ----
OS: Linux , Windows
Pri: 3
Type: Bug

Blocking:
issue 495204



Sign in to add a comment

previous_version_mini_installer.exe doesn't build in cross builds

Project Member Reported by thakis@chromium.org, Jan 8 2018

Issue description

[12583/23180] ACTION //chrome/installer/mini_installer:next_version_mini_installer(//build/toolchain/win:win_clang_x86)
FAILED: next_version_mini_installer.exe 
python ../../chrome/installer/mini_installer/generate_next_version_mini_installer.py --out next_version_mini_installer.exe
Traceback (most recent call last):
  File "../../chrome/installer/mini_installer/generate_next_version_mini_installer.py", line 26, in <module>
    sys.exit(main())
  File "../../chrome/installer/mini_installer/generate_next_version_mini_installer.py", line 21, in main
    '--out=' + args.out,
  File "/usr/lib/python2.7/subprocess.py", line 522, in call
    return Popen(*popenargs, **kwargs).wait()
  File "/usr/lib/python2.7/subprocess.py", line 710, in __init__
    errread, errwrite)
  File "/usr/lib/python2.7/subprocess.py", line 1327, in _execute_child
    raise child_exception
OSError: [Errno 2] No such file or directory


That one is easy to fix, alternate_version_generator.exe (not yet sure why that's not a python script) also shells out to 7za.exe, makecab.exe etc. Probably needs similar changes as in  bug 762073 .
 

Comment 1 by thakis@chromium.org, Jan 14 2018

Cc: grt@chromium.org
Labels: OS-Linux
alternate_version_generator as far as I can tell takes mini_installer.exe, tweaks the VERSIONINFO resource, tweaks it a bit, and writes a copy with the tweaked resource. (In a recursive and slightly involved way, nicely documented at https://cs.chromium.org/chromium/src/chrome/installer/test/alternate_version_generator.cc?type=cs&sq=package:chromium&l=5)

It's used at build-time (read: host binary) to generate a newer-versioned file which then some python tests seem to use (https://cs.chromium.org/chromium/src/chrome/test/mini_installer/test_installer.py?q=NEXT_VERSION_MINI_INSTALLER&sq=package:chromium&l=415&dr=C , https://cs.chromium.org/chromium/src/chrome/test/mini_installer/config/config.config?type=cs&q=NEXT_VERSION_MINI_INSTALLER&sq=package:chromium&l=237 etc).

It's also used at test time (read: target binary) by one test here: https://cs.chromium.org/chromium/src/chrome/installer/util/delete_old_versions_unittest.cc?type=cs&q=delete.*unittest+file:installer&sq=package:chromium&l=46

alternate_version_generator pretty heavily depends on Windows APIs for resource inspection. It also duplicates some of the create_installer_archive.py logic in c++ (7za invocation, makecab invocation, etc).

Options for making this go in the cross bulid (where the host system isn't windows):

1. Make alternate_version_installer not use any windows apis; implement PE reading and resource reading. Not that hard; slightly tedious.

2. Rewrite alternate_version_generator in python. The pefile.py module has nice support for reading resources and we might be able to harness create_installer_archive.py for zipping up the new archive. This might be a lot less code and will be cross-platform. delete_old_versions_unittest.cc would have to shell out to python (also slightly tedious), or it would have to have a build-time dep on a gn action that provides the old binary for testing (that way alternate_version_generator would be host-only)

3. Execute alternate_version_generator not at build-time but at test-time for the python test. That way it doesn't have to exist as a host binary.


grt, does the summary sound roughly right? Do you prefer one of these approaches? (Or do you know if someone else might have an opinion?)

Comment 2 by grt@chromium.org, Jan 16 2018

How about option 4: don't add //chrome/installer/mini_installer:next_version_mini_installer or //chrome/installer/test:alternate_version_generator as dependencies on any targets (e.g., gn_all) when building on non-win hosts? Hmm, I guess the goal to build the world on non-win and then run tests on win, huh?

The other options all seem fine to me as long as there's still a way to run the tool manually. I don't have a strong opinion one way or the other.
Mostly note to self: I wonder if it's possible to make the next_version_mini_installer step much faster by not unzipping all of chrome.7z but only the version bits, and then overwriting them with the modified parts (on a copied chrome.7z -- copying a file and changing some small parts should be much faster than recompressing _everythying_. Depends on the 7z bitstream containing sync points for its lz77 thing of course; not sure if it does)
Cc: tikuta@chromium.org
Project Member

Comment 5 by bugdroid1@chromium.org, Feb 23 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/300365f1fee63ad4d9a84186c19a9ea4b9928df3

commit 300365f1fee63ad4d9a84186c19a9ea4b9928df3
Author: Nico Weber <thakis@chromium.org>
Date: Fri Feb 23 20:45:04 2018

Omit next_version_mini_installer target from win/cross builds for now.

I need some more time to figure out what to do here, but there's a bot
now and it also needs to be able to build.  So remove the target from
the build in cross builds for now.

TBR=grt

Bug: 799827
Change-Id: I3a07ae1298025651e873342b3e1220e546d3e24c
Reviewed-on: https://chromium-review.googlesource.com/935262
Reviewed-by: Nico Weber <thakis@chromium.org>
Commit-Queue: Nico Weber <thakis@chromium.org>
Cr-Commit-Position: refs/heads/master@{#538887}
[modify] https://crrev.com/300365f1fee63ad4d9a84186c19a9ea4b9928df3/BUILD.gn
[modify] https://crrev.com/300365f1fee63ad4d9a84186c19a9ea4b9928df3/chrome/installer/mini_installer/BUILD.gn

Project Member

Comment 6 by bugdroid1@chromium.org, Mar 29 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/2a1e0f88dbefa67ce4832b8b59c788e4b48bfaae

commit 2a1e0f88dbefa67ce4832b8b59c788e4b48bfaae
Author: Nico Weber <thakis@chromium.org>
Date: Thu Mar 29 23:44:21 2018

Run more tests on the clang tot bots.

In particular, if it runs on the main waterfall, it should also run
on the tot bots.

Adds app_shell_unittests, aura_unittests, chrome_elf_import_unittests,
compositor_unittests, install_static_unittests, mini_installer_tests,
wm_unittests.

Don't run mini_installer_tests on the cross bot due to
next_version_mini_installer.exe not yet working there.

Bug:  827075 , 827082 ,799827
Change-Id: Iffe1cf0be5a95892988aab93feefecf2c5b48cbd
Reviewed-on: https://chromium-review.googlesource.com/986679
Reviewed-by: Reid Kleckner <rnk@chromium.org>
Commit-Queue: Nico Weber <thakis@chromium.org>
Cr-Commit-Position: refs/heads/master@{#547025}
[modify] https://crrev.com/2a1e0f88dbefa67ce4832b8b59c788e4b48bfaae/testing/buildbot/chromium.clang.json
[modify] https://crrev.com/2a1e0f88dbefa67ce4832b8b59c788e4b48bfaae/testing/buildbot/chromium.fyi.json
[modify] https://crrev.com/2a1e0f88dbefa67ce4832b8b59c788e4b48bfaae/testing/buildbot/test_suite_exceptions.pyl
[modify] https://crrev.com/2a1e0f88dbefa67ce4832b8b59c788e4b48bfaae/testing/buildbot/test_suites.pyl

Project Member

Comment 7 by bugdroid1@chromium.org, Apr 6 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/tools/build/+/cd0d098cfc259aa30458804ec621a6894b8fe7c6

commit cd0d098cfc259aa30458804ec621a6894b8fe7c6
Author: Nico Weber <thakis@chromium.org>
Date: Fri Apr 06 13:03:29 2018

Don't run test_installer.py on 32-bit debug bots or on the cross bot.

next_version_mini_installer isn't hooked up in the build in 32-bit debug
and in cross builds, so the test can't pass there.

Reverts parts of https://chromium-review.googlesource.com/c/chromium/tools/build/+/987913

Bug:  827082 , 799827
Change-Id: Ie57e96a7db58e4715434d9988609856c39d9ec8e
Reviewed-on: https://chromium-review.googlesource.com/999319
Reviewed-by: Hans Wennborg <hans@chromium.org>
Reviewed-by: Sergiy Byelozyorov <sergiyb@chromium.org>
Commit-Queue: Nico Weber <thakis@chromium.org>

[modify] https://crrev.com/cd0d098cfc259aa30458804ec621a6894b8fe7c6/scripts/slave/recipe_modules/chromium_tests/chromium_clang.py

Summary: previous_version_mini_installer.exe doesn't build in cross builds (was: next_version_mini_installer.exe doesn't build in cross builds)
(retitling; target got renamed)

Sign in to add a comment