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

Control nouveau.yaml in configure and dev/run #4956

Merged
merged 1 commit into from
Jan 10, 2024
Merged

Conversation

rnewson
Copy link
Member

@rnewson rnewson commented Jan 9, 2024

Move the generation/templating of nouveau.yaml to couchdb side to make packaging of nouveau simpler and more consistent with the rest.

@@ -265,7 +265,7 @@ private IndexDefinition loadIndexDefinition(final String name) throws IOExceptio
}

private Path indexRootPath(final String name) {
final Path result = rootDir.resolve(name).normalize();
Copy link
Member Author

@rnewson rnewson Jan 9, 2024

Choose a reason for hiding this comment

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

removed this because we send a relative path in a common setup prefixed with ./. For reasons best known to the Great Java In The Sky, data/foo does not 'startsWith' ./data

@rnewson rnewson force-pushed the nouveau-package-2 branch 2 times, most recently from 90be6fb to a32ac66 Compare January 9, 2024 20:38
@rnewson rnewson requested a review from nickva January 9, 2024 21:25
Copy link
Contributor

@nickva nickva left a comment

Choose a reason for hiding this comment

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

+1, very nice!

Was able to create a release from a dist tarball and start it with ./nouveau/bin/nouveau server ./etc/nouveau.yaml

@@ -140,7 +140,6 @@ $InstallDir="$LibDir\couchdb"
$LogFile="$LogDir\couch.log"
$BuildFauxton = [int](-not $DisableFauxton)
$BuildDocs = [int](-not $DisableDocs)
$BuildNouveau = $(If ($EnableNouveau) {1} else {0})
Copy link
Contributor

Choose a reason for hiding this comment

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

iirc, there was a reason to do it that way. $false will get to False, but I need to test this in powershell ...

Copy link
Contributor

Choose a reason for hiding this comment

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

Example:

$EnableNouveau = $false

$CouchDBConfig = @"
{nouveau_enable, "$EnableNouveau"}.
"@

Write-Output "$CouchDBConfig"

Result:

PS F:\> .\test.ps1
{nouveau_enable, "False"}.

Can this be a problem later with the capital "F"?

Note: as we traditionally use relative paths for _dir variables, I
had to modify IndexManager to not normalize() the root dir.
@rnewson rnewson merged commit b275635 into main Jan 10, 2024
14 checks passed
@rnewson rnewson deleted the nouveau-package-2 branch January 10, 2024 10:32
jiahuili430 added a commit that referenced this pull request Jan 10, 2024
Fix `dev/run` setup_configs, generate nouveau configuration when
nouveau is enabled

Related PR: #4956
jiahuili430 added a commit that referenced this pull request Jan 10, 2024
Fix `dev/run` setup_configs, generate nouveau configuration when
nouveau is enabled

Related PR: #4956
pgj pushed a commit to pgj/couchdb that referenced this pull request Jan 10, 2024
Fix `dev/run` setup_configs, generate nouveau configuration when
nouveau is enabled

Related PR: apache#4956
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.

None yet

3 participants