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.

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

bump dev-nginx to 1.5.0 #88

Merged
merged 3 commits into from
Apr 4, 2023
Merged

bump dev-nginx to 1.5.0 #88

merged 3 commits into from
Apr 4, 2023

Conversation

joelochlann
Copy link
Member

@joelochlann joelochlann requested a review from akash1810 April 3, 2023 15:49
@joelochlann
Copy link
Member Author

I'm not sure what's up with the build. It seems to be choking on some git conflict lines but I can't see any in the code...

@akash1810
Copy link
Member

I'm not sure what's up with the build. It seems to be choking on some git conflict lines but I can't see any in the code...

@shtukas and @jonathonherbert have been looking at this as part of #84. Worth sharing notes?

@shtukas
Copy link
Contributor

shtukas commented Apr 3, 2023

Ah.... Interesting. Yes we ( @jonathonherbert and myself ) had a problem with ( #84 ), but the successful test at ( #87 ), shows that it's not on every branch 🤔

@joelochlann @akash1810

@shtukas
Copy link
Contributor

shtukas commented Apr 3, 2023

The only idea I have right now, and that might just be stupid, but I would make a PR from a freshly cloned repository. Might be worth a try.

@joelochlann @jonathonherbert @akash1810

@akash1810
Copy link
Member

akash1810 commented Apr 3, 2023

Our CI script currently uses macos-latest.

In January, macos-latest was moved to macos-12. This uses homebrew 4.0.6.

Looking at this comment, it looks like CI was added to this repository assuming macos-10.15, which comes with homebrew 3.6.6.

That is, we're using a new major version of homebrew. Maybe worth pinning our CI to macos-10.15, and creating an issue to move to macos-latest at a later time? Though I'm not sure why #87 is happily passing the build.

@joelochlann
Copy link
Member Author

Setting macos-10.15 didn't work, nor did trying a fresh clone of the repo

@shtukas
Copy link
Contributor

shtukas commented Apr 3, 2023

There is a difference between your attempt and mine. I have an extra line, which seems to make a difference: https://github.com/guardian/homebrew-devtools/pull/90/files#diff-944291df2c9c06359d37cc8833d182d705c9e8c3108e7cfe132d61a06e9133ddR19

akash1810 added a commit that referenced this pull request Apr 3, 2023
The build log of #88 contains:

```log
error: could not apply 1728e63... Merge 0da6e2d into 02dd552
hint: Resolve all conflicts manually, mark them as resolved with
hint: "git add/rm <conflicted_files>", then run "git rebase --continue".
hint: You can instead skip this commit: run "git rebase --skip".
hint: To abort and get back to the state before "git rebase", run "git rebase --abort".
Could not apply 1728e63... Merge 0da6e2d into 02dd552
```

Unsure why, let's perform `git status` to narrow the cause down.
This was referenced Apr 3, 2023
@akash1810
Copy link
Member

#93 offers a fix - the exact cause remains somewhat unknown, but at least the build passes.

akash1810 added a commit that referenced this pull request Apr 4, 2023
The builds in #84 and #88 are failing with messages that suggest a git conflict:

```log
Error: guardian/devtools/ssm: /usr/local/Homebrew/Library/Taps/guardian/homebrew-devtools/Formula/ssm.rb:4: syntax error, unexpected <<, expecting end
<<<<<<< HEAD
^~
/usr/local/Homebrew/Library/Taps/guardian/homebrew-devtools/Formula/ssm.rb:8: syntax error, unexpected ===, expecting end
=======
^~~
/usr/local/Homebrew/Library/Taps/guardian/homebrew-devtools/Formula/ssm.rb:12: syntax error, unexpected >>, expecting end
>>>>>>> c7fbe61 (Merge dac26ab...
^~
/usr/local/Homebrew/Library/Taps/guardian/homebrew-devtools/Formula/ssm.rb:12: syntax error, unexpected tIDENTIFIER, expecting ')'
...ec66d58337ac276f718186ac0c8a059)
...^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
```

Indeed, this is confirmed via #92, which demonstrates that somewhere in the build homebrew is making changes to the checked out code in a way that causes git conflicts. This results in the above error message being witnessed when we attempt to install from the checked out code.

In this change, we split the CI steps up, changing the ordering.

Before
  1. Checkout branch
  2. Apply AWS CLI work-around
  3. Update homebrew
  4. Install from local file system

After
  1. Apply AWS CLI work-around
  2. Update homebrew
  3. Checkout branch
  4. Install from local file system

This should guarantee that homebrew does not mutate the checked out branch, and therefore make the build more deterministic, and stable.
akash1810 added a commit that referenced this pull request Apr 4, 2023
The builds in #84 and #88 are failing with messages that suggest a git conflict:

```log
Error: guardian/devtools/ssm: /usr/local/Homebrew/Library/Taps/guardian/homebrew-devtools/Formula/ssm.rb:4: syntax error, unexpected <<, expecting end
<<<<<<< HEAD
^~
/usr/local/Homebrew/Library/Taps/guardian/homebrew-devtools/Formula/ssm.rb:8: syntax error, unexpected ===, expecting end
=======
^~~
/usr/local/Homebrew/Library/Taps/guardian/homebrew-devtools/Formula/ssm.rb:12: syntax error, unexpected >>, expecting end
>>>>>>> c7fbe61 (Merge dac26ab...
^~
/usr/local/Homebrew/Library/Taps/guardian/homebrew-devtools/Formula/ssm.rb:12: syntax error, unexpected tIDENTIFIER, expecting ')'
...ec66d58337ac276f718186ac0c8a059)
...^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
```

Indeed, this is confirmed via #92, which demonstrates that somewhere in the build homebrew is making changes to the checked out code in a way that causes git conflicts. This results in the above error message being witnessed when we attempt to install from the checked out code.

In this change, we split the CI steps up, changing the ordering.

Before
  1. Checkout branch
  2. Apply AWS CLI work-around
  3. Update homebrew
  4. Install from local file system

After
  1. Apply AWS CLI work-around
  2. Update homebrew
  3. Checkout branch
  4. Install from local file system

This should guarantee that homebrew does not mutate the checked out branch, and therefore make the build more deterministic, and stable.
@akash1810
Copy link
Member

#94 has been merged now. A rebase of main should get the build in this PR passing 🤞🏽.

@joelochlann
Copy link
Member Author

Thanks @akash1810!

This reverts commit 9d02924.
@joelochlann
Copy link
Member Author

Build looks good! ✅

Are you able to give a 👍 @akash1810?

@joelochlann joelochlann merged commit 06b87d3 into main Apr 4, 2023
@joelochlann joelochlann deleted the js-bump-to-v1.5.0 branch April 4, 2023 12:51
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