Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Warn if initrd cannot be created when udev not there #1176

Closed
wants to merge 11 commits into from
Closed

Warn if initrd cannot be created when udev not there #1176

wants to merge 11 commits into from

Conversation

jsmeix
Copy link
Member

@jsmeix jsmeix commented Jan 23, 2017

Warn if initrd cannot be created when udev not there
and some more code cleanup, see
#1171 (comment)

@jsmeix jsmeix added cleanup enhancement Adaptions and new features labels Jan 23, 2017
@jsmeix jsmeix self-assigned this Jan 23, 2017
@jsmeix jsmeix requested a review from gdha January 23, 2017 13:33
@jsmeix
Copy link
Member Author

jsmeix commented Jan 23, 2017

@gdha
currently this one is not at all tested by me.
I will test it tomorrow.
I only did that pull request soon so that
you could have a first look if you like.

gdha
gdha reviewed Jan 23, 2017
View reviewed changes
Copy link
Member

@gdha gdha left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You may test it out - looks ok, but as you mentioned not sure it is 100% yet.

usr/share/rear/finalize/Debian/i386/170_rebuild_initramfs.sh
fi
# During "rear mkbackup/mkrescue" 260_storage_drivers.sh creates $VAR_DIR/recovery/storage_drivers
if cmp -s $TMP_DIR/storage_drivers $VAR_DIR/recovery/storage_drivers ; then
Log "Skip recreating initramfs: '$TMP_DIR/storage_drivers' and '$VAR_DIR/recovery/storage_drivers' are same"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

perhaps are same should be are the same?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I even asked a native English speaking colleague
and you are right:
It is like "foo and bar are the same"
versus "foo and bar are same as baz".

@gdha
Copy link
Member

gdha commented Jan 23, 2017 •
edited

@jsmeix Wait for the merging pull request 'till you have tested it yourself first.

@jsmeix
Copy link
Member Author

jsmeix commented Jan 24, 2017

FYI regarding my code cleanup:
I documented the basic idea behind in the new section
"Return early, return often" in
https://github.com/rear/rear/wiki/Coding-Style

@jsmeix jsmeix added this to the Rear v2.1 milestone Jan 24, 2017
@jsmeix jsmeix mentioned this pull request Jan 24, 2017
Closed
@jsmeix
Copy link
Member Author

jsmeix commented Jan 24, 2017

Does not yet work because
'cmp' is missing in the recovery system:

+ source /usr/share/rear/finalize/SUSE_LINUX/i386/170_rebuild_initramfs.sh
++ test -s /tmp/rear.ZrH4yz5LjAGGsMC/tmp/storage_drivers
++ cmp -s /tmp/rear.ZrH4yz5LjAGGsMC/tmp/storage_drivers /var/lib/rear/recovery/storage_drivers
/usr/share/rear/finalize/SUSE_LINUX/i386/170_rebuild_initramfs.sh: line 11: cmp: command not found

@jsmeix jsmeix mentioned this pull request Jan 25, 2017
Merged
@jsmeix
Copy link
Member Author

jsmeix commented Jan 25, 2017

Damn!
Now I have messed up this pull request.

@jsmeix jsmeix closed this Jan 25, 2017
@jsmeix jsmeix mentioned this pull request Jan 25, 2017
Merged
@jsmeix jsmeix deleted the warn_if_initrd_cannot_be_created_when_udev_not_there_related_to_issue1171 branch January 25, 2017 15:10
@jsmeix
Copy link
Member Author

jsmeix commented Jan 25, 2017

I did a new (and hopefully clean) pull request:
#1179

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gdha gdha gdha left review comments

Assignees

@jsmeix jsmeix

Labels
cleanup enhancement Adaptions and new features fixed / solved / done
Projects
None yet
Milestone
ReaR v2.1
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@jsmeix @gdha @gozora @schabrolles