Re: [PATCH 2/2] gold: add a task in incremental build that can be used to check which inputs were modified

[Ian Lance Taylor <iant at google dot com>]

Mikolaj Zalewski <mikolajz@google.com> writes:

>   This patch adds a task that can be later used in an incremental
> build to check if a full build is needed and what files needs to be
> included. In an incremental build, it will be executed after all
> Read_symbols and before Add_symbols, Add_archive_symbols and
> Read_script. In non-incremental build, the order shouldn't change.

I don't think your patch quite does this.  You have a blocker token
which ensures that your Incremental_inputs_checker task will run after
all of the Read_symbols tasks.  But I don't see anything which forces
the Add_symbols task to wait until the Incremental_inputs_checker task
is complete.


>   There are two issues. Doing it after Read_symbols is suboptimal, as
> it already loaded symbols from object files, including the ones that
> were not modified. Later, I can divide this task into two and plug the
> input checker between detecting the type of file and loading symbols.

When doing an incremental link, we don't want to even open a file which
is unchanged via --incremental-unchanged.  And actually one of the first
things we need to do it to open the output file, if it exists, so that
we can stat input files and see whether we need to open them.  This
isn't going to all work right away, but even at the start I think we
need a different approach than your patch.  I don't think we have to
split Read_symbols.  I think we need something which runs before
Read_symbols which checks whether Read_symbols is needed at all.  Since
all that job has to do with any given file is to run stat, I don't think
we need a separate Task for each input file just to find out whether we
need to read it.  A single Task will suffice for that.  I'm envisioning
a single Task which opens the output file, decides what to for each
input file, and queues up Read_symbols tasks as needed.


> A bigger problem is that we do it before Read_scripts thus we won't
> see input files included by scripts. This can be fixed by dividing the
> a Read_script task into one that reads script and another the applies
> it to the layout. Before doing this, as a hack, I could treat every
> file included by a script as modified.

This is, unfortunately, important to solve in the long run since libc.so
on GNU/Linux systems is a linker script, and we definitely don't want to
assume that libc.so.6 is modified each time we do an incremental link.
Here it may suffice to call stat in the function which Read_scripts
queues up to read the input file symbols.


>   I don't know how plug-ins interacts with this - when do plugins add
> input objects? Currently I print an error if they do it too late.

I *think* that when a plugin adds an input file we have to assume it is
modified.  In any case this is certainly lower priority.


>   BTW, `make check` in gold fails (in ver_test_4 assertion failed in
> parameters.h:84 - gold_assert(this->options_valid())), but it also
> fails without my patches - it's probably caused by an earlier patch. I
> could try to look into it tomorrow.

This should be fixed now.


I'm not going to apply this patch, I think the task structure needs some
more work.

Thanks.

Ian