Skip to content
This repository has been archived by the owner on Feb 26, 2020. It is now read-only.

copy-builtin cannot be run more than once against the same target tree #526

Closed
infowolfe opened this issue Jan 25, 2016 · 8 comments
Closed
Milestone

Comments

@infowolfe
Copy link
Contributor

line 119 of copy-builtin:
sed -i 's#+= kernel/#+= kernel/ spl/#' "$KERNEL_DIR/Makefile"
results in line 887 of /usr/src/linux/Makefile:
core-y += kernel/ spl/ spl/ mm/ fs/ ipc/ security/ crypto/ block/
upon make:
`Makefile:946: target 'spl' given more than once in the same rule``

something like
grep 'kernel/ spl/' "$KERNEL_DIR/Makefile" || sed -i 's#+= kernel/#+= kernel/ spl/#' "$KERNEL_DIR/Makefile"
is completely inelegant, but would fix this.

@behlendorf
Copy link
Contributor

Good point, I'm a little surprised this hasn't come up until now. Your fix looks reasonable to me and is similar to what add_after() does. It's not elegant, but it's definitely functional. Could you please open a pull request with your proposed fix so we can get it tested and reviewed.

@infowolfe
Copy link
Contributor Author

Is there some reason why we're not just using add_after() in this case?

@behlendorf
Copy link
Contributor

Yes, because we need to modify an existing line and not insert a new one.

@infowolfe
Copy link
Contributor Author

Hence the why the PR makes the sed less fragile instead of replacing it with add_after.

@behlendorf
Copy link
Contributor

I must have misunderstood your previous comment, yes the fix looks good.

behlendorf pushed a commit that referenced this issue Jan 29, 2016
Update copy-builtin so it may be run multiple times against
the kernel source tree.  This change makes sed more discriminating
to ensure spl/ only occurs once in core-y.

Signed-off-by: Chip Parker <aparker@enthought.com>
Signed-off-by: Brian Behlendorf <behlendorf1@llnl.gov>
Closes #526
@behlendorf behlendorf added this to the 0.6.5 milestone Mar 23, 2016
@simonvanderveldt
Copy link

simonvanderveldt commented May 8, 2016

Wouldn't it be cleaner to add spl to core-y on a separate line?
That way you could check if the line you've added exists and that's it. Now you have to check a line that already exists and you have no control over.

Expected result in the Makefile:

core-y      += kernel/ mm/ fs/ ipc/ security/ crypto/ block/ <whatever upstream adds>
core-y      += spl/

@behlendorf
Copy link
Contributor

@simonvanderveldt that would be cleaner but there's a dependency issue. The spl directory must be built before the fs directory but after kernel and mm.

@simonvanderveldt
Copy link

@behlendorf ah, didn't know that, that's annoying/unfortunate. Not much we can do about it then.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants