In short, the concept of any computer system is usually straight forward. The complexity comes from the exceptions. Take commits for example. Yes, each commit affects a file. Or so you’d think. Consider this commit.
FreshPorts parses the email. In that email, there are no files mentioned. This caused a problem during the loading. FreshPorts, as a sanity test, complained there were no files. This complaint is now just a notification, not a fatal error.
The fix for that was rather simple:
- FreshPorts::Utilities::ReportError('Err', "No files found in commit '$Updates{MessageId}'. Has someone done a cvs import instead of addport?", 1)
+ FreshPorts::Utilities::ReportError('Err', "No files found in commit '$Updates{MessageId}'. Has someone done a cvs import instead of addport?", 0)
In this case, the 1 means die, fatal error. Hmmm, wouldn’t that be better with a constant?
The next issue was the display of the commit. Production was displaying the commit. Dev and beta was not. Eventually I figured out the problem was a long-standing stored procedure.
The fix for that issue was much more subtle:
@@ -2185,13 +2185,14 @@ SELECT 0 AS port_id,
NULL::text AS only_for_archs,
NULL::text AS not_for_archs,
OnWatchList($4, CLE.element_id) AS watch
- FROM commit_log CL, commit_log_elements CLE, element E
- WHERE CL.id = CLE.commit_log_id
- AND CL.message_id = $1
- AND CLE.element_id = E.id
+ FROM commit_log CL LEFT OUTER JOIN commit_log_elements CLE ON (CL.id = CLE.commit_log_id)
+ LEFT OUTER JOIN element E ON (CLE.element_id = E.id)
+ WHERE CL.message_id = $1
ORDER BY port, element_pathname
LIMIT $2
OFFSET $3;
+
+ -- NOTE some commits touch nothing.... e.g. 201109230051.p8N0pbV2045995@svn.freebsd.org
$$ LANGUAGE SQL STABLE;
This is the code for retrieving a commit. It also retrieves the files touched by that commit. Thus, if there are no files, it will not retrieve anything. Thereby, incorrectly reporting ‘Sorry, nothing found in the database….’.
The old code assumed that for every entry (commit) in the commit_log table, there would be at least one element (file). The SQL change made entries in the commit_log_elements and the element tables would be optional.
This commit now displays correctly in all three environments:
But why was production displaying the commit? Because it still contains older code which has since been fixed. This fix stopped FreshPorts from incorrectly treating ‘Directory Properties:’ and ‘(props changed)’ as ‘files”. The fix was on beta and dev. Hence, those systems had no entries in commit_log_elements for those bogus files. This is why they would not display the commit.
But now I have bogus files sitting around. I deleted them by hand.
Then I noticed that commit_log_elements was missing foreign keys. I added them, but first had to delete rows which referred to invalid references. These two commands took care of that:
delete from commit_log_ports_elements where not exists (select * from commit_log where commit_log_id = id);
delete from commit_log_ports_elements where not exists (select * from element where element_id = id);
Then I added two new foreign keys:
alter table commit_log_ports_elements
add foreign key (element_id)
references element(id) on update cascade on delete cascade;
alter table commit_log_ports_elements
add foreign key (commit_log_id)
references commit_log(id) on update cascade on delete cascade;
The table in question now looks like this:
freshports.org=# \d commit_log_elements
Table "public.commit_log_elements"
Column | Type | Modifiers
---------------+--------------+------------------------------------------------------------------
id | integer | not null default nextval('commit_log_elements_id_seq'::regclass)
commit_log_id | integer | not null
element_id | integer | not null
revision_name | text |
change_type | character(1) | not null
Indexes:
"commit_log_elements_pkey" PRIMARY KEY, btree (id)
"commit_log_elements_clid" btree (commit_log_id)
"commit_log_elements_ei" btree (element_id)
Check constraints:
"commit_log_elements_change_type" CHECK (change_type = 'A'::bpchar OR change_type = 'M'::bpchar OR change_type = 'R'::bpchar)
Foreign-key constraints:
"$1" FOREIGN KEY (commit_log_id) REFERENCES commit_log(id) ON UPDATE CASCADE ON DELETE CASCADE
"$2" FOREIGN KEY (element_id, revision_name) REFERENCES element_revision(element_id, revision_name) ON UPDATE CASCADE ON DELETE CASCADE
Referenced by:
TABLE "commit_log_port_elements" CONSTRAINT "$2" FOREIGN KEY (commit_log_element_id) REFERENCES commit_log_elements(id) ON UPDATE CASCADE ON DELETE CASCADE
Triggers:
element_delete_check BEFORE INSERT OR UPDATE ON commit_log_elements FOR EACH ROW EXECUTE PROCEDURE element_delete_check()
Now I should be able to get into other things. :)
