AU: Minor fixes to get it to do full update on real device
- Link w/ gtest only for unittests.
- filesystem_copier_action: always send install plan on the output pipe
on success, even if no copy was performed.
- omaha_response_handler: properly choose install partition based on
new GPT partition spec.
- More useful logging to match memento_updater (which update URL is
used, dump of the request/response for the update check).
- Fixed a bug where I wasn't bonding the proper actions together in update_attempter.
BUG=None
TEST=attached unittests/did full update on Eee PC
Review URL: http://codereview.chromium.org/2044001
diff --git a/SConstruct b/SConstruct
index dc05e37..5adf16b 100644
--- a/SConstruct
+++ b/SConstruct
@@ -94,21 +94,21 @@
env['ENV'][key] = os.environ[key]
- # -Wclobbered
- # -Wempty-body
- # -Wignored-qualifiers
- # -Wtype-limits
env['CCFLAGS'] = ' '.join("""-g
-fno-exceptions
-fno-strict-aliasing
-Wall
-Wclobbered
+ -Wclobbered
+ -Wempty-body
-Wempty-body
-Werror
-Wignored-qualifiers
+ -Wignored-qualifiers
-Wmissing-field-initializers
-Wsign-compare
-Wtype-limits
+ -Wtype-limits
-Wuninitialized
-D__STDC_FORMAT_MACROS=1
-D_FILE_OFFSET_BITS=64
@@ -120,7 +120,6 @@
curl
gflags
glib-2.0
- gtest
gthread-2.0
libpcrecpp
protobuf
@@ -216,16 +215,22 @@
delta_generator_main = ['generate_delta_main.cc']
-env.Program('update_engine', sources + main)
-unittest_cmd = env.Program('update_engine_unittests',
- sources + unittest_sources + unittest_main)
-client_cmd = env.Program('update_engine_client', sources + client_main);
+update_engine_core = env.Library('update_engine_core', sources)
+env.Prepend(LIBS=[update_engine_core])
-Clean(unittest_cmd, Glob('*.gcda') + Glob('*.gcno') + Glob('*.gcov') +
- Split('html app.info'))
+env.Program('update_engine', main)
+
+client_cmd = env.Program('update_engine_client', client_main);
delta_generator_cmd = env.Program('delta_generator',
- sources + delta_generator_main)
+ delta_generator_main)
http_server_cmd = env.Program('test_http_server', 'test_http_server.cc')
+
+unittest_env = env.Clone()
+unittest_env.Append(LIBS=['gtest'])
+unittest_cmd = unittest_env.Program('update_engine_unittests',
+ unittest_sources + unittest_main)
+Clean(unittest_cmd, Glob('*.gcda') + Glob('*.gcno') + Glob('*.gcov') +
+ Split('html app.info'))
diff --git a/action.h b/action.h
index b45100a..9e6eb94 100644
--- a/action.h
+++ b/action.h
@@ -187,7 +187,7 @@
return out_pipe_->contents();
}
-protected:
+ protected:
// We use a shared_ptr to the pipe. shared_ptr objects destroy what they
// point to when the last such shared_ptr object dies. We consider the
// Actions on either end of a pipe to "own" the pipe. When the last Action
diff --git a/filesystem_copier_action.cc b/filesystem_copier_action.cc
index bd44867..6d7cd22 100755
--- a/filesystem_copier_action.cc
+++ b/filesystem_copier_action.cc
@@ -43,6 +43,8 @@
if (install_plan_.is_full_update) {
// No copy needed. Done!
+ if (HasOutputPipe())
+ SetOutputObject(install_plan_);
abort_action_completer.set_success(true);
return;
}
diff --git a/filesystem_copier_action_unittest.cc b/filesystem_copier_action_unittest.cc
index 6b43817..b5cd64d 100644
--- a/filesystem_copier_action_unittest.cc
+++ b/filesystem_copier_action_unittest.cc
@@ -230,7 +230,8 @@
processor.set_delegate(&delegate);
ObjectFeederAction<InstallPlan> feeder_action;
- InstallPlan install_plan(true, "", "", "", "");
+ const char* kUrl = "http://some/url";
+ InstallPlan install_plan(true, kUrl, "", "", "");
feeder_action.set_obj(install_plan);
FilesystemCopierAction copier_action(false);
ObjectCollectorAction<InstallPlan> collector_action;
@@ -245,6 +246,7 @@
EXPECT_FALSE(processor.IsRunning());
EXPECT_TRUE(delegate.ran_);
EXPECT_TRUE(delegate.success_);
+ EXPECT_EQ(kUrl, collector_action.object().download_url);
}
TEST_F(FilesystemCopierActionTest, NonExistentDriveTest) {
diff --git a/omaha_request_prep_action.cc b/omaha_request_prep_action.cc
index 6ee2c24..2b019b3 100644
--- a/omaha_request_prep_action.cc
+++ b/omaha_request_prep_action.cc
@@ -43,7 +43,7 @@
version, // app version (from lsb-release)
"en-US", // lang
track, // track
- UpdateCheckParams::kUpdateUrl);
+ update_url);
CHECK(HasOutputPipe());
SetOutputObject(out);
diff --git a/omaha_response_handler_action.cc b/omaha_response_handler_action.cc
index 4bdef7d..f90255a 100644
--- a/omaha_response_handler_action.cc
+++ b/omaha_response_handler_action.cc
@@ -32,6 +32,8 @@
TEST_AND_RETURN(GetInstallDev(
(!boot_device_.empty() ? boot_device_ : utils::BootDevice()),
&install_plan.install_path));
+ install_plan.kernel_install_path =
+ utils::BootKernelDevice(install_plan.install_path);
// Get the filename part of the url. Assume that if it has kFullUpdateTag
// in the name, it's a full update.
@@ -47,27 +49,23 @@
// Very long name. Let's shorten it
filename.resize(255);
}
+ TEST_AND_RETURN(HasOutputPipe());
if (HasOutputPipe())
SetOutputObject(install_plan);
LOG(INFO) << "Using this install plan:";
install_plan.Dump();
+
completer.set_success(true);
}
bool OmahaResponseHandlerAction::GetInstallDev(const std::string& boot_dev,
std::string* install_dev) {
- TEST_AND_RETURN_FALSE(!boot_dev.empty());
+ TEST_AND_RETURN_FALSE(utils::StringHasPrefix(boot_dev, "/dev/"));
string ret(boot_dev);
- char last_char = *ret.rbegin();
- TEST_AND_RETURN_FALSE((last_char >= '0') && (last_char <= '9'));
- // Chop off last char
- ret.resize(ret.size() - 1);
- // If last_char is odd (1 or 3), increase it, otherwise decrease
- if (last_char % 2)
- last_char++;
- else
- last_char--;
- ret += last_char;
+ string::reverse_iterator it = ret.rbegin(); // last character in string
+ // Right now, we just switch '3' and '5' partition numbers.
+ TEST_AND_RETURN_FALSE((*it == '3') || (*it == '5'));
+ *it = (*it == '3') ? '5' : '3';
*install_dev = ret;
return true;
}
diff --git a/omaha_response_handler_action_unittest.cc b/omaha_response_handler_action_unittest.cc
index 4464332..e997ea9 100644
--- a/omaha_response_handler_action_unittest.cc
+++ b/omaha_response_handler_action_unittest.cc
@@ -89,11 +89,11 @@
in.needs_admin = true;
in.prompt = false;
InstallPlan install_plan;
- EXPECT_TRUE(DoTest(in, "/dev/sda1", &install_plan));
+ EXPECT_TRUE(DoTest(in, "/dev/sda3", &install_plan));
EXPECT_TRUE(install_plan.is_full_update);
EXPECT_EQ(in.codebase, install_plan.download_url);
EXPECT_EQ(in.hash, install_plan.download_hash);
- EXPECT_EQ("/dev/sda2", install_plan.install_path);
+ EXPECT_EQ("/dev/sda5", install_plan.install_path);
}
{
UpdateCheckResponse in;
@@ -106,7 +106,7 @@
in.needs_admin = true;
in.prompt = true;
InstallPlan install_plan;
- EXPECT_TRUE(DoTest(in, "/dev/sda4", &install_plan));
+ EXPECT_TRUE(DoTest(in, "/dev/sda5", &install_plan));
EXPECT_FALSE(install_plan.is_full_update);
EXPECT_EQ(in.codebase, install_plan.download_url);
EXPECT_EQ(in.hash, install_plan.download_hash);
@@ -123,11 +123,11 @@
in.needs_admin = true;
in.prompt = true;
InstallPlan install_plan;
- EXPECT_TRUE(DoTest(in, "/dev/sda4", &install_plan));
+ EXPECT_TRUE(DoTest(in, "/dev/sda3", &install_plan));
EXPECT_FALSE(install_plan.is_full_update);
EXPECT_EQ(in.codebase, install_plan.download_url);
EXPECT_EQ(in.hash, install_plan.download_hash);
- EXPECT_EQ("/dev/sda3", install_plan.install_path);
+ EXPECT_EQ("/dev/sda5", install_plan.install_path);
}
}
diff --git a/update_attempter.cc b/update_attempter.cc
index e12fed9..ad3bd9e 100644
--- a/update_attempter.cc
+++ b/update_attempter.cc
@@ -70,14 +70,18 @@
// Bond them together. We have to use the leaf-types when calling
// BondActions().
- BondActions(request_prep_action.get(), update_check_action.get());
- BondActions(update_check_action.get(), response_handler_action.get());
- BondActions(response_handler_action.get(), filesystem_copier_action.get());
+ BondActions(request_prep_action.get(),
+ update_check_action.get());
+ BondActions(update_check_action.get(),
+ response_handler_action.get());
BondActions(response_handler_action.get(),
+ filesystem_copier_action.get());
+ BondActions(filesystem_copier_action.get(),
kernel_filesystem_copier_action.get());
BondActions(kernel_filesystem_copier_action.get(),
download_action.get());
- BondActions(download_action.get(), postinstall_runner_action_precommit.get());
+ BondActions(download_action.get(),
+ postinstall_runner_action_precommit.get());
BondActions(postinstall_runner_action_precommit.get(),
set_bootable_flag_action.get());
BondActions(set_bootable_flag_action.get(),
@@ -96,15 +100,7 @@
return;
}
if (!success) {
- if (!full_update_) {
- LOG(ERROR) << "Update failed. Attempting full update";
- actions_.clear();
- response_handler_action_.reset();
- Update(true);
- return;
- } else {
- LOG(ERROR) << "Full update failed. Aborting";
- }
+ LOG(INFO) << "Update failed.";
}
g_main_loop_quit(loop_);
}
diff --git a/update_check_action.cc b/update_check_action.cc
index 106400b..57e82eb 100644
--- a/update_check_action.cc
+++ b/update_check_action.cc
@@ -107,6 +107,8 @@
http_fetcher_->set_delegate(this);
string request_post(FormatRequest(params_));
http_fetcher_->SetPostData(request_post.data(), request_post.size());
+ LOG(INFO) << "Checking for update at " << params_.update_url;
+ LOG(INFO) << "Request: " << request_post;
http_fetcher_->BeginTransfer(params_.update_url);
}
@@ -190,8 +192,12 @@
void UpdateCheckAction::TransferComplete(HttpFetcher *fetcher,
bool successful) {
ScopedActionCompleter completer(processor_, this);
- if (!successful)
+ LOG(INFO) << "Update check response: " << string(response_buffer_.begin(),
+ response_buffer_.end());
+ if (!successful) {
+ LOG(ERROR) << "Update check network transfer failed.";
return;
+ }
if (!HasOutputPipe()) {
// Just set success to whether or not the http transfer succeeded,
// which must be true at this point in the code.