#668 PR merged: Check for FDRUPSTREAM changed files and required programs

rowens275 opened issue at 2015-10-12 17:25:

Some improvements, but no major change in function. I've created CHECK_CONFIG_FILES_FDRUPSTREAM to track changes in certain files in $FDRUPSTREAM_INSTALL_PATH which should trigger a new ISO build. I've also created REQUIRED_PROGS_FDRUPSTREAM which only checks for one critical file, but that is enough to verify that the user has $FDRUPSTREAM_INSTALL_PATH set correctly.

schlomo commented at 2015-10-15 13:52:

I would always advice to not quote the patterns so that they are expanded there where you write them and not somewhere else. That gives you much more control and reduces risk for other code using those arrays. Maybe the other code does not handle such patterns well, you can never know for sure.

In any case, if you want to enable users to change the install path then all that has to go to the prep stage. Please nevertheless leave the variables in the default.conf (e.g. set to empty arrays) because we use the default.conf as documentation for user changeable variables. In the prep stage you should then extend the arrays thereby giving users the ability to also add stuff to the arrays.

schlomo commented at 2015-10-15 13:52:

Do you want us to merge this now? In any case we cannot test it as we don't have your software.

rowens275 commented at 2015-10-15 14:59:

Yes, go ahead and merge. It is working as it is, but is probably not ideal. I will make some changes and submit another pull request, but things are very busy for me here right now so that may take several days.

Is there an impending release? I just want to know if I have a hard deadline...

rowens275 commented at 2015-10-29 17:25:

I moved the variables to the prep stage as you suggested, but that broke checklayout for me. The reason is that I am adding files to CHECK_CONFIG_FILES via CHECK_CONFIG_FILES_FDRUPSTREAM in layout/save/default/40_check_backup_special_files.sh, but 51_compare_files.sh doesn't know anything about that. So my additional files get included in files.md5sum, but 51_compare_files.sh only generates md5sums for the files defined in default.conf.

I am struggling to find the proper way to address this. I created layout/compare/FDRUPSTREAM/default/40_stuff.sh, but as far as it knows CHECK_CONFIG_FILES_FDRUPSTREAM is empty. I don't think it's a good idea for me to hard-code those values in two places -- I already extend that array (from its default empty value) in the prep stage like this:

CHECK_CONFIG_FILES_FDRUPSTREAM=( "${CHECK_CONFIG_FILES_FDRUPSTREAM[@]}" "$FDRUPSTREAM_INSTALL_PATH"/uscmd* "$FDRUPSTREAM_INSTALL_PATH"/usdaemon* "$FDRUPSTREAM_INSTALL_PATH"/us "$FDRUPSTREAM_INSTALL_PATH"/us1 "$FDRUPSTREAM_INSTALL_PATH"/us.ser "$FDRUPSTREAM_INSTALL_PATH"/.so "$FDRUPSTREAM_INSTALL_PATH"/usudb "$FDRUPSTREAM_INSTALL_PATH"/.dat "$FDRUPSTREAM_INSTALL_PATH"/.cfg )

I could move this line to its own script in the prep stage, and call that script in the layout/compare stage, but that feels kind of wrong. Is there somewhere more "central" I could define this array?

schlomo commented at 2015-10-29 17:36:

Do I understand you correctly that you want to use rear savelayout which should also include FDRUPSTREAM stuff?

I just realized that the savelayout workflow does not call the prepstage. Maybe this is the problem. To check this can you please try to edit usr/share/rear/lib/savelayout-workflow.sh so that it looks like this:

if [[ "$VERBOSE" ]]; then
    WORKFLOW_savelayout_DESCRIPTION="save the disk layout of the system"
fi
WORKFLOWS=( ${WORKFLOWS[@]} savelayout )
WORKFLOW_savelayout () {
    SourceStage "prep"
    DISKLAYOUT_FILE=$VAR_DIR/layout/disklayout.conf
    SourceStage "layout/save"
}

Please let me know if this solves your problem. Actually this is also what happens in the mkrescue workflow :-)

@gdha what do you think: Maybe we should always prep before savelayout so that it becomes extensible?

rowens275 commented at 2015-10-29 17:44:

I haven't tried your suggestion yet, but wanted to quickly comment. mkrescue properly updates files.md5sum, but it's probably because of layout/save/default/40_check_backup_special_files.sh, which I now realize must have been a workaround created by the TSM folks.

I haven't yet tried running savelayout by itself, but checklayout is where I noticed the problem of my array not being extended.

gdha commented at 2015-10-29 19:06:

The checklayout workflow:

WORKFLOW_checklayout () {
    ORIG_LAYOUT=$VAR_DIR/layout/disklayout.conf
    TEMP_LAYOUT=$TMP_DIR/checklayout.conf

    SourceStage "layout/precompare"

    DISKLAYOUT_FILE=$TEMP_LAYOUT
    SourceStage "layout/save"

    SourceStage "layout/compare"
}

also doesn't include the prep workflow. It should be tested I guess - not sure it benefits the situation?

rowens275 commented at 2015-10-29 19:11:

I made the change Schlomo suggested, and also added this line to lib/checklayout-workflow.sh, before all other SourceStage lines (not sure if the order matters):

SourceStage "prep"

This makes checklayout work for me, and includes CHECK_CONFIG_FILES_FDRUPSTREAM. I have not tested for any negative consequences yet.

schlomo commented at 2015-10-29 19:14:

Nice. Yes, the order matters. What you see there is the "recipe" which ReaR works through when you call the workflow.

rowens275 commented at 2015-10-29 19:49:

I can test this with BACKUP=FDRUPSTREAM. But I am concerned that I don't know enough about the inner workings of ReaR to feel confident that there are no adverse effects for other backup methods. If you guys have any suggestions of things to look out for, let me know.

[Export of Github issue for rear/rear.]