Skip to content

Conversation

@yamt
Copy link
Contributor

@yamt yamt commented Feb 21, 2020

This reverts commit 7a76a97.

@yamt
Copy link
Contributor Author

yamt commented Feb 21, 2020

consider this after apache/nuttx-apps#81

@xiaoxiang781216
Copy link
Contributor

consider this after apache/incubator-nuttx-apps#81

But that patch will break some arch. If you really want to use shlock on macOS, it is better to add a bash script and call shlock/flock before check the platform, but keep ARCHIEVE call.
BTW, It's better to check the platform in the bash instead Make.defs.

@yamt
Copy link
Contributor Author

yamt commented Feb 21, 2020

@xiaoxiang781216 which arch does it break?
using shlock on macOS is not the main point of apache/nuttx-apps#81 .
we can't assume bash, can we? i don't think it's worth to write .bat file for this.

@xiaoxiang781216
Copy link
Contributor

xiaoxiang781216 commented Feb 21, 2020

@xiaoxiang781216 which arch does it break?
using shlock on macOS is not the main point of apache/incubator-nuttx-apps#81 .
we can't assume bash, can we? i don't think it's worth to write .bat file for this.

Here:
https://github.com/FishsemiCode/nuttx/blob/song-u1/arch/ceva/src/song/Toolchain.defs
And you can see we don't define AR at all.
By desgin, the interface between the common Makefile and toolchain is ARCHIVE/COMIPLE... and the comment said that the user can overwrite these macros as need. AR/CC isn't public interface.
If we take your approach: we should use AR/CC.. directly and then ARCHIVE/COMPILE should be removed to avoid the confusion.

@yamt
Copy link
Contributor Author

yamt commented Feb 21, 2020

@xiaoxiang781216 it seems using AR via ARNOEMP. but i got your point.

i wrote another alternative. how do you think?
apache/nuttx-apps#83

@xiaoxiang781216
Copy link
Contributor

@xiaoxiang781216 it seems using AR via ARNOEMP. but i got your point.

i wrote another alternative. how do you think?
apache/incubator-nuttx-apps#83

Has one concern, I reply in that PR.

@patacongo
Copy link
Contributor

The z80 and z16 families are different becuse the ZDS II archiver will only archive one file at a time.

@patacongo
Copy link
Contributor

@xiaoxiang781216: "we can't assume bash, can we?". Yes, in most cases you can assume bash or similar in all cases except Windows native build. You should try not to break the Windows native build any more that possible. That will be revisited in the near future.

Has this discussion reached a conclusion? Is thie PR ready to be merged?

@xiaoxiang781216
Copy link
Contributor

This depend on which approach you want to fix the parellel build issue:
1.Use flock
a.Close this PR directly
b.Merge #342 and apache/nuttx-apps#84
2.Or apply this patch and apache/nuttx-apps#83
but we may need more test since the change is little big, especially the command line may become very long(basiclly all full path .o archieve into libapps.a in one step).

@patacongo patacongo merged commit 884fcb6 into apache:master Feb 21, 2020
julianoes pushed a commit to julianoes/nuttx that referenced this pull request Dec 18, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants