'sed -i' need postfix on mac by DaanHoogland · Pull Request #3312 · apache/cloudstack · GitHub

Description

build fails on mac due to #3247 . adding extension that will only be created in '/target/' anyway

Types of changes

Breaking change (fix or feature that would cause existing functionality to change)
New feature (non-breaking change which adds functionality)
Bug fix (non-breaking change which fixes an issue)
Enhancement (improves an existing feature and functionality)
Cleanup (Code refactoring and cleanup, that may add test cases)

Screenshots (if appropriate):

How Has This Been Tested?

ping @GabrielBrascher @nathanejohnson , this slipped in end of march. didn't notice, too much on fedora :(, not enough on a mac :)

What if we execute the commands at line 63 and 64 like the one in 62? Or, maybe the other way around.

I mean, they do the same thing, but with different approaches. Therefore, it might be interesting to maintain some consistency here.

@rafaelweingartner, not entirely sure what you mean but 62 an 63 can certainly be combined. not sure how 64 can, it is touching a different file.

I meant the following:
All of the commands are replacing "place holders" in the same file. However, two of them are doing in line (via -i flag), whereas line 62 is replacing the entire file (not using the -i flag and redirecting the output of the sed to a file with the same name). I do not mind if we do in different commands the replace, but we could at least try to use the same approach in all of them.

By approach here I mean the method, either inline or not.

I think you are misreading the code,
line 62 redirects to a new file, from "$thisdir/generatetoc_header.xsl" to generatetoc.xsl
then 63 operates on the resulting generatetoc.xsl
but 64 is operating on generatecommands.xsl

I don't think shell operations would allow redecting to input, it would empty the file before openening it for read if you tried that.

@DaanHoogland @rafaelweingartner A (ugly) solution is running the command with the -e and move the new file to the old one:
sed -e "s/%ACS_RELEASE%/${ACS_RELEASE}/g" newGeneratetoc.xsl && mv newGeneratetoc.xsl generatetoc.xsl;

@DaanHoogland on linux the -i must be appended with the .bak (-i.bak)

-i[SUFFIX], --in-place[=SUFFIX]
              edit files in place (makes backup if SUFFIX supplied)

@DaanHoogland env issue, the sed on OSX is freebsd based you can try gnu-sed like:

$ brew install gnu-sed

@DaanHoogland env issue, the sed on OSX is freebsd based

I realize that, but it complicates build and thus inhibits people entering the playing field. I would prefer adding the extension, it is in the target dir and doesn't hurt anybody.

@rhtyd you can easily add gnu sed to mac via homebrew as you point out, however the binary is called 'gsed' to avoid conflicting with the main sed installed in /usr/bin .

LGTM

@blueorangutan package

@rhtyd a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.

Packaging result: ✖centos6 ✖centos7 ✖debian. JID-2752

@DaanHoogland I don't think that this would work on linux. The -i must be appended with the .bak (-i.bak)

Documentation:

-i[SUFFIX], --in-place[=SUFFIX]
              edit files in place (makes backup if SUFFIX supplied)

closed-open PR to rekick Travis
@blueorangutan package

@rhtyd a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.

@DaanHoogland I don't think that this would work on linux. The -i must be appended with the .bak (-i.bak)
@GaborApatiNagy, I'll try and find time to test this on fedora and if so I'll checnge it to the long option.
I hope there is some compatible option/argument set between the two.

Packaging result: ✖centos6 ✖centos7 ✖debian. JID-2799

travis is quite clear in this case: needs tlc

@DaanHoogland can you debug via the backend jenkins what failed?

@rhtyd if you mean travis, yes. but kind of high prio -3.

@DaanHoogland fails on linux with:

Warning, API Cmd class com.cloud.api.commands.SimulatorAddSecondaryAgent has no APICommand annotation 
Scanned and found 607 APIs
sed: -e expression #1, char 1: unknown command: `.'
[ERROR] Failed to execute goal org.codehaus.mojo:exec-maven-plugin:1.2.1:exec (compile) on project cloud-apidoc: Command execution failed. Process exited with an error: 1 (Exit value: 1) -> [Help 1]

This is onhold until Travis passes.

apidoc build failed in Mac OS because of sed in-place command. It is a minor change that fixed this issue. More information here Fixes: #3247 Fixes: #3312

Closing as #4253 fixed it

'sed -i' need postfix on mac

067a591

DaanHoogland added component:api component:build labels May 2, 2019

DaanHoogland requested review from GabrielBrascher and nathanejohnson May 2, 2019 11:44

oops, commented code :(

025851c

rafaelweingartner reviewed May 2, 2019

View reviewed changes

nathanejohnson approved these changes May 23, 2019

View reviewed changes

yadvr approved these changes May 23, 2019

View reviewed changes

yadvr added this to the 4.13.0.0 milestone May 23, 2019

dhlaluku approved these changes May 27, 2019

View reviewed changes

yadvr closed this May 27, 2019

yadvr reopened this May 27, 2019

DaanHoogland added the status:needs-work label May 27, 2019

yadvr removed this from the 4.13.0.0 milestone Jun 25, 2019

borisstoyanov approved these changes Jul 3, 2019

View reviewed changes

yadvr added status:On Hold and removed status:needs-work labels Jul 3, 2019

DaanHoogland mentioned this pull request Mar 24, 2020

maven quality tool plugins #3956

Merged

5 tasks

DaanHoogland marked this pull request as draft May 7, 2020 08:33

soreana mentioned this pull request Aug 8, 2020

Fix sed command failure in Mac OS. #4253

Merged

1 of 5 tasks

yadvr pushed a commit that referenced this pull request Aug 10, 2020

build: Fix sed command failure in Mac OS. (#4253 )

ce7a32c

apidoc build failed in Mac OS because of sed in-place command. It is a minor change that fixed this issue. More information here Fixes: #3247 Fixes: #3312

yadvr closed this Aug 10, 2020

DaanHoogland deleted the apidocToolingOnMac branch August 16, 2023 11:30

                sed -e 's,%API_HEADER%,All APIs,g' "$thisdir/generatetoc_header.xsl" >generatetoc.xsl
-               sed -i "s/%ACS_RELEASE%/${ACS_RELEASE}/g" generatetoc.xsl
-               sed -i "s/%ACS_RELEASE%/${ACS_RELEASE}/g" generatecommands.xsl
+               sed -i .bak "s/%ACS_RELEASE%/${ACS_RELEASE}/g" generatetoc.xsl

'sed -i' need postfix on mac by DaanHoogland · Pull Request #3312 · apache/cloudstack · GitHub

Repository navigation

'sed -i' need postfix on mac - #3312

'sed -i' need postfix on mac#3312DaanHoogland wants to merge 2 commits intoapache:masterapache/cloudstack:masterfrom DaanHoogland:apidocToolingOnMacDaanHoogland/cloudstack:apidocToolingOnMacCopy head branch name to clipboard

Conversation

DaanHoogland commented May 2, 2019

Description

Types of changes

Screenshots (if appropriate):

How Has This Been Tested?

Uh oh!

DaanHoogland commented May 2, 2019

Uh oh!

rafaelweingartner May 2, 2019

Choose a reason for hiding this comment

Uh oh!

DaanHoogland May 2, 2019

Choose a reason for hiding this comment

Uh oh!

rafaelweingartner May 2, 2019

Choose a reason for hiding this comment

Uh oh!

DaanHoogland May 2, 2019

Choose a reason for hiding this comment

Uh oh!

GabrielBrascher May 17, 2019

Choose a reason for hiding this comment

Uh oh!

GabrielBrascher May 17, 2019

Choose a reason for hiding this comment

Uh oh!

yadvr commented May 16, 2019

Uh oh!

DaanHoogland commented May 16, 2019

Uh oh!

nathanejohnson commented May 16, 2019

Uh oh!

nathanejohnson left a comment

Choose a reason for hiding this comment

Uh oh!

yadvr commented May 23, 2019

Uh oh!

blueorangutan commented May 23, 2019

Uh oh!

blueorangutan commented May 23, 2019

Uh oh!

GabrielBrascher commented May 27, 2019

Uh oh!

yadvr commented May 27, 2019

Uh oh!

blueorangutan commented May 27, 2019

Uh oh!

DaanHoogland commented May 27, 2019

Uh oh!

blueorangutan commented May 27, 2019

Uh oh!

DaanHoogland commented May 27, 2019

Uh oh!

yadvr commented May 27, 2019

Uh oh!

DaanHoogland commented May 28, 2019

Uh oh!

yadvr commented Jun 20, 2019

Uh oh!

yadvr commented Jul 3, 2019

Uh oh!

yadvr commented Aug 10, 2020

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

'sed -i' need postfix on mac#3312
DaanHoogland wants to merge 2 commits into
apache:masterfrom
DaanHoogland:apidocToolingOnMac